martong updated this revision to Diff 189131.
martong marked 2 inline comments as done.
martong added a comment.
- Fix some comments
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58673/new/
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,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);
- }
-
- 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 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 +7809,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
