[ASTImporter] Fix redecl failures of ClassTemplateSpec
Summary:
Redecl chains of class 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, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58673
llvm-svn: 356452
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 9158e90..b845492 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5052,17 +5052,6 @@
ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl(
ClassTemplateSpecializationDecl *D) {
- // If this record has a definition in the translation unit we're coming from,
- // but this particular declaration is not that definition, import the
- // definition and map to that.
- TagDecl *Definition = D->getDefinition();
- if (Definition && Definition != D) {
- if (ExpectedDecl ImportedDefOrErr = import(Definition))
- return Importer.MapImported(D, *ImportedDefOrErr);
- else
- return ImportedDefOrErr.takeError();
- }
-
ClassTemplateDecl *ClassTemplate;
if (Error Err = importInto(ClassTemplate, D->getSpecializedTemplate()))
return std::move(Err);
@@ -5080,154 +5069,141 @@
// Try to find an existing specialization with these template arguments.
void *InsertPos = nullptr;
- ClassTemplateSpecializationDecl *D2 = nullptr;
+ ClassTemplateSpecializationDecl *PrevDecl = nullptr;
ClassTemplatePartialSpecializationDecl *PartialSpec =
dyn_cast<ClassTemplatePartialSpecializationDecl>(D);
if (PartialSpec)
- D2 = ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos);
+ PrevDecl =
+ ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos);
else
- D2 = ClassTemplate->findSpecialization(TemplateArgs, InsertPos);
- ClassTemplateSpecializationDecl * const PrevDecl = D2;
- RecordDecl *FoundDef = D2 ? D2->getDefinition() : nullptr;
- if (FoundDef) {
- if (!D->isCompleteDefinition()) {
- // The "From" translation unit only had a forward declaration; call it
- // the same declaration.
- // TODO Handle the redecl chain properly!
- return Importer.MapImported(D, FoundDef);
- }
+ PrevDecl = ClassTemplate->findSpecialization(TemplateArgs, InsertPos);
- if (IsStructuralMatch(D, FoundDef)) {
+ if (PrevDecl) {
+ if (IsStructuralMatch(D, PrevDecl)) {
+ if (D->isThisDeclarationADefinition() && PrevDecl->getDefinition()) {
+ Importer.MapImported(D, PrevDecl->getDefinition());
+ // Import those default field initializers which have been
+ // instantiated in the "From" context, but not in the "To" context.
+ for (auto *FromField : D->fields())
+ Importer.Import(FromField);
- Importer.MapImported(D, FoundDef);
+ // Import those methods which have been instantiated in the
+ // "From" context, but not in the "To" context.
+ for (CXXMethodDecl *FromM : D->methods())
+ Importer.Import(FromM);
- // Import those those default field initializers which have been
- // instantiated in the "From" context, but not in the "To" context.
- for (auto *FromField : D->fields()) {
- auto ToOrErr = import(FromField);
- if (!ToOrErr)
- // FIXME: return the error?
- consumeError(ToOrErr.takeError());
+ // TODO Import instantiated default arguments.
+ // TODO Import instantiated exception specifications.
+ //
+ // Generally, ASTCommon.h/DeclUpdateKind enum gives a very good hint
+ // what else could be fused during an AST merge.
+ return PrevDecl;
}
-
- // Import those methods which have been instantiated in the
- // "From" context, but not in the "To" context.
- for (CXXMethodDecl *FromM : D->methods()) {
- auto ToOrErr = import(FromM);
- if (!ToOrErr)
- // FIXME: return the error?
- consumeError(ToOrErr.takeError());
- }
-
- // TODO Import instantiated default arguments.
- // TODO Import instantiated exception specifications.
- //
- // Generally, ASTCommon.h/DeclUpdateKind enum gives a very good hint what
- // else could be fused during an AST merge.
-
- return FoundDef;
+ } else { // ODR violation.
+ // FIXME HandleNameConflict
+ return nullptr;
}
- } else { // We either couldn't find any previous specialization in the "To"
- // context, or we found one but without definition. Let's create a
- // new specialization and register that at the class template.
+ }
- // Import the location of this declaration.
- ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
- if (!BeginLocOrErr)
- return BeginLocOrErr.takeError();
- ExpectedSLoc IdLocOrErr = import(D->getLocation());
- if (!IdLocOrErr)
- return IdLocOrErr.takeError();
+ // Import the location of this declaration.
+ ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
+ if (!BeginLocOrErr)
+ return BeginLocOrErr.takeError();
+ ExpectedSLoc IdLocOrErr = import(D->getLocation());
+ if (!IdLocOrErr)
+ return IdLocOrErr.takeError();
- if (PartialSpec) {
- // Import TemplateArgumentListInfo.
- TemplateArgumentListInfo ToTAInfo;
- const auto &ASTTemplateArgs = *PartialSpec->getTemplateArgsAsWritten();
- if (Error Err = ImportTemplateArgumentListInfo(ASTTemplateArgs, ToTAInfo))
- return std::move(Err);
+ // Create the specialization.
+ ClassTemplateSpecializationDecl *D2 = nullptr;
+ if (PartialSpec) {
+ // Import TemplateArgumentListInfo.
+ TemplateArgumentListInfo ToTAInfo;
+ const auto &ASTTemplateArgs = *PartialSpec->getTemplateArgsAsWritten();
+ if (Error Err = ImportTemplateArgumentListInfo(ASTTemplateArgs, ToTAInfo))
+ return std::move(Err);
- QualType CanonInjType;
- if (Error Err = importInto(
- CanonInjType, PartialSpec->getInjectedSpecializationType()))
- return std::move(Err);
- CanonInjType = CanonInjType.getCanonicalType();
+ QualType CanonInjType;
+ if (Error Err = importInto(
+ CanonInjType, PartialSpec->getInjectedSpecializationType()))
+ return std::move(Err);
+ CanonInjType = CanonInjType.getCanonicalType();
- auto ToTPListOrErr = ImportTemplateParameterList(
- PartialSpec->getTemplateParameters());
- if (!ToTPListOrErr)
- return ToTPListOrErr.takeError();
+ auto ToTPListOrErr = ImportTemplateParameterList(
+ PartialSpec->getTemplateParameters());
+ if (!ToTPListOrErr)
+ return ToTPListOrErr.takeError();
- if (GetImportedOrCreateDecl<ClassTemplatePartialSpecializationDecl>(
- D2, D, Importer.getToContext(), D->getTagKind(), DC,
- *BeginLocOrErr, *IdLocOrErr, *ToTPListOrErr, ClassTemplate,
- llvm::makeArrayRef(TemplateArgs.data(), TemplateArgs.size()),
- ToTAInfo, CanonInjType,
- cast_or_null<ClassTemplatePartialSpecializationDecl>(PrevDecl)))
- return D2;
+ if (GetImportedOrCreateDecl<ClassTemplatePartialSpecializationDecl>(
+ D2, D, Importer.getToContext(), D->getTagKind(), DC,
+ *BeginLocOrErr, *IdLocOrErr, *ToTPListOrErr, ClassTemplate,
+ llvm::makeArrayRef(TemplateArgs.data(), TemplateArgs.size()),
+ ToTAInfo, CanonInjType,
+ cast_or_null<ClassTemplatePartialSpecializationDecl>(PrevDecl)))
+ return D2;
- // Update InsertPos, because preceding import calls may have invalidated
- // it by adding new specializations.
- if (!ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos))
- // Add this partial specialization to the class template.
- ClassTemplate->AddPartialSpecialization(
- cast<ClassTemplatePartialSpecializationDecl>(D2), InsertPos);
+ // Update InsertPos, because preceding import calls may have invalidated
+ // it by adding new specializations.
+ if (!ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos))
+ // Add this partial specialization to the class template.
+ ClassTemplate->AddPartialSpecialization(
+ cast<ClassTemplatePartialSpecializationDecl>(D2), InsertPos);
- } else { // Not a partial specialization.
- if (GetImportedOrCreateDecl(
- D2, D, Importer.getToContext(), D->getTagKind(), DC,
- *BeginLocOrErr, *IdLocOrErr, ClassTemplate, TemplateArgs,
- PrevDecl))
- return D2;
+ } else { // Not a partial specialization.
+ if (GetImportedOrCreateDecl(
+ D2, D, Importer.getToContext(), D->getTagKind(), DC,
+ *BeginLocOrErr, *IdLocOrErr, ClassTemplate, TemplateArgs,
+ PrevDecl))
+ return D2;
- // Update InsertPos, because preceding import calls may have invalidated
- // it by adding new specializations.
- if (!ClassTemplate->findSpecialization(TemplateArgs, InsertPos))
- // Add this specialization to the class template.
- ClassTemplate->AddSpecialization(D2, InsertPos);
- }
+ // Update InsertPos, because preceding import calls may have invalidated
+ // it by adding new specializations.
+ if (!ClassTemplate->findSpecialization(TemplateArgs, InsertPos))
+ // Add this specialization to the class template.
+ ClassTemplate->AddSpecialization(D2, InsertPos);
+ }
- D2->setSpecializationKind(D->getSpecializationKind());
+ D2->setSpecializationKind(D->getSpecializationKind());
- // Import the qualifier, if any.
- if (auto LocOrErr = import(D->getQualifierLoc()))
- D2->setQualifierInfo(*LocOrErr);
+ // Set the context of this specialization/instantiation.
+ D2->setLexicalDeclContext(LexicalDC);
+
+ // Add to the DC only if it was an explicit specialization/instantiation.
+ if (D2->isExplicitInstantiationOrSpecialization()) {
+ LexicalDC->addDeclInternal(D2);
+ }
+
+ // Import the qualifier, if any.
+ if (auto LocOrErr = import(D->getQualifierLoc()))
+ D2->setQualifierInfo(*LocOrErr);
+ else
+ return LocOrErr.takeError();
+
+ if (auto *TSI = D->getTypeAsWritten()) {
+ if (auto TInfoOrErr = import(TSI))
+ D2->setTypeAsWritten(*TInfoOrErr);
+ else
+ return TInfoOrErr.takeError();
+
+ if (auto LocOrErr = import(D->getTemplateKeywordLoc()))
+ D2->setTemplateKeywordLoc(*LocOrErr);
else
return LocOrErr.takeError();
- if (auto *TSI = D->getTypeAsWritten()) {
- if (auto TInfoOrErr = import(TSI))
- D2->setTypeAsWritten(*TInfoOrErr);
- else
- return TInfoOrErr.takeError();
-
- if (auto LocOrErr = import(D->getTemplateKeywordLoc()))
- D2->setTemplateKeywordLoc(*LocOrErr);
- else
- return LocOrErr.takeError();
-
- if (auto LocOrErr = import(D->getExternLoc()))
- D2->setExternLoc(*LocOrErr);
- else
- return LocOrErr.takeError();
- }
-
- if (D->getPointOfInstantiation().isValid()) {
- if (auto POIOrErr = import(D->getPointOfInstantiation()))
- D2->setPointOfInstantiation(*POIOrErr);
- else
- return POIOrErr.takeError();
- }
-
- D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
-
- // Set the context of this specialization/instantiation.
- D2->setLexicalDeclContext(LexicalDC);
-
- // Add to the DC only if it was an explicit specialization/instantiation.
- if (D2->isExplicitInstantiationOrSpecialization()) {
- LexicalDC->addDeclInternal(D2);
- }
+ if (auto LocOrErr = import(D->getExternLoc()))
+ D2->setExternLoc(*LocOrErr);
+ else
+ return LocOrErr.takeError();
}
+
+ if (D->getPointOfInstantiation().isValid()) {
+ if (auto POIOrErr = import(D->getPointOfInstantiation()))
+ D2->setPointOfInstantiation(*POIOrErr);
+ else
+ return POIOrErr.takeError();
+ }
+
+ D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
+
if (D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2))
return std::move(Err);
@@ -7843,6 +7819,12 @@
return ToDOrErr;
ToD = *ToDOrErr;
+ // FIXME Use getImportDeclErrorIfAny() here (and return with the error) once
+ // the error handling is finished in GetImportedOrCreateSpecialDecl().
+ if (!ToD) {
+ return nullptr;
+ }
+
// Once the decl is connected to the existing declarations, i.e. when the
// redecl chain is properly set then we populate the lookup again.
// This way the primary context will be able to find all decls.
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index ca59bc0..497d220 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -3575,12 +3575,10 @@
// The second specialization is different from the first, thus it violates
// ODR, consequently we expect to keep the first specialization only, which is
// already in the "To" context.
- EXPECT_TRUE(ImportedSpec);
- auto *ToSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match(
- ToTU, classTemplateSpecializationDecl(hasName("X")));
- EXPECT_EQ(ImportedSpec, ToSpec);
- EXPECT_EQ(1u, DeclCounter<ClassTemplateSpecializationDecl>().match(
- ToTU, classTemplateSpecializationDecl()));
+ EXPECT_FALSE(ImportedSpec);
+ EXPECT_EQ(1u,
+ DeclCounter<ClassTemplateSpecializationDecl>().match(
+ ToTU, classTemplateSpecializationDecl(hasName("X"))));
}
TEST_P(ASTImporterOptionSpecificTestBase,
@@ -3955,6 +3953,23 @@
}
};
+struct ClassTemplateSpec {
+ using DeclTy = ClassTemplateSpecializationDecl;
+ static constexpr auto *Prototype =
+ R"(
+ template <class T> class X;
+ template <> class X<int>;
+ )";
+ static constexpr auto *Definition =
+ R"(
+ template <class T> class X;
+ template <> class X<int> {};
+ )";
+ BindableMatcher<Decl> getPattern() {
+ return classTemplateSpecializationDecl(hasName("X"), unless(isImplicit()));
+ }
+};
+
template <typename TypeParam>
struct RedeclChain : ASTImporterOptionSpecificTestBase {
@@ -4276,6 +4291,9 @@
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
RedeclChain, FunctionTemplateSpec, ,
PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
+ RedeclChain, ClassTemplateSpec, ,
+ PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
RedeclChain, Function, , DefinitionShouldBeImportedAsADefinition)
@@ -4291,6 +4309,8 @@
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
RedeclChain, FunctionTemplateSpec, ,
DefinitionShouldBeImportedAsADefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
+ RedeclChain, ClassTemplateSpec, , DefinitionShouldBeImportedAsADefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
ImportPrototypeAfterImportedPrototype)
@@ -4304,6 +4324,8 @@
ImportPrototypeAfterImportedPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
ImportPrototypeAfterImportedPrototype)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
+ ImportPrototypeAfterImportedPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
ImportDefinitionAfterImportedPrototype)
@@ -4317,6 +4339,8 @@
ImportDefinitionAfterImportedPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
ImportDefinitionAfterImportedPrototype)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
+ ImportDefinitionAfterImportedPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
ImportPrototypeAfterImportedDefinition)
@@ -4330,6 +4354,8 @@
ImportPrototypeAfterImportedDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
ImportPrototypeAfterImportedDefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
+ ImportPrototypeAfterImportedDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
ImportPrototypes)
@@ -4343,6 +4369,8 @@
// 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, Function, ,
ImportDefinitions)
@@ -4357,6 +4385,8 @@
// 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, Function, ,
ImportDefinitionThenPrototype)
@@ -4372,6 +4402,8 @@
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec,
DISABLED_,
ImportDefinitionThenPrototype)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
+ ImportDefinitionThenPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
ImportPrototypeThenDefinition)
@@ -4387,6 +4419,8 @@
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec,
DISABLED_,
ImportPrototypeThenDefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplateSpec, ,
+ ImportPrototypeThenDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
WholeRedeclChainIsImportedAtOnce)
@@ -4420,6 +4454,8 @@
DefaultTestValuesForRunOptions, );
INSTANTIATE_TEST_CASE_P(ParameterizedTests, RedeclChainFunctionTemplateSpec,
DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, RedeclChainClassTemplateSpec,
+ DefaultTestValuesForRunOptions, );