Improved import of 'C implicit' functions. (#689)

Author: balazske

lib/AST/ASTImporter.cpp

@@ -3001,7 +3001,6 @@ Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD,
 }
 
 ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
-
   SmallVector<Decl *, 2> Redecls = getCanonicalForwardRedeclChain(D);
   auto RedeclIt = Redecls.begin();
   // Import the first part of the decl chain. I.e. import all previous
@@ -3048,9 +3047,22 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
   }
   // Try to find a function in our own ("to") context with the same name, same
   // type, and in the same context as the function we're importing.
-  else if (!LexicalDC->isFunctionOrMethod()) {
+  else {
+    // Detect functions that have no prototype before (valid in C only).
+    // These are implicitly contained in a CallExpr without previous
+    // FunctionDecl, in namespace IDNS_LocalExtern. The AST contains the same
+    // FunctionDecl at every CallExpr where that function is called.
+    auto HasNoPrototype = [](FunctionDecl *D) {
+      return D->getLexicalDeclContext()->isFunctionOrMethod() &&
+             D->isImplicit();
+    };
+    bool NoPrototype = HasNoPrototype(D);
+    assert((!NoPrototype || !D->getPreviousDecl()) &&
+           "Function without prototype but with previous decl?");
+
     SmallVector<NamedDecl *, 4> ConflictingDecls;
-    unsigned IDNS = Decl::IDNS_Ordinary | Decl::IDNS_OrdinaryFriend;
+    unsigned IDNS = Decl::IDNS_Ordinary | Decl::IDNS_OrdinaryFriend |
+                    Decl::IDNS_LocalExtern;
     auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
     for (auto *FoundDecl : FoundDecls) {
       if (!FoundDecl->isInIdentifierNamespace(IDNS))
@@ -3063,6 +3075,14 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
         if (isStructuralMatch(D, FoundFunction, false)) {
           if (Decl *Def = FindAndMapDefinition(D, FoundFunction))
             return Def;
+
+          if (NoPrototype && HasNoPrototype(FoundFunction)) {
+            // We want to use the same FoundFunction for every occurrence.
+            // FIXME: In C the same function can be called with different
+            // parameters.
+            return Importer.MapImported(D, FoundFunction);
+          }
+
           FoundByLookup = FoundFunction;
           break;
         }

lib/AST/ASTImporterLookupTable.cpp

@@ -56,6 +56,20 @@ struct Builder : RecursiveASTVisitor<Builder> {
     return true;
   }
 
+  // Functions without previous prototype are not visited by the AST visitor
+  // so they must be added explicitly (like the FriendDecl cases). These are
+  // referenced by the expression that calls them so find these.
+  // It is necessary to visit these to find and link them together at AST
+  // import.
+  bool VisitCallExpr(CallExpr *E) {
+    FunctionDecl *Callee = E->getDirectCallee();
+    // Our case: Declaration that is inside a function and implicit.
+    if (Callee && Callee->getLexicalDeclContext()->isFunctionOrMethod() &&
+        Callee->isImplicit())
+      LT.add(Callee);
+    return true;
+  }
+
   // Override default settings of base.
   bool shouldVisitTemplateInstantiations() const { return true; }
   bool shouldVisitImplicitCode() const { return true; }

unittests/AST/ASTImporterFixtures.cpp

@@ -49,6 +49,15 @@ void checkImportedSourceLocations(const Decl *FromD, const Decl *ToD) {
   // Print debug information.
   const bool Print = false;
 
+  // Skip non-declared C functions: The same FunctionDecl can be linked into
+  // multiple contexts of different functions that have different source
+  // locations but the implicit function has always the same.
+  if (FromD->getLexicalDeclContext()->isFunctionOrMethod() &&
+      FromD->isImplicit()) {
+    // FIXME: Should check FromD->getLocation() "==" ToD->getLocation().
+    return;
+  }
+
   SmallString<1024> ToPrinted;
   SmallString<1024> FromPrinted;
   llvm::raw_svector_ostream ToStream(ToPrinted);

unittests/AST/ASTImporterTest.cpp

@@ -5171,6 +5171,102 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportOfFriendTemplateWithArgExpr) {
   EXPECT_NE(ToD1->getCanonicalDecl(), ToD2->getCanonicalDecl());
 }
 
+struct ASTImporterCImplicitFunctionTest : ASTImporterOptionSpecificTestBase {
+  static FunctionDecl *GetImplicitF(FunctionDecl *F) {
+    return cast<CallExpr>(cast<CompoundStmt>(F->getBody())->body_front())
+        ->getDirectCallee();
+  };
+};
+
+TEST_P(ASTImporterCImplicitFunctionTest, ImportCImplicitFunction) {
+  auto Code = "void f() { implicit_f(); }";
+
+  TranslationUnitDecl *FromTU = getTuDecl(Code, Lang_C, "input.c");
+  auto *FromF = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("f")));
+  FunctionDecl *FromImplicitF = GetImplicitF(FromF);
+
+  ASSERT_TRUE(FromImplicitF->getDeclContext() == cast<DeclContext>(FromTU));
+  ASSERT_TRUE(FromImplicitF->getLexicalDeclContext() ==
+              cast<DeclContext>(FromF));
+  ASSERT_FALSE(FromTU->containsDecl(FromImplicitF));
+  ASSERT_TRUE(FromF->containsDecl(FromImplicitF));
+
+  auto *ToImplicitF = Import(FromImplicitF, Lang_C);
+  auto *ToF = Import(FromF, Lang_C);
+  ASSERT_TRUE(ToImplicitF);
+  TranslationUnitDecl *ToTU = ToImplicitF->getTranslationUnitDecl();
+  EXPECT_FALSE(ToTU->containsDecl(ToImplicitF));
+  EXPECT_TRUE(ToF->containsDecl(ToImplicitF));
+}
+
+TEST_P(ASTImporterCImplicitFunctionTest, ImportedCImplicitFunctionsLinked) {
+  auto Code =
+      R"(
+        void f1() { implicit_f(); }
+        void f2() { implicit_f(); }
+        )";
+
+  TranslationUnitDecl *FromTU = getTuDecl(Code, Lang_C, "input.c");
+  auto *FromF1 = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("f1")));
+  FunctionDecl *FromImplicitF1 = GetImplicitF(FromF1);
+  auto *FromF2 = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("f2")));
+  FunctionDecl *FromImplicitF2 = GetImplicitF(FromF2);
+
+  ASSERT_EQ(FromImplicitF1, FromImplicitF2);
+
+  auto *ToImplicitF1 = Import(FromImplicitF1, Lang_C);
+  auto *ToF1 = Import(FromF1, Lang_C);
+  auto *ToF2 = Import(FromF2, Lang_C);
+  ASSERT_TRUE(ToImplicitF1);
+  EXPECT_EQ(GetImplicitF(ToF1), ToImplicitF1);
+  EXPECT_EQ(GetImplicitF(ToF2), ToImplicitF1);
+}
+
+TEST_P(ASTImporterCImplicitFunctionTest,
+       ImportAndLinkCImplicitFunctionAfterExisting) {
+  auto ToCode = "void f1() { implicit_f(); }";
+  auto FromCode = "void f2() { implicit_f(); }";
+
+  TranslationUnitDecl *ToTU = getToTuDecl(ToCode, Lang_C);
+  TranslationUnitDecl *FromTU = getTuDecl(FromCode, Lang_C);
+  auto *ToF1 =
+      FirstDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f1")));
+  FunctionDecl *ToImplicitF1 = GetImplicitF(ToF1);
+  auto *FromF2 = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("f2")));
+  FunctionDecl *FromImplicitF2 = GetImplicitF(FromF2);
+
+  auto *ToImplicitF2 = Import(FromImplicitF2, Lang_C);
+  ASSERT_TRUE(ToImplicitF2);
+  EXPECT_EQ(ToImplicitF2, ToImplicitF1);
+}
+
+TEST_P(ASTImporterCImplicitFunctionTest,
+       ImportAndLinkCImplicitFunctionDefinition) {
+  auto ToCode = "void f() { implicit_f(); }";
+  auto FromCode = "int implicit_f() {}";
+
+  TranslationUnitDecl *ToTU = getToTuDecl(ToCode, Lang_C);
+  TranslationUnitDecl *FromTU = getTuDecl(FromCode, Lang_C);
+  auto *ToF =
+      FirstDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f")));
+  FunctionDecl *ToImplicitF = GetImplicitF(ToF);
+  auto *FromImplicitFDef = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("implicit_f")));
+
+  auto *ToImplicitFDef = Import(FromImplicitFDef, Lang_C);
+  ASSERT_TRUE(ToImplicitFDef);
+  EXPECT_NE(ToImplicitFDef, ToImplicitF);
+  EXPECT_EQ(ToImplicitFDef->getPreviousDecl(), ToImplicitF);
+  EXPECT_EQ(ToImplicitF->getDefinition(), ToImplicitFDef);
+}
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterCImplicitFunctionTest,
+                        DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
                         DefaultTestValuesForRunOptions, );