[Lldb-commits] [PATCH] D85993: [lldb] Set the access property on member function decls

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

In D85993#2220724 <https://reviews.llvm.org/D85993#2220724>, @labath wrote:

> Dwarf parser uses `TypeSystemClang::AddMethodToCXXRecordType` instead of this function to create methods (which is why there are no assertions like this when using dwarf). Maybe it would be better to change the pdb parser to use that function instead (as it allows the user to specify not only accessibility, but also other potentially useful properties of the created method -- static, virtual, etc.).

The NativePDB AST parser also uses `AddMethodToCXXRecordType`.  It's in udtcompleter.cpp.

But the bug is triggered while the plugin is trying to synthesize the function declaration during a `ResolveSymbolContext` call.  It looks like the DWARF implementation of `ResolveSymbolContext` relies on its AST parser, but the NativePDB one does not.  I'm trying to figure out how to do that.

Also note that `AddMethodToCXXRecordType` just sets whatever access type it's asked to set.  Before calling `AddMethodToCXXRecordType`, the DWARF parser has this gem:

  // Neither GCC 4.2 nor clang++ currently set a valid
  // accessibility in the DWARF for C++ methods...
  // Default to public for now...
  if (attrs.accessibility == eAccessNone)
    attrs.accessibility = eAccessPublic;

That bit of logic is what prevents the assertion from failing for DWARF.  I'll can make the NativePDB AST parser do the same thing.

But that won't fix the bug, since the NativePDB AST parser isn't involved (at least, my breakpoints in the parser never hit during the course of the test).  In the mean time, I'll try to figure out how to get the NativePDB plugin's `ResolveSymbolContext` implementation to use `AddMethodToCXXRecord`.

> And this function could assert that it is _only_ used for creating free functions ?

Sure.  Sounds like a good idea.

I'll also assume that, since you don't want my access-fixup in `TypeSystemClang::CreateFunctionDeclaration`, then you also would want me to remove the already-existing such fixup from `TypeSystemClang::CreateFunctionTemplateDecl`.  Does that make sense?


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

https://reviews.llvm.org/D85993