[PATCH] D68140: [lldb][clang][modern-type-lookup] Use ASTImporterSharedState in ExternalASTMerger

[Gabor Marton via Phabricator via llvm-commits]

martong added a comment.

Raphael, thanks for working on this. Overall, the changes look good to me, but please see my comment for the constructor.



================
Comment at: cfe/trunk/lib/AST/ExternalASTMerger.cpp:320
+  SharedState = std::make_shared<ASTImporterSharedState>(
+      *Target.AST.getTranslationUnitDecl());
   AddSources(Sources);
----------------
For safety, it would make sense to check that there isn't any ExternalASTSource attached to `Target.AST` yet.
(`assert(Target.AST.getExternalSource() == nullptr)` ?)

If that is not the case then the constructor of the ASTImporterSpecificLookup (inited from the SharedState ctor) will traverse through the decls and that would result in consulting with the existing and already set ExternalSource. And that is something that we should avoid because we are right now in the middle of setting up the ExternalSource.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68140