]> granicus.if.org Git - clang/commitdiff
[ASTImporter] Fix import of VarDecl init
authorGabor Marton <martongabesz@gmail.com>
Mon, 17 Sep 2018 12:04:52 +0000 (12:04 +0000)
committerGabor Marton <martongabesz@gmail.com>
Mon, 17 Sep 2018 12:04:52 +0000 (12:04 +0000)
Summary:
The init expression of a VarDecl is overwritten in the "To" context if we
import a VarDecl without an init expression (and with a definition).  Please
refer to the added tests, especially InitAndDefinitionAreInDifferentTUs.  This
patch fixes the malfunction by importing the whole Decl chain similarly as we
did that in case of FunctionDecls.  We handle the init expression similarly to
a  definition, alas only one init expression will be in the merged ast.

Reviewers: a_sidorin, xazax.hun, r.stahl, a.sidorin

Subscribers: rnkovacs, dkrupp, cfe-commits

Differential Revision: https://reviews.llvm.org/D51597

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@342384 91177308-0d34-0410-b5e6-96231b3b80d8

lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp

index fa8b3c739a2d02ea9a83d7a64de2c3681c81524d..062284ccb6f9ef348ffa79c3d7e5ebeb3cd8741c 100644 (file)
@@ -107,9 +107,11 @@ namespace clang {
   }
 
   SmallVector<Decl*, 2> getCanonicalForwardRedeclChain(Decl* D) {
-    // Currently only FunctionDecl is supported
-    auto FD = cast<FunctionDecl>(D);
-    return getCanonicalForwardRedeclChain<FunctionDecl>(FD);
+    if (auto *FD = dyn_cast<FunctionDecl>(D))
+      return getCanonicalForwardRedeclChain<FunctionDecl>(FD);
+    if (auto *VD = dyn_cast<VarDecl>(D))
+      return getCanonicalForwardRedeclChain<VarDecl>(VD);
+    llvm_unreachable("Bad declaration kind");
   }
 
   void updateFlags(const Decl *From, Decl *To) {
@@ -280,10 +282,9 @@ namespace clang {
              (IDK == IDK_Default && !Importer.isMinimalImport());
     }
 
+    bool ImportInitializer(VarDecl *From, VarDecl *To);
     bool ImportDefinition(RecordDecl *From, RecordDecl *To,
                           ImportDefinitionKind Kind = IDK_Default);
-    bool ImportDefinition(VarDecl *From, VarDecl *To,
-                          ImportDefinitionKind Kind = IDK_Default);
     bool ImportDefinition(EnumDecl *From, EnumDecl *To,
                           ImportDefinitionKind Kind = IDK_Default);
     bool ImportDefinition(ObjCInterfaceDecl *From, ObjCInterfaceDecl *To,
@@ -1424,18 +1425,26 @@ bool ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To,
   return false;
 }
 
-bool ASTNodeImporter::ImportDefinition(VarDecl *From, VarDecl *To,
-                                       ImportDefinitionKind Kind) {
+bool ASTNodeImporter::ImportInitializer(VarDecl *From, VarDecl *To) {
   if (To->getAnyInitializer())
     return false;
 
-  // FIXME: Can we really import any initializer? Alternatively, we could force
-  // ourselves to import every declaration of a variable and then only use
-  // getInit() here.
-  To->setInit(Importer.Import(const_cast<Expr *>(From->getAnyInitializer())));
+  Expr *FromInit = From->getInit();
+  if (!FromInit)
+    return false;
 
-  // FIXME: Other bits to merge?
+  Expr *ToInit = Importer.Import(const_cast<Expr *>(FromInit));
+  if (!ToInit)
+    return true;
 
+  To->setInit(ToInit);
+  if (From->isInitKnownICE()) {
+    EvaluatedStmt *Eval = To->ensureEvaluatedStmt();
+    Eval->CheckedICE = true;
+    Eval->IsICE = From->isInitICE();
+  }
+
+  // FIXME: Other bits to merge?
   return false;
 }
 
@@ -3138,6 +3147,16 @@ Decl *ASTNodeImporter::VisitObjCIvarDecl(ObjCIvarDecl *D) {
 }
 
 Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
+
+  SmallVector<Decl*, 2> Redecls = getCanonicalForwardRedeclChain(D);
+  auto RedeclIt = Redecls.begin();
+  // Import the first part of the decl chain. I.e. import all previous
+  // declarations starting from the canonical decl.
+  for (; RedeclIt != Redecls.end() && *RedeclIt != D; ++RedeclIt)
+    if (!Importer.Import(*RedeclIt))
+      return nullptr;
+  assert(*RedeclIt == D);
+
   // Import the major distinguishing characteristics of a variable.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
@@ -3150,8 +3169,8 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
 
   // Try to find a variable in our own ("to") context with the same name and
   // in the same context as the variable we're importing.
+  VarDecl *FoundByLookup = nullptr;
   if (D->isFileVarDecl()) {
-    VarDecl *MergeWithVar = nullptr;
     SmallVector<NamedDecl *, 4> ConflictingDecls;
     unsigned IDNS = Decl::IDNS_Ordinary;
     SmallVector<NamedDecl *, 2> FoundDecls;
@@ -3166,7 +3185,23 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
             D->hasExternalFormalLinkage()) {
           if (Importer.IsStructurallyEquivalent(D->getType(),
                                                 FoundVar->getType())) {
-            MergeWithVar = FoundVar;
+
+            // The VarDecl in the "From" context has a definition, but in the
+            // "To" context we already have a definition.
+            VarDecl *FoundDef = FoundVar->getDefinition();
+            if (D->isThisDeclarationADefinition() && FoundDef)
+              // FIXME Check for ODR error if the two definitions have
+              // different initializers?
+              return Importer.MapImported(D, FoundDef);
+
+            // The VarDecl in the "From" context has an initializer, but in the
+            // "To" context we already have an initializer.
+            const VarDecl *FoundDInit = nullptr;
+            if (D->getInit() && FoundVar->getAnyInitializer(FoundDInit))
+              // FIXME Diagnose ODR error if the two initializers are different?
+              return Importer.MapImported(D, const_cast<VarDecl*>(FoundDInit));
+
+            FoundByLookup = FoundVar;
             break;
           }
 
@@ -3183,11 +3218,11 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
                 return nullptr;
 
               FoundVar->setType(T);
-              MergeWithVar = FoundVar;
+              FoundByLookup = FoundVar;
               break;
             } else if (isa<IncompleteArrayType>(TArray) &&
                        isa<ConstantArrayType>(FoundArray)) {
-              MergeWithVar = FoundVar;
+              FoundByLookup = FoundVar;
               break;
             }
           }
@@ -3202,32 +3237,6 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
       ConflictingDecls.push_back(FoundDecl);
     }
 
-    if (MergeWithVar) {
-      // An equivalent variable with external linkage has been found. Link
-      // the two declarations, then merge them.
-      Importer.MapImported(D, MergeWithVar);
-      updateFlags(D, MergeWithVar);
-
-      if (VarDecl *DDef = D->getDefinition()) {
-        if (VarDecl *ExistingDef = MergeWithVar->getDefinition()) {
-          Importer.ToDiag(ExistingDef->getLocation(),
-                          diag::err_odr_variable_multiple_def)
-            << Name;
-          Importer.FromDiag(DDef->getLocation(), diag::note_odr_defined_here);
-        } else {
-          Expr *Init = Importer.Import(DDef->getInit());
-          MergeWithVar->setInit(Init);
-          if (DDef->isInitKnownICE()) {
-            EvaluatedStmt *Eval = MergeWithVar->ensureEvaluatedStmt();
-            Eval->CheckedICE = true;
-            Eval->IsICE = DDef->isInitICE();
-          }
-        }
-      }
-
-      return MergeWithVar;
-    }
-
     if (!ConflictingDecls.empty()) {
       Name = Importer.HandleNameConflict(Name, DC, IDNS,
                                          ConflictingDecls.data(),
@@ -3255,17 +3264,27 @@ Decl *ASTNodeImporter::VisitVarDecl(VarDecl *D) {
   ToVar->setAccess(D->getAccess());
   ToVar->setLexicalDeclContext(LexicalDC);
 
-  // Templated declarations should never appear in the enclosing DeclContext.
-  if (!D->getDescribedVarTemplate())
-    LexicalDC->addDeclInternal(ToVar);
+  if (FoundByLookup) {
+    auto *Recent = const_cast<VarDecl *>(FoundByLookup->getMostRecentDecl());
+    ToVar->setPreviousDecl(Recent);
+  }
 
-  // Merge the initializer.
-  if (ImportDefinition(D, ToVar))
+  if (ImportInitializer(D, ToVar))
     return nullptr;
 
   if (D->isConstexpr())
     ToVar->setConstexpr(true);
 
+  if (D->getDeclContext()->containsDeclAndLoad(D))
+    DC->addDeclInternal(ToVar);
+  if (DC != LexicalDC && D->getLexicalDeclContext()->containsDeclAndLoad(D))
+    LexicalDC->addDeclInternal(ToVar);
+
+  // Import the rest of the chain. I.e. import all subsequent declarations.
+  for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt)
+    if (!Importer.Import(*RedeclIt))
+      return nullptr;
+
   return ToVar;
 }
 
@@ -4914,12 +4933,7 @@ Decl *ASTNodeImporter::VisitVarTemplateSpecializationDecl(
     D2->setAccess(D->getAccess());
   }
 
-  // NOTE: isThisDeclarationADefinition() can return DeclarationOnly even if
-  // declaration has initializer. Should this be fixed in the AST?.. Anyway,
-  // we have to check the declaration for initializer - otherwise, it won't be
-  // imported.
-  if ((D->isThisDeclarationADefinition() || D->hasInit()) &&
-      ImportDefinition(D, D2))
+  if (ImportInitializer(D, D2))
     return nullptr;
 
   return D2;
@@ -7084,6 +7098,7 @@ Decl *ASTImporter::Import(Decl *FromD) {
   // Notify subclasses.
   Imported(FromD, ToD);
 
+  updateFlags(FromD, ToD);
   return ToD;
 }
 
index 96a8d43cdf1ce64c3dafb1be9d4fdbddd669cfd6..366ff7f0e08a5f93a7e36b1a029246d0a9a11c87 100644 (file)
@@ -1828,13 +1828,62 @@ TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag) {
   {
     Decl *FromTU =
         getTuDecl("extern int x; int f() { return x; }", Lang_CXX, "input2.cc");
-    auto *FromD =
-        FirstDeclMatcher<FunctionDecl>().match(FromTU, functionDecl());
+    auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
+        FromTU, functionDecl(hasName("f")));
     Import(FromD, Lang_CXX);
   }
   EXPECT_TRUE(Imported2->isUsed(false));
 }
 
+TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag2) {
+  auto Pattern = varDecl(hasName("x"));
+  VarDecl *ExistingD;
+  {
+    Decl *ToTU = getToTuDecl("int x = 1;", Lang_CXX);
+    ExistingD = FirstDeclMatcher<VarDecl>().match(ToTU, Pattern);
+  }
+  EXPECT_FALSE(ExistingD->isUsed(false));
+  {
+    Decl *FromTU = getTuDecl(
+        "int x = 1; int f() { return x; }", Lang_CXX, "input1.cc");
+    auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
+        FromTU, functionDecl(hasName("f")));
+    Import(FromD, Lang_CXX);
+  }
+  EXPECT_TRUE(ExistingD->isUsed(false));
+}
+
+TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag3) {
+  auto Pattern = varDecl(hasName("a"));
+  VarDecl *ExistingD;
+  {
+    Decl *ToTU = getToTuDecl(
+        R"(
+        struct A {
+          static const int a = 1;
+        };
+        )", Lang_CXX);
+    ExistingD = FirstDeclMatcher<VarDecl>().match(ToTU, Pattern);
+  }
+  EXPECT_FALSE(ExistingD->isUsed(false));
+  {
+    Decl *FromTU = getTuDecl(
+        R"(
+        struct A {
+          static const int a = 1;
+        };
+        const int *f() { return &A::a; } // requires storage,
+                                         // thus used flag will be set
+        )", Lang_CXX, "input1.cc");
+    auto *FromFunD = FirstDeclMatcher<FunctionDecl>().match(
+        FromTU, functionDecl(hasName("f")));
+    auto *FromD = FirstDeclMatcher<VarDecl>().match(FromTU, Pattern);
+    ASSERT_TRUE(FromD->isUsed(false));
+    Import(FromFunD, Lang_CXX);
+  }
+  EXPECT_TRUE(ExistingD->isUsed(false));
+}
+
 TEST_P(ASTImporterTestBase, ReimportWithUsedFlag) {
   auto Pattern = varDecl(hasName("x"));
 
@@ -3244,6 +3293,94 @@ TEST_P(ASTImporterTestBase, InitListExprValueKindShouldBeImported) {
   EXPECT_TRUE(ToInitExpr->isGLValue());
 }
 
+struct ImportVariables : ASTImporterTestBase {};
+
+TEST_P(ImportVariables, ImportOfOneDeclBringsInTheWholeChain) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      struct A {
+        static const int a = 1 + 2;
+      };
+      const int A::a;
+      )", Lang_CXX, "input1.cc");
+
+  auto *FromDWithInit = FirstDeclMatcher<VarDecl>().match(
+      FromTU, varDecl(hasName("a"))); // Decl with init
+  auto *FromDWithDef = LastDeclMatcher<VarDecl>().match(
+      FromTU, varDecl(hasName("a"))); // Decl with definition
+  ASSERT_NE(FromDWithInit, FromDWithDef);
+  ASSERT_EQ(FromDWithDef->getPreviousDecl(), FromDWithInit);
+
+  auto *ToD0 = cast<VarDecl>(Import(FromDWithInit, Lang_CXX11));
+  auto *ToD1 = cast<VarDecl>(Import(FromDWithDef, Lang_CXX11));
+  ASSERT_TRUE(ToD0);
+  ASSERT_TRUE(ToD1);
+  EXPECT_NE(ToD0, ToD1);
+  EXPECT_EQ(ToD1->getPreviousDecl(), ToD0);
+}
+
+TEST_P(ImportVariables, InitAndDefinitionAreInDifferentTUs) {
+  auto StructA =
+      R"(
+      struct A {
+        static const int a = 1 + 2;
+      };
+      )";
+  Decl *ToTU = getToTuDecl(StructA, Lang_CXX);
+  Decl *FromTU = getTuDecl(std::string(StructA) + "const int A::a;", Lang_CXX,
+                           "input1.cc");
+
+  auto *FromDWithInit = FirstDeclMatcher<VarDecl>().match(
+      FromTU, varDecl(hasName("a"))); // Decl with init
+  auto *FromDWithDef = LastDeclMatcher<VarDecl>().match(
+      FromTU, varDecl(hasName("a"))); // Decl with definition
+  ASSERT_EQ(FromDWithInit, FromDWithDef->getPreviousDecl());
+  ASSERT_TRUE(FromDWithInit->getInit());
+  ASSERT_FALSE(FromDWithInit->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition());
+  ASSERT_FALSE(FromDWithDef->getInit());
+
+  auto *ToD = FirstDeclMatcher<VarDecl>().match(
+      ToTU, varDecl(hasName("a"))); // Decl with init
+  ASSERT_TRUE(ToD->getInit());
+  ASSERT_FALSE(ToD->getDefinition());
+
+  auto *ImportedD = cast<VarDecl>(Import(FromDWithDef, Lang_CXX11));
+  EXPECT_TRUE(ImportedD->getAnyInitializer());
+  EXPECT_TRUE(ImportedD->getDefinition());
+}
+
+TEST_P(ImportVariables, InitAndDefinitionAreInTheFromContext) {
+  auto StructA =
+      R"(
+      struct A {
+        static const int a;
+      };
+      )";
+  Decl *ToTU = getToTuDecl(StructA, Lang_CXX);
+  Decl *FromTU = getTuDecl(std::string(StructA) + "const int A::a = 1 + 2;",
+                           Lang_CXX, "input1.cc");
+
+  auto *FromDDeclarationOnly = FirstDeclMatcher<VarDecl>().match(
+      FromTU, varDecl(hasName("a")));
+  auto *FromDWithDef = LastDeclMatcher<VarDecl>().match(
+      FromTU, varDecl(hasName("a"))); // Decl with definition and with init.
+  ASSERT_EQ(FromDDeclarationOnly, FromDWithDef->getPreviousDecl());
+  ASSERT_FALSE(FromDDeclarationOnly->getInit());
+  ASSERT_FALSE(FromDDeclarationOnly->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromDWithDef->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromDWithDef->getInit());
+
+  auto *ToD = FirstDeclMatcher<VarDecl>().match(
+      ToTU, varDecl(hasName("a")));
+  ASSERT_FALSE(ToD->getInit());
+  ASSERT_FALSE(ToD->getDefinition());
+
+  auto *ImportedD = cast<VarDecl>(Import(FromDWithDef, Lang_CXX11));
+  EXPECT_TRUE(ImportedD->getAnyInitializer());
+  EXPECT_TRUE(ImportedD->getDefinition());
+}
+
 struct DeclContextTest : ASTImporterTestBase {};
 
 TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
@@ -3623,5 +3760,11 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTests,
                         ImportFunctionTemplateSpecializations,
                         DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportImplicitMethods,
+                        DefaultTestValuesForRunOptions, );
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables,
+                        DefaultTestValuesForRunOptions, );
+
 } // end namespace ast_matchers
 } // end namespace clang