[Lldb-commits] [PATCH] D72946: [lldb] Remove ClangASTImporter reference from Target

[Raphael Isemann via Phabricator via lldb-commits]

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