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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
