zturner added inline comments.
================
Comment at: lit/SymbolFile/PDB/class-layout.test:3
+RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/ClassLayoutTest.cpp /o
%T/ClassLayoutTest.cpp.obj
+RUN: link %T/ClassLayoutTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main
/OUT:%T/ClassLayoutTest.cpp.exe
+RUN: lldb-test symbols %T/ClassLayoutTest.cpp.exe | FileCheck %s
----------------
If you change this to `lld-link` this test may be able to run on non windows
today.
================
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:186
+ return lldb::eAccessProtected;
+ return lldb::eAccessNone;
+}
----------------
I wonder if this would be better as an `llvm_unreachable`. Being able to
assume this function returns a sane value would simplify calling code.
================
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:198
+ return lldb::eAccessPrivate;
+ return lldb::eAccessNone;
+}
----------------
And here.
================
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:246
+ return ty ? ty->shared_from_this() : nullptr;
+ } break;
case PDB_SymType::UDT: {
----------------
This looks unusual. Did you clang-format it?
================
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:271-272
+ AccessType access = TranslateMemberAccess(udt->getAccess());
+ if (access == lldb::eAccessNone && udt->isNested() &&
+ udt->getClassParent()) {
+ access = GetDefaultAccessibilityForUdtKind(udt_kind);
----------------
So is this `access == None` a thing that can actually happen? AFAICT it's
impossible for `getAccess()` to return anything other than public, protected,
or private, in which case this code path will never get executed, so we can
just delete it.
================
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:291-295
+ if (udt->isConstType())
+ clang_type = clang_type.AddConstModifier();
+
+ if (udt->isVolatileType())
+ clang_type = clang_type.AddVolatileModifier();
----------------
Can you remind me what this means? How would an entire type be marked const or
volatile? There's no instance variable in play here, these are just types as
they are declared in the source code, if I'm not mistaken. Something like
`const class Foo {int x; };` doesn't make any sense. So I'm curious under what
circumstances these 2 if statements would evaluate to true.
https://reviews.llvm.org/D49410
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
