r356455 - [ASTImporter] Fix redecl failures of FunctionTemplateSpec

Gabor Marton via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 19 07:04:50 PDT 2019


Author: martong
Date: Tue Mar 19 07:04:50 2019
New Revision: 356455

URL: http://llvm.org/viewvc/llvm-project?rev=356455&view=rev
Log:
[ASTImporter] Fix redecl failures of FunctionTemplateSpec

Summary:
Redecl chains of function template specializations are not handled well
currently. We want to handle them similarly to functions, i.e. try to
keep the structure of the original AST as much as possible. The aim is
to not squash a prototype with a definition, rather we create both and
put them in a redecl chain.

Reviewers: a_sidorin, shafik, a.sidorin

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
    cfe/trunk/lib/AST/ASTImporter.cpp
    cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=356455&r1=356454&r2=356455&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Mar 19 07:04:50 2019
@@ -290,6 +290,16 @@ namespace clang {
         ToD->setImplicit();
     }
 
+    // Check if we have found an existing definition.  Returns with that
+    // definition if yes, otherwise returns null.
+    Decl *FindAndMapDefinition(FunctionDecl *D, FunctionDecl *FoundFunction) {
+      const FunctionDecl *Definition = nullptr;
+      if (D->doesThisDeclarationHaveABody() &&
+          FoundFunction->hasBody(Definition))
+        return Importer.MapImported(D, const_cast<FunctionDecl *>(Definition));
+      return nullptr;
+    }
+
   public:
     explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
 
@@ -2973,8 +2983,8 @@ ExpectedDecl ASTNodeImporter::VisitFunct
     if (!FoundFunctionOrErr)
       return FoundFunctionOrErr.takeError();
     if (FunctionDecl *FoundFunction = *FoundFunctionOrErr) {
-      if (D->doesThisDeclarationHaveABody() && FoundFunction->hasBody())
-        return Importer.MapImported(D, FoundFunction);
+      if (Decl *Def = FindAndMapDefinition(D, FoundFunction))
+        return Def;
       FoundByLookup = FoundFunction;
     }
   }
@@ -2993,11 +3003,8 @@ ExpectedDecl ASTNodeImporter::VisitFunct
           continue;
 
         if (IsStructuralMatch(D, FoundFunction)) {
-          const FunctionDecl *Definition = nullptr;
-          if (D->doesThisDeclarationHaveABody() &&
-              FoundFunction->hasBody(Definition))
-            return Importer.MapImported(D,
-                                        const_cast<FunctionDecl *>(Definition));
+          if (Decl *Def = FindAndMapDefinition(D, FoundFunction))
+            return Def;
           FoundByLookup = FoundFunction;
           break;
         }

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=356455&r1=356454&r2=356455&view=diff
==============================================================================
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue Mar 19 07:04:50 2019
@@ -3978,6 +3978,45 @@ struct RedeclChain : ASTImporterOptionSp
   std::string getDefinition() { return TypeParam::Definition; }
   BindableMatcher<Decl> getPattern() const { return TypeParam().getPattern(); }
 
+  void CheckPreviousDecl(Decl *Prev, Decl *Current) {
+    ASSERT_NE(Prev, Current);
+    ASSERT_EQ(&Prev->getASTContext(), &Current->getASTContext());
+    EXPECT_EQ(Prev->getCanonicalDecl(), Current->getCanonicalDecl());
+
+    // Templates.
+    if (auto *PrevT = dyn_cast<TemplateDecl>(Prev)) {
+      EXPECT_EQ(Current->getPreviousDecl(), Prev);
+      auto *CurrentT = cast<TemplateDecl>(Current);
+      ASSERT_TRUE(PrevT->getTemplatedDecl());
+      ASSERT_TRUE(CurrentT->getTemplatedDecl());
+      EXPECT_EQ(CurrentT->getTemplatedDecl()->getPreviousDecl(),
+                PrevT->getTemplatedDecl());
+      return;
+    }
+
+    // Specializations.
+    if (auto *PrevF = dyn_cast<FunctionDecl>(Prev)) {
+      if (PrevF->getTemplatedKind() ==
+          FunctionDecl::TK_FunctionTemplateSpecialization) {
+        // There may be a hidden fwd spec decl before a spec decl.
+        // In that case the previous visible decl can be reached through that
+        // invisible one.
+        EXPECT_THAT(Prev, testing::AnyOf(
+                              Current->getPreviousDecl(),
+                              Current->getPreviousDecl()->getPreviousDecl()));
+        auto *ToTU = Prev->getTranslationUnitDecl();
+        auto *TemplateD = FirstDeclMatcher<FunctionTemplateDecl>().match(
+            ToTU, functionTemplateDecl());
+        auto *FirstSpecD = *(TemplateD->spec_begin());
+        EXPECT_EQ(FirstSpecD->getCanonicalDecl(), PrevF->getCanonicalDecl());
+        return;
+      }
+    }
+
+    // The rest: Classes, Functions, etc.
+    EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  }
+
   void
   TypedTest_PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition() {
     Decl *FromTU = getTuDecl(getPrototype(), Lang_CXX);
@@ -4030,14 +4069,8 @@ struct RedeclChain : ASTImporterOptionSp
     EXPECT_TRUE(Imported1 == To1);
     EXPECT_FALSE(To0->isThisDeclarationADefinition());
     EXPECT_FALSE(To1->isThisDeclarationADefinition());
-    EXPECT_EQ(To1->getPreviousDecl(), To0);
-    if (auto *ToT0 = dyn_cast<TemplateDecl>(To0)) {
-      auto *ToT1 = cast<TemplateDecl>(To1);
-      ASSERT_TRUE(ToT0->getTemplatedDecl());
-      ASSERT_TRUE(ToT1->getTemplatedDecl());
-      EXPECT_EQ(ToT1->getTemplatedDecl()->getPreviousDecl(),
-                ToT0->getTemplatedDecl());
-    }
+
+    CheckPreviousDecl(To0, To1);
   }
 
   void TypedTest_ImportDefinitionAfterImportedPrototype() {
@@ -4059,14 +4092,8 @@ struct RedeclChain : ASTImporterOptionSp
     EXPECT_TRUE(ImportedDef == ToDef);
     EXPECT_FALSE(ToProto->isThisDeclarationADefinition());
     EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
-    EXPECT_EQ(ToDef->getPreviousDecl(), ToProto);
-    if (auto *ToProtoT = dyn_cast<TemplateDecl>(ToProto)) {
-      auto *ToDefT = cast<TemplateDecl>(ToDef);
-      ASSERT_TRUE(ToProtoT->getTemplatedDecl());
-      ASSERT_TRUE(ToDefT->getTemplatedDecl());
-      EXPECT_EQ(ToDefT->getTemplatedDecl()->getPreviousDecl(),
-                ToProtoT->getTemplatedDecl());
-    }
+
+    CheckPreviousDecl(ToProto, ToDef);
   }
 
   void TypedTest_ImportPrototypeAfterImportedDefinition() {
@@ -4088,14 +4115,8 @@ struct RedeclChain : ASTImporterOptionSp
     EXPECT_TRUE(ImportedProto == ToProto);
     EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
     EXPECT_FALSE(ToProto->isThisDeclarationADefinition());
-    EXPECT_EQ(ToProto->getPreviousDecl(), ToDef);
-    if (auto *ToDefT = dyn_cast<TemplateDecl>(ToDef)) {
-      auto *ToProtoT = cast<TemplateDecl>(ToProto);
-      ASSERT_TRUE(ToDefT->getTemplatedDecl());
-      ASSERT_TRUE(ToProtoT->getTemplatedDecl());
-      EXPECT_EQ(ToProtoT->getTemplatedDecl()->getPreviousDecl(),
-                ToDefT->getTemplatedDecl());
-    }
+
+    CheckPreviousDecl(ToDef, ToProto);
   }
 
   void TypedTest_ImportPrototypes() {
@@ -4117,27 +4138,8 @@ struct RedeclChain : ASTImporterOptionSp
     EXPECT_TRUE(Imported1 == To1);
     EXPECT_FALSE(To0->isThisDeclarationADefinition());
     EXPECT_FALSE(To1->isThisDeclarationADefinition());
-    EXPECT_EQ(To1->getPreviousDecl(), To0);
-    if (auto *ToT0 = dyn_cast<TemplateDecl>(To0)) {
-      auto *ToT1 = cast<TemplateDecl>(To1);
-      ASSERT_TRUE(ToT0->getTemplatedDecl());
-      ASSERT_TRUE(ToT1->getTemplatedDecl());
-      EXPECT_EQ(ToT1->getTemplatedDecl()->getPreviousDecl(),
-                ToT0->getTemplatedDecl());
-    }
-    // Extra check for specializations.
-    // FIXME Add this check to other tests too (possibly factor out into a
-    // function), when they start to pass.
-    if (auto *From0F = dyn_cast<FunctionDecl>(From0)) {
-      auto *To0F = cast<FunctionDecl>(To0);
-      if (From0F->getTemplatedKind() ==
-          FunctionDecl::TK_FunctionTemplateSpecialization) {
-        auto *TemplateD = FirstDeclMatcher<FunctionTemplateDecl>().match(
-            ToTU, functionTemplateDecl());
-        auto *FirstSpecD = *(TemplateD->spec_begin());
-        EXPECT_EQ(FirstSpecD->getCanonicalDecl(), To0F->getCanonicalDecl());
-      }
-    }
+
+    CheckPreviousDecl(To0, To1);
   }
 
   void TypedTest_ImportDefinitions() {
@@ -4182,14 +4184,8 @@ struct RedeclChain : ASTImporterOptionSp
     EXPECT_TRUE(ImportedProto == ToProto);
     EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
     EXPECT_FALSE(ToProto->isThisDeclarationADefinition());
-    EXPECT_EQ(ToProto->getPreviousDecl(), ToDef);
-    if (auto *ToDefT = dyn_cast<TemplateDecl>(ToDef)) {
-      auto *ToProtoT = cast<TemplateDecl>(ToProto);
-      ASSERT_TRUE(ToDefT->getTemplatedDecl());
-      ASSERT_TRUE(ToProtoT->getTemplatedDecl());
-      EXPECT_EQ(ToProtoT->getTemplatedDecl()->getPreviousDecl(),
-                ToDefT->getTemplatedDecl());
-    }
+
+    CheckPreviousDecl(ToDef, ToProto);
   }
 
   void TypedTest_ImportPrototypeThenDefinition() {
@@ -4213,14 +4209,8 @@ struct RedeclChain : ASTImporterOptionSp
     EXPECT_TRUE(ImportedProto == ToProto);
     EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
     EXPECT_FALSE(ToProto->isThisDeclarationADefinition());
-    EXPECT_EQ(ToDef->getPreviousDecl(), ToProto);
-    if (auto *ToDefT = dyn_cast<TemplateDecl>(ToDef)) {
-      auto *ToProtoT = cast<TemplateDecl>(ToProto);
-      ASSERT_TRUE(ToDefT->getTemplatedDecl());
-      ASSERT_TRUE(ToProtoT->getTemplatedDecl());
-      EXPECT_EQ(ToDefT->getTemplatedDecl()->getPreviousDecl(),
-                ToProtoT->getTemplatedDecl());
-    }
+
+    CheckPreviousDecl(ToProto, ToDef);
   }
 
   void TypedTest_WholeRedeclChainIsImportedAtOnce() {
@@ -4262,7 +4252,8 @@ struct RedeclChain : ASTImporterOptionSp
     EXPECT_TRUE(DefinitionD->getPreviousDecl());
     EXPECT_FALSE(
         DefinitionD->getPreviousDecl()->isThisDeclarationADefinition());
-    EXPECT_EQ(DefinitionD->getPreviousDecl()->getPreviousDecl(), ProtoD);
+
+    CheckPreviousDecl(ProtoD, DefinitionD->getPreviousDecl());
   }
 };
 
@@ -4366,11 +4357,10 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
                                         ImportPrototypes)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
                                         ImportPrototypes)
-// FIXME This does not pass, possible error with Spec import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec,
-                                        DISABLED_, ImportPrototypes)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
                                         ImportPrototypes)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
+                                        ImportPrototypes)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
                                         ImportDefinitions)
@@ -4382,11 +4372,10 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
                                         ImportDefinitions)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
                                         ImportDefinitions)
-// FIXME This does not pass, possible error with Spec import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec,
-                                        DISABLED_, ImportDefinitions)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
                                         ImportDefinitions)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
+                                        ImportDefinitions)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
                                         ImportDefinitionThenPrototype)
@@ -4398,9 +4387,7 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
                                         ImportDefinitionThenPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
                                         ImportDefinitionThenPrototype)
-// FIXME This does not pass, possible error with Spec import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec,
-                                        DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
                                         ImportDefinitionThenPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
                                         ImportDefinitionThenPrototype)
@@ -4415,9 +4402,7 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
                                         ImportPrototypeThenDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
                                         ImportPrototypeThenDefinition)
-// FIXME This does not pass, possible error with Spec import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec,
-                                        DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
                                         ImportPrototypeThenDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
                                         ImportPrototypeThenDefinition)
@@ -4437,9 +4422,7 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
                                         ImportPrototypeThenProtoAndDefinition)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate, ,
                                         ImportPrototypeThenProtoAndDefinition)
-// FIXME This does not pass, possible error with Spec import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec,
-                                        DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
                                         ImportPrototypeThenProtoAndDefinition)
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, RedeclChainFunction,




More information about the cfe-commits mailing list