[PATCH] D38208: Add support for remembering origins to ExternalASTMerger

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added inline comments.


================
Comment at: lib/AST/ExternalASTMerger.cpp:79
+      return cast<DeclContext>(SearchResultDecl)->getPrimaryContext();
+    else
+      return nullptr; // This type of lookup is unsupported
----------------
No need for `else` here.


================
Comment at: lib/AST/ExternalASTMerger.cpp:173
+ASTImporter &ExternalASTMerger::ImporterForOrigin(ASTContext &OriginContext) {
+  for (const std::unique_ptr<ASTImporter> &I : Importers) {
+    if (&I->getFromContext() == &OriginContext)
----------------
No need for curly braces here.


================
Comment at: lib/AST/ExternalASTMerger.cpp:189
+bool ExternalASTMerger::HasImporterForOrigin(ASTContext &OriginContext) {
+  for (const std::unique_ptr<ASTImporter> &I : Importers) {
+    if (&I->getFromContext() == &OriginContext)
----------------
No need for curly braces here.


================
Comment at: lib/AST/ExternalASTMerger.cpp:289
+      LookupSameContext(Origin.AST->getTranslationUnitDecl(), ToDC, Reverse);
+  if (!FoundFromDC || !IsSameDC(FoundFromDC.get(), Origin.DC)) {
+    if (LoggingEnabled())
----------------
You can probably simplify this to something like:


```
bool RecordOrigin = !FoundFromDC || !IsSameDC(FoundFromDC.get(), Origin.DC);

if (RecordOrigin)
  RecordOriginImpl(ToDC, Origin, Importer);
  
if (LoggingEnabled()) {
  logs() << "(ExternalASTMerger*)" << (void*)this
         << " decided";
         
  if (!RecordOrigin)
    logs() << " NOT";
    
  logs() << " to record origin (DeclContext*)" << (void*)Origin.DC
         << ", (ASTContext*)" << (void*)&Origin.AST
         << "\n";
}
```


================
Comment at: lib/AST/ExternalASTMerger.cpp:380
+
+  if (Candidates.empty()) {
+    return false;
----------------
No need for curly braces here.


================
Comment at: tools/clang-import-test/clang-import-test.cpp:329
     CG.GetModule()->print(llvm::outs(), nullptr);
-  if (CI->getDiagnosticClient().getNumErrors()) {
+  if (CI.getDiagnosticClient().getNumErrors()) {
     return llvm::make_error<llvm::StringError>(
----------------
No need for curly braces here


Repository:
  rL LLVM

https://reviews.llvm.org/D38208