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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to