[Lldb-commits] [PATCH] D129807: [LLDB][NativePDB] Add MSInheritanceAttr when completing CXXRecordDecl

[Reid Kleckner via Phabricator via lldb-commits]

rnk added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:492
   if (pr.isPointerToMember()) {
     MemberPointerInfo mpi = pr.getMemberInfo();
     GetOrCreateType(mpi.ContainingType);
----------------
I think you want to make code changes here to look at `mpi.Representation`.


================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:762
 
 VariableSP SymbolFileNativePDB::CreateGlobalVariable(PdbGlobalSymId var_id) {
   CVSymbol sym = m_index->symrecords().readRecord(var_id.offset);
----------------
This codepath seems specific to global variable creation. Are there other ways to hit this issue through other codepaths, such as local variables or non-type template parameters?

I would consider moving this logic closer to the logic which creates the AST member pointer type. Any time LLDB loads a member pointer type, it should check the inheritance kind, and then check if the class already has an MSInheritanceAttr, and add one if it is missing.

If the existing attribute doesn't match the attribute you want to add, that represents an ODR violation in the user program. I'm not sure what LLDB should do in that case.


================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:319
+      spelling = clang::MSInheritanceAttr::Keyword_virtual_inheritance;
+    else if (bases.size() > 1)
+      spelling = clang::MSInheritanceAttr::Keyword_multiple_inheritance;
----------------
We shouldn't try to reimplement the frontend calculation logic, we should just trust the debug info. Users can use the [pointers_to_members pragma](https://docs.microsoft.com/en-us/cpp/preprocessor/pointers-to-members?view=msvc-170) to force the compiler to use different representations, so this calculation won't always be right.

Also, I don't think this case handles multiple inheritance via indirect bases. Consider:
```
struct A { int a; };
struct B { int b; };
struct C : A, B { int c; };
struct D : C { int d; };
```

If we need this logic in LLDB, try calling `CXXRecordDecl::calculateInheritanceModel`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129807/new/

https://reviews.llvm.org/D129807