teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

In D72946#1827372 <https://reviews.llvm.org/D72946#1827372>, @clayborg wrote:

> Is an AST importer specific to a target? Can we just put it into the Clang 
> AST type system subclass and create it lazily?


If ClangASTImporter was stateless I would agree. But as the ClangASTImporter 
stores and hands out ASTImporters, it has a *lot* of state. So in theory we 
should only have one in LLDB to not have different state that goes out of sync. 
That doesn't fully work as the modules and the target are independent, so 
having 1(target) + N(modules) is the best we can do I think.

Anyway, the current approach moves it into the ClangPersistentVariables (which 
are part of the TypeSystemClangForExpressions which is the one for the 
target-unique scratch ASTContext) so this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72946



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to