[clang-import-test] Lookup inside entities

[spyffe]

Summary

clang-import-test has until now been only able to report top-level entities. This is clearly insufficient; we should be able to look inside structs and namespaces also.
This patch adds new test cases for a variety of lookups inside existing entities, and adds the functionality necessary to make most of these testcases work.
One testcase is known to fail because of ASTImporter limitations when importing templates; this will be addressed separately.

[aprantl]

A few stylistic issues inline.
It also wouldn't hurt to document what is being tested, either in the clang-import-test sources or in the input files. Otherwise this looks fine. More tests are always great!

tools/clang-import-test/clang-import-test.cpp

213

Sort of an irrelevant nitpick, but we don't typically use curly braces on single statements.

228

->

// Couldn't find the name, so we have to give up.
return nullptr;

[spyffe]

• Applied Adrian's suggestion to move comments before the line they apply to.
• Got to 100% coverage by changing a test and making an assert for something I couldn't make happen in a test.

I did not take Adrian's suggestion to remove braces around a single-statement if because (as far as I can tell) the LLVM coding standard is silent on the topic and I prefer to have the braces there.

[a.sidorin]

Hello Sean,

It is good to have the ability of recursive lookup. But for me (and I'm sorry), the code becomes very hard to understand: I need to track if the Decl/DC is in the source AST or in the AST being imported, if it was imported or if was in the AST before across lambdas and calls. Can we make this more clear?

tools/clang-import-test/clang-import-test.cpp

201

Can we make the usage of qualified and non-qualified casts consistent? Usually, casts are used as unqualified.

301

I guess we can use SmallVectors here too.

306

Nit: to make the code cleaner, we can extract this std::none_of to a separate function like IsFoundAsForward(). Nested lambdas are harder to read.

336

The loop:

Decls.reserve(DeclsToReport.size());
for (Candidate &C : DeclsToReport)
  Decls.push_back(cast<NamedDecl>(C.second->Import(C.first)));

will look a bit nicer here. What do you think?

[spyffe]

• Removed namespace qualification of cast across the board.
• Standardized on SmallVector
• Factored out the filtering of forward declarations
• Fixed a loop as requested.
• Made a Origin<> wrapper for arbitrary AST objects that makes it clear where they live, so importation and searching gets a little clearer.

[spyffe]

• Broke all the ASTImporter and ExternalASTSource subclassing logic into its own source file, with its API defined in AST/ExternalASTMerger.h.
• Cleaned up the API for LLDB's consumption: now rather than a CompilerInstance a client just has to provide an ASTContext and a FileManager (just like when setting up an ASTImporter). This allows e.g. DWARF to be a valid source.

[spyffe]

Added the ExternalASTMerger implementation/interface, which I hadn't svn added before generating the last diff.

[a.sidorin]

Looks excellent now. Thank you Sean!

test/Import/forward-declared-struct/Inputs/S1.c

+struct S;

test/Import/forward-declared-struct/Inputs/S2.c

+struct S {
+  int a;
+};

test/Import/forward-declared-struct/test.c

+// RUN: clang-import-test -import %S/Inputs/S1.c --import %S/Inputs/S2.c -expression %s
+void expr() {
+  struct S MyS;
+  MyS.a = 3;
+}

test/Import/member-in-struct/Inputs/S.c

+struct S {
+  int a;
+};

test/Import/member-in-struct/test.c

+// RUN: clang-import-test -import %S/Inputs/S.c -expression %s
+void expr() {
+  struct S MyS;
+  MyS.a = 3;
+}

test/Import/multiple-forward-declarations/Inputs/S1.c

+struct S;

test/Import/multiple-forward-declarations/Inputs/S2.c

+struct S;

test/Import/multiple-forward-declarations/test.c

+// RUN: clang-import-test -import %S/Inputs/S1.c --import %S/Inputs/S2.c -expression %s
+void expr() {
+  struct S *MySPtr;
+}

test/Import/overloaded-function/Inputs/F1.c

+void f(int arg) { }
 No newline at end of file

test/Import/overloaded-function/Inputs/F2.c

+struct S { int a; };
+
+void f(const char *arg) { }
+void f(S arg) { }
 No newline at end of file

test/Import/overloaded-function/test.c

+// RUN: clang-import-test -import %S/Inputs/F1.c -import %S/Inputs/F2.c -expression %s
+void expr() {
+  f(2);
+  f("world");
+  S s;
+  f(s);
+}

test/Import/struct-in-namespace/Inputs/N1.cpp

+namespace N {
+  struct S {
+    int a;
+  };
+}
+
+namespace N {
+  struct T {
+    int b;
+  };
+}
 No newline at end of file

test/Import/struct-in-namespace/Inputs/N2.cpp

+namespace N {
+  struct U {
+    int c;
+  };
+}
 No newline at end of file

test/Import/struct-in-namespace/test.cpp

+// RUN: clang-import-test -import %S/Inputs/N1.cpp -import %S/Inputs/N2.cpp -expression %s
+void expr() {
+  N::S s;
+  N::T t;
+  N::U u;
+  int d = s.a + t.b + u.c;
+}

test/Import/template-specialization/Inputs/T.cpp

+template <typename T> struct A {
+};
+
+template <> struct A<int> {
+  struct B {
+    int f;
+  };
+};
+
+template <> struct A<bool> {
+  struct B {
+    int g;
+  };
+};
 No newline at end of file

test/Import/template-specialization/test.cpp

+// RUN: clang-import-test -import %S/Inputs/T.cpp -expression %s
+// XFAIL: *
+void expr() {
+  A<int>::B b1;
+  A<bool>::B b2;
+  b1.f + b2.g;
+}

tools/clang-import-test/clang-import-test.cpp

 //===-- import-test.cpp - ASTImporter/ExternalASTSource testbed -----------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTImporter.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "clang/CodeGen/ModuleBuilder.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticBuffer.h"
std::unique_ptr<CodeGenerator> BuildCodeGen(CompilerInstance &CI,
   StringRef ModuleName("$__module");
   return std::unique_ptr<CodeGenerator>(CreateLLVMCodeGen(
       CI.getDiagnostics(), ModuleName, CI.getHeaderSearchOpts(),
       CI.getPreprocessorOpts(), CI.getCodeGenOpts(), LLVMCtx));
 }
 } // end namespace
 
 namespace {
+class TestASTImporter : public ASTImporter {
+public:
+  TestASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
+                  ASTContext &FromContext, FileManager &FromFileManager,
+                  bool MinimalImport)
+      : ASTImporter(ToContext, ToFileManager, FromContext, FromFileManager,
+                    MinimalImport) {}
+  Decl *Imported(Decl *From, Decl *To) override {
+    if (auto ToTag = llvm::dyn_cast<TagDecl>(To)) {
+      ToTag->setHasExternalLexicalStorage();
+    } else if (auto ToNamespace = llvm::dyn_cast<NamespaceDecl>(To)) {
+      ToNamespace->setHasExternalVisibleStorage();
+    }
+    return ASTImporter::Imported(From, To);
+  }
+};
+
+const DeclContext *LookupSameContext(const TranslationUnitDecl *SearchTU,
+                                     const DeclContext *SourceDC,
+                                     ASTImporter &SourceToSearchImporter) {
+  if (SourceDC->isTranslationUnit()) {
+    return SearchTU;
+  }
+  const DeclContext *SearchParentDC =
+      LookupSameContext(SearchTU, SourceDC->getParent(), SourceToSearchImporter);
+  if (!SearchParentDC) {
+    return nullptr;
+  }
+  if (auto SourceNamedDecl = dyn_cast<NamedDecl>(SourceDC)) {
+    DeclarationName SourceName = SourceNamedDecl->getDeclName();
+    DeclarationName SearchName = SourceToSearchImporter.Import(SourceName);
+    DeclContext::lookup_result SearchResult =
+        SearchParentDC->lookup(SearchName);
+    size_t SearchResultSize = SearchResult.size();
+    if (SearchResultSize == 0) {
+      return nullptr; // couldn't find the name, so we have to give up
+    } else if (SearchResultSize > 1) {
+      return nullptr; // unhandled case; here we need to perform overload
+                      // resolution
+    } else {
+      NamedDecl *SearchResultDecl = SearchResult[0];
+      return dyn_cast<DeclContext>(SearchResultDecl);
+    }
+  } else {
+    return nullptr; // unhandled case; here we need to handle e.g.
+                    // NamespaceDecls
+  }
+}
+
+bool IsForwardDeclaration(Decl *D) {
+  assert(!isa<ObjCInterfaceDecl>(D)); // TODO handle this case
+  if (auto TD = dyn_cast<TagDecl>(D)) {
+    return !TD->isThisDeclarationADefinition();
+  } else if (auto FD = dyn_cast<FunctionDecl>(D)) {
+    return !FD->isThisDeclarationADefinition();
+  } else {
+    return false;
+  }
+}
+
 class TestExternalASTSource : public ExternalASTSource {
 private:
   llvm::ArrayRef<std::unique_ptr<CompilerInstance>> ImportCIs;
-  std::map<CompilerInstance *, std::unique_ptr<ASTImporter>> ForwardImporters;
+  std::map<CompilerInstance *, std::unique_ptr<TestASTImporter>>
+      ForwardImporters;
   std::map<CompilerInstance *, std::unique_ptr<ASTImporter>> ReverseImporters;
 
 public:
   TestExternalASTSource(
       CompilerInstance &ExpressionCI,
       llvm::ArrayRef<std::unique_ptr<CompilerInstance>> ImportCIs)
       : ImportCIs(ImportCIs) {
     for (const std::unique_ptr<CompilerInstance> &ImportCI : ImportCIs) {
-      ForwardImporters[ImportCI.get()] = llvm::make_unique<ASTImporter>(
+      ForwardImporters[ImportCI.get()] = llvm::make_unique<TestASTImporter>(
           ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
           ImportCI->getASTContext(), ImportCI->getFileManager(),
           /*MinimalImport=*/true);
       ReverseImporters[ImportCI.get()] = llvm::make_unique<ASTImporter>(
           ImportCI->getASTContext(), ImportCI->getFileManager(),
           ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
           /*MinimalImport=*/true);
     }
   }
 
+  void ForEachMatchingDC(const DeclContext *DC,
+                         std::function<void(ASTImporter &ForwardImporter,
+                                            ASTImporter &ReverseImporter,
+                                            const DeclContext *SearchDC)>
+                             Callback) {
+    for (const std::unique_ptr<CompilerInstance> &I : ImportCIs) {
+      TranslationUnitDecl *SearchTU =
+          I->getASTContext().getTranslationUnitDecl();
+      ASTImporter &ForwardImporter = *ForwardImporters[I.get()];
+      ASTImporter &ReverseImporter = *ReverseImporters[I.get()];
+      const DeclContext *SearchDC =
+          LookupSameContext(SearchTU, DC, ReverseImporter);
+      if (SearchDC) {
+        Callback(ForwardImporter, ReverseImporter, SearchDC);
+      }
+    }
+  }
+
   bool FindExternalVisibleDeclsByName(const DeclContext *DC,
                                       DeclarationName Name) override {
     llvm::SmallVector<NamedDecl *, 1> Decls;
 
-    if (isa<TranslationUnitDecl>(DC)) {
-      for (const std::unique_ptr<CompilerInstance> &I : ImportCIs) {
-        DeclarationName FromName = ReverseImporters[I.get()]->Import(Name);
-        DeclContextLookupResult Result =
-            I->getASTContext().getTranslationUnitDecl()->lookup(FromName);
-        for (NamedDecl *FromD : Result) {
-          NamedDecl *D =
-              llvm::cast<NamedDecl>(ForwardImporters[I.get()]->Import(FromD));
-          Decls.push_back(D);
+    typedef std::pair<NamedDecl *, ASTImporter *> Candidate;
+
+    std::vector<Candidate> CompleteDecls;
+    std::vector<Candidate> ForwardDecls;
+
+    auto FilterFoundDecl = [&CompleteDecls, &ForwardDecls](const Candidate &C) {
+      if (IsForwardDeclaration(C.first)) {
+        if (std::none_of(ForwardDecls.begin(), ForwardDecls.end(),
+                         [&C](Candidate &D) {
+                           return C.first->getKind() == D.first->getKind();
+                         })) {
+          ForwardDecls.push_back(C);
         }
+      } else {
+        CompleteDecls.push_back(C);
       }
+    };
+
+    ForEachMatchingDC(
+        DC, [Name, &FilterFoundDecl](ASTImporter &ForwardImporter,
+                                     ASTImporter &ReverseImporter,
+                                     const DeclContext *SearchDC) {
+          DeclarationName FromName = ReverseImporter.Import(Name);
+          DeclContextLookupResult Result = SearchDC->lookup(FromName);
+          for (NamedDecl *FromD : Result) {
+            FilterFoundDecl(std::make_pair(FromD, &ForwardImporter));
           }
-    if (Decls.empty()) {
+        });
+
+    std::vector<Candidate> &DeclsToReport =
+        CompleteDecls.empty() ? ForwardDecls : CompleteDecls;
+
+    if (DeclsToReport.empty()) {
       return false;
-    } else {
-      SetExternalVisibleDeclsForName(DC, Name, Decls);
-      return true;
-    }
+    }
+
+    Decls.resize(DeclsToReport.size());
+    std::transform(DeclsToReport.begin(), DeclsToReport.end(), Decls.begin(),
+                   [](Candidate &C) {
+                     return llvm::cast<NamedDecl>(C.second->Import(C.first));
+                   });
+
+  void
+  FindExternalLexicalDecls(const DeclContext *DC,
+                           llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
+                           SmallVectorImpl<Decl *> &Result) override {
+    ForEachMatchingDC(DC, [DC, IsKindWeWant](ASTImporter &ForwardImporter,
+                                             ASTImporter &ReverseImporter,
+                                             const DeclContext *SearchDC) {
+      for (Decl *SearchDecl : SearchDC->decls()) {
+        if (IsKindWeWant(SearchDecl->getKind())) {
+          Decl *ImportedDecl = ForwardImporter.Import(SearchDecl);
+          assert(ImportedDecl->getDeclContext() == DC);
+        }
+      }
+    });
   }
 };
 
 void AddExternalSource(
     CompilerInstance &CI,
     llvm::ArrayRef<std::unique_ptr<CompilerInstance>> Imports) {
   ASTContext &AST = CI.getASTContext();
   auto ES = llvm::make_unique<TestExternalASTSource>(CI, Imports);