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

[Raphael Isemann via Phabricator via lldb-commits]

teemperor marked an inline comment as done.
teemperor added inline comments.


================
Comment at: lldb/source/Symbol/StdModuleHandler.cpp:242
+    // Instantiate the template.
+    found_decl = ClassTemplateSpecializationDecl::Create(
+        m_sema->getASTContext(),
----------------
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`?


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

https://reviews.llvm.org/D59537