[Lldb-commits] [PATCH] D71909: [lldb] Fix crash in AccessDeclContextSanity when copying FunctionTemplateDecl inside a record.

[Shafik Yaghmour via Phabricator via lldb-commits]

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