martong created this revision.
martong added reviewers: a_sidorin, shafik.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
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.
Repository:
rC Clang
https://reviews.llvm.org/D58668
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -3859,6 +3859,47 @@
std::string getDefinition() { return TypeParam::Definition; }
BindableMatcher<Decl> getPattern() const { return TypeParam().getPattern(); }
+ void CheckPreviousDecl(Decl *To0, Decl *To1) {
+ ASSERT_NE(To0, To1);
+ ASSERT_EQ(&To0->getASTContext(), &To1->getASTContext());
+
+ auto *ToTU = To0->getTranslationUnitDecl();
+
+ // Templates.
+ if (auto *ToT0 = dyn_cast<TemplateDecl>(To0)) {
+ EXPECT_EQ(To1->getPreviousDecl(), To0);
+ auto *ToT1 = cast<TemplateDecl>(To1);
+ ASSERT_TRUE(ToT0->getTemplatedDecl());
+ ASSERT_TRUE(ToT1->getTemplatedDecl());
+ EXPECT_EQ(ToT1->getTemplatedDecl()->getPreviousDecl(),
+ ToT0->getTemplatedDecl());
+ return;
+ }
+
+ // Specializations.
+ if (auto *From0F = dyn_cast<FunctionDecl>(To0)) {
+ auto *To0F = cast<FunctionDecl>(To0);
+ if (From0F->getTemplatedKind() ==
+ FunctionDecl::TK_FunctionTemplateSpecialization) {
+ EXPECT_EQ(To0->getCanonicalDecl(), To1->getCanonicalDecl());
+ // 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(To0,
+ testing::AnyOf(To1->getPreviousDecl(),
+ To1->getPreviousDecl()->getPreviousDecl()));
+ auto *TemplateD = FirstDeclMatcher<FunctionTemplateDecl>().match(
+ ToTU, functionTemplateDecl());
+ auto *FirstSpecD = *(TemplateD->spec_begin());
+ EXPECT_EQ(FirstSpecD->getCanonicalDecl(), To0F->getCanonicalDecl());
+ return;
+ }
+ }
+
+ // The rest: Classes, Functions, etc.
+ EXPECT_EQ(To1->getPreviousDecl(), To0);
+ }
+
void
TypedTest_PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition() {
Decl *FromTU = getTuDecl(getPrototype(), Lang_CXX);
@@ -3911,14 +3952,8 @@
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() {
@@ -3940,14 +3975,8 @@
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() {
@@ -3969,14 +3998,8 @@
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() {
@@ -3998,27 +4021,8 @@
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() {
@@ -4063,14 +4067,8 @@
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() {
@@ -4094,14 +4092,8 @@
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() {
@@ -4143,7 +4135,8 @@
EXPECT_TRUE(DefinitionD->getPreviousDecl());
EXPECT_FALSE(
DefinitionD->getPreviousDecl()->isThisDeclarationADefinition());
- EXPECT_EQ(DefinitionD->getPreviousDecl()->getPreviousDecl(), ProtoD);
+
+ CheckPreviousDecl(ProtoD, DefinitionD->getPreviousDecl());
}
};
@@ -4249,9 +4242,8 @@
DISABLED_, 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, FunctionTemplateSpec, ,
+ ImportPrototypes)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
ImportDefinitions)
@@ -4264,9 +4256,8 @@
DISABLED_, 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, FunctionTemplateSpec, ,
+ ImportDefinitions)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
ImportDefinitionThenPrototype)
@@ -4280,9 +4271,7 @@
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, Function, ,
@@ -4297,9 +4286,7 @@
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, Function, ,
@@ -4321,9 +4308,7 @@
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
DISABLED_,
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,
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2999,6 +2999,16 @@
FunctionDecl *FoundByLookup = nullptr;
FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate();
+ // Check if we have found an existing definition. Returns with that
+ // definition if yes, otherwise returns null.
+ auto FindAndMapDefinition = [this](
+ FunctionDecl *D, FunctionDecl *FoundFunction) -> Decl * {
+ const FunctionDecl *Definition = nullptr;
+ if (D->doesThisDeclarationHaveABody() && FoundFunction->hasBody(Definition))
+ return Importer.MapImported(D, const_cast<FunctionDecl *>(Definition));
+ return nullptr;
+ };
+
// If this is a function template specialization, then try to find the same
// existing specialization in the "to" context. The lookup below will not
// find any specialization, but would find the primary template; thus, we
@@ -3010,8 +3020,8 @@
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;
}
}
@@ -3030,11 +3040,8 @@
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
