shafik added inline comments.
================ Comment at: lldb/source/Symbol/ClangASTContext.cpp:1347 + if (decl_ctx->isRecord()) + func_tmpl_decl->setAccess(clang::AccessSpecifier::AS_public); ---------------- teemperor wrote: > shafik wrote: > > Where is the method being added and why are we not setting the access > > there? Are we creating it though `CreateFunctionTemplateDecl` should be be > > checking the `DeclContext` there to see if it is a `RecordDecl`? > We actually don't add it anywhere but instead use an already created related > FunctionDecl and set this FunctionTemplateDecl as its specialisation (see > `lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1216`). > > We could also add the FunctionTemplateDecl to the DeclContext explicitly. > That doesn't seem to break anything and seems consistent with the other > functions for creating Decls. On the other hand I'm not sure if this is is > some untested workaround for something. > > But in any case, whatever we returned from this function (or whether it was > added to the DeclContext) is in an invalid state that triggers an assert as > soon as anyone calls getAccess on it. Right so b/c this is a templated class member function we have a large comment above this check in `DWARFASTParserClang.cpp`: ``` if (is_cxx_method && has_template_params) ``` explaining why it is not added as a C++ method, which means we don't add this via `m_ast.AddMethodToCXXRecordType(...)` which would get the access specifiers set at all. I feel like we should handle this in `DWARFASTParserClang.cpp` around line `1216` and get the access specifier correct instead of hack where we just set it to public. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71909/new/ https://reviews.llvm.org/D71909 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
