martong created this revision.
martong added reviewers: a_sidorin, shafik.
Herald added subscribers: cfe-commits, jdoerfert, gamesh411, Szelethus, dkrupp,
rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
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.
Repository:
rC Clang
https://reviews.llvm.org/D58673
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -3471,12 +3471,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,
@@ -3851,6 +3849,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 {
@@ -4173,6 +4188,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)
@@ -4189,6 +4207,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)
@@ -4204,6 +4224,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)
@@ -4221,6 +4243,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)
@@ -4238,6 +4262,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)
@@ -4252,6 +4278,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)
@@ -4267,6 +4295,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)
@@ -4284,6 +4314,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)
@@ -4301,6 +4333,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)
@@ -4338,6 +4372,8 @@
DefaultTestValuesForRunOptions, );
INSTANTIATE_TEST_CASE_P(ParameterizedTests, RedeclChainFunctionTemplateSpec,
DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, RedeclChainClassTemplateSpec,
+ DefaultTestValuesForRunOptions, );
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -5097,17 +5097,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);
@@ -5125,154 +5114,142 @@
// 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);
- }
-
- if (IsStructuralMatch(D, FoundDef)) {
-
- Importer.MapImported(D, FoundDef);
-
- // 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());
+ PrevDecl = ClassTemplate->findSpecialization(TemplateArgs, InsertPos);
+
+ if (PrevDecl) {
+ if (IsStructuralMatch(D, PrevDecl)) {
+ if (D->isThisDeclarationADefinition() && PrevDecl->getDefinition()) {
+ Importer.MapImported(D, PrevDecl->getDefinition());
+ // 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())
+ Importer.Import(FromField);
+
+ // 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);
+
+ // 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;
}
+ } else { // ODR violation.
+ // FIXME HandleNameConflict
+ return nullptr;
+ }
+ }
- // 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());
- }
+ // 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();
- // 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.
+ // 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);
- return FoundDef;
- }
- } 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.
+ 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();
+
+ 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;
- // 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);
+ // 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);
- QualType CanonInjType;
- if (Error Err = importInto(
- CanonInjType, PartialSpec->getInjectedSpecializationType()))
- return std::move(Err);
- CanonInjType = CanonInjType.getCanonicalType();
+ } else { // Not a partial specialization.
+ if (GetImportedOrCreateDecl(
+ D2, D, Importer.getToContext(), D->getTagKind(), DC,
+ *BeginLocOrErr, *IdLocOrErr, ClassTemplate, TemplateArgs,
+ PrevDecl))
+ return D2;
- auto ToTPListOrErr = ImportTemplateParameterList(
- PartialSpec->getTemplateParameters());
- if (!ToTPListOrErr)
- return ToTPListOrErr.takeError();
+ // 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);
+ }
- 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;
+ D2->setSpecializationKind(D->getSpecializationKind());
- // 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);
+ // Set the context of this specialization/instantiation.
+ D2->setLexicalDeclContext(LexicalDC);
- } else { // Not a partial specialization.
- if (GetImportedOrCreateDecl(
- D2, D, Importer.getToContext(), D->getTagKind(), DC,
- *BeginLocOrErr, *IdLocOrErr, ClassTemplate, TemplateArgs,
- PrevDecl))
- return D2;
+ // Add to the DC only if it was an explicit specialization/instantiation.
+ if (D2->isExplicitInstantiationOrSpecialization()) {
+ LexicalDC->addDeclInternal(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);
- }
+ // Import the qualifier, if any.
+ if (auto LocOrErr = import(D->getQualifierLoc()))
+ D2->setQualifierInfo(*LocOrErr);
+ else
+ return LocOrErr.takeError();
- D2->setSpecializationKind(D->getSpecializationKind());
+ if (auto *TSI = D->getTypeAsWritten()) {
+ if (auto TInfoOrErr = import(TSI))
+ D2->setTypeAsWritten(*TInfoOrErr);
+ else
+ return TInfoOrErr.takeError();
- // Import the qualifier, if any.
- if (auto LocOrErr = import(D->getQualifierLoc()))
- D2->setQualifierInfo(*LocOrErr);
+ 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();
- }
+ if (auto LocOrErr = import(D->getExternLoc()))
+ D2->setExternLoc(*LocOrErr);
+ else
+ return LocOrErr.takeError();
+ }
- D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
+ if (D->getPointOfInstantiation().isValid()) {
+ if (auto POIOrErr = import(D->getPointOfInstantiation()))
+ D2->setPointOfInstantiation(*POIOrErr);
+ else
+ return POIOrErr.takeError();
+ }
- // Set the context of this specialization/instantiation.
- D2->setLexicalDeclContext(LexicalDC);
+ D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
- // Add to the DC only if it was an explicit specialization/instantiation.
- if (D2->isExplicitInstantiationOrSpecialization()) {
- LexicalDC->addDeclInternal(D2);
- }
- }
if (D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2))
return std::move(Err);
@@ -7833,6 +7810,12 @@
}
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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
