teemperor added a comment. In D85993#2253465 <https://reviews.llvm.org/D85993#2253465>, @amccarth wrote:
> 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`. No idea about the PDB parsing code, but `PdbAstBuilder::GetOrCreateFunctionDecl` seems to be the right place. It already has a check for when the context is a TagDecl or a namespace, so doing the same thing for the CreateFunctionDecl call below should do the trick (namespace -> CreateFunctionDecl, TagDecl -> AddMethodToCXXRecordType). > 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? Confusingly FunctionTemplateDecls *can* be inside a record and the name is just misleading. The actual specialisations of a FunctionTemplateDecl in a record are again CXXMethodDecls, so `CreateFunctionTemplateDecl` is fine: struct Foo { template<typename T> int x() { return 3; } int foo() { return x<double>(); } }; $ clang -fsyntax-only -Xclang -ast-dump foo.cpp ... `-CXXRecordDecl 0x7f9849077f50 <method.cpp:1:1, line:9:1> line:1:8 struct Foo definition |-FunctionTemplateDecl 0x7f98490782d0 <line:2:3, line:5:3> line:3:7 x | |-TemplateTypeParmDecl 0x7f98490780f8 <line:2:12, col:21> col:21 typename depth 0 index 0 T | |-CXXMethodDecl 0x7f9849078230 <line:3:3, line:5:3> line:3:7 x 'int ()' | `-CXXMethodDecl 0x7f98490785e0 <line:3:3, line:5:3> line:3:7 used x 'int ()' ... 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
