[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

[Gabor Marton via Phabricator via lldb-commits]

martong added inline comments.


================
Comment at: lldb/source/Symbol/StdModuleHandler.cpp:242
+    // Instantiate the template.
+    found_decl = ClassTemplateSpecializationDecl::Create(
+        m_sema->getASTContext(),
----------------
teemperor wrote:
> martong wrote:
> > martong wrote:
> > > Is there any guarantee that the before any subsequent clang::ASTImporter::Import call this newly created node is registered as imported (via ASTImporter::MapImported)?
> > > 
> > > The reason why I am asking this is because we have to enforce in clang::ASTImporter that after every Decl is created then immediately it is registered as an imported Decl, see `ASTImporter::GetImportedOrCreateDecl`. This we we make sure that no subsequent import calls will create the same node again.
> > > The cycles in the AST are handled properly only this way.
> > > 
> > > This is one reason which makes me worried about exposing the import mechanism via `ImportInternal`.  Would it be working if `GetImportedOrCreateDecl` was virtual and overridden here?
> > > Would it be working if GetImportedOrCreateDecl was virtual and overridden here?
> > 
> > Sorry, that would certainly not work, but perhaps a virtual hook inside that function could work...
> Sorry for the late reply.
> 
> So I was looking into changing the code to your suggestion, but I don't think we can get this working reliably. The problem is that if we intercept the importing in GetImportedOrCreateDecl then we need to intercept not just `std::vector` but also all related imports that the ASTImporter may make before importing `std::vector` itself, e.g. DeclContext, parent classes, return type, template args, etc. Also we constantly would need to update this list in case the standard library or the ASTImporter change internally to make sure we intercept everything.
> 
> Maybe we could instead implement an mechanism in the LLDB code that ensures we do call MapImported and add the decl to the lookup directly after creating a decl? E.g. by implementing something similar to ASTImporter's `GetImportedOrCreateDecl`?
Yes, I think that could work.
So, we could have a new function:
```
void CreateDeclPostamble(Decl *FromD, Decl* ToD) {  // better naming?
      // Keep track of imported Decls.
      Importer.MapImported(FromD, ToD);
      Importer.AddToLookupTable(ToD);
      InitializeImportedDecl(FromD, ToD);
}
```
And we'd call this from `GetImportedOrCreateDecl` and here after `::Create`.


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

https://reviews.llvm.org/D59537