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



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

Reply via email to