[clang] [clang][ASTImporter] Improve import of variable template specializations. (PR #78284)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 16 06:13:15 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Balázs Kéri (balazske)
<details>
<summary>Changes</summary>
Code of `VisitVarTemplateSpecializationDecl` was rewritten based on code of `VisitVarDecl`. Additional changes (in structural equivalence) were made to make tests pass.
---
Patch is 22.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78284.diff
4 Files Affected:
- (modified) clang/lib/AST/ASTImporter.cpp (+118-99)
- (modified) clang/lib/AST/ASTStructuralEquivalence.cpp (+10-3)
- (modified) clang/unittests/AST/ASTImporterGenericRedeclTest.cpp (+54-7)
- (modified) clang/unittests/AST/ASTImporterTest.cpp (+53-1)
``````````diff
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c8085c5dd48a59..cb5901bb7dfc6a 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6360,16 +6360,19 @@ ExpectedDecl ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) {
ExpectedDecl ASTNodeImporter::VisitVarTemplateSpecializationDecl(
VarTemplateSpecializationDecl *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.
- VarDecl *Definition = D->getDefinition();
- if (Definition && Definition != D) {
- if (ExpectedDecl ImportedDefOrErr = import(Definition))
- return Importer.MapImported(D, *ImportedDefOrErr);
- else
- return ImportedDefOrErr.takeError();
+ // A VarTemplateSpecializationDecl inherits from VarDecl, the import is done
+ // in an analog way (but specialized for this case).
+
+ SmallVector<Decl *, 2> Redecls = getCanonicalForwardRedeclChain(D);
+ auto RedeclIt = Redecls.begin();
+ // Import the first part of the decl chain. I.e. import all previous
+ // declarations starting from the canonical decl.
+ for (; RedeclIt != Redecls.end() && *RedeclIt != D; ++RedeclIt) {
+ ExpectedDecl RedeclOrErr = import(*RedeclIt);
+ if (!RedeclOrErr)
+ return RedeclOrErr.takeError();
}
+ assert(*RedeclIt == D);
VarTemplateDecl *VarTemplate = nullptr;
if (Error Err = importInto(VarTemplate, D->getSpecializedTemplate()))
@@ -6397,116 +6400,132 @@ ExpectedDecl ASTNodeImporter::VisitVarTemplateSpecializationDecl(
// Try to find an existing specialization with these template arguments.
void *InsertPos = nullptr;
- VarTemplateSpecializationDecl *D2 = VarTemplate->findSpecialization(
- TemplateArgs, InsertPos);
- if (D2) {
- // We already have a variable template specialization with these template
- // arguments.
-
- // FIXME: Check for specialization vs. instantiation errors.
-
- if (VarDecl *FoundDef = D2->getDefinition()) {
- if (!D->isThisDeclarationADefinition() ||
- IsStructuralMatch(D, FoundDef)) {
- // The record types structurally match, or the "from" translation
- // unit only had a forward declaration anyway; call it the same
- // variable.
- return Importer.MapImported(D, FoundDef);
+ VarTemplateSpecializationDecl *FoundSpecialization =
+ VarTemplate->findSpecialization(TemplateArgs, InsertPos);
+ if (FoundSpecialization) {
+ if (IsStructuralMatch(D, FoundSpecialization)) {
+ VarDecl *FoundDef = FoundSpecialization->getDefinition();
+ if (D->getDeclContext()->isRecord()) {
+ // In a record, it is allowed only to have one optional declaration and
+ // one definition of the (static or constexpr) variable template.
+ assert(
+ FoundSpecialization->getDeclContext()->isRecord() &&
+ "Member variable template specialization imported as non-member, "
+ "inconsistent imported AST?");
+ if (FoundDef)
+ return Importer.MapImported(D, FoundDef);
+ if (!D->isThisDeclarationADefinition())
+ return Importer.MapImported(D, FoundSpecialization);
+ } else {
+ // If definition is imported and there is already one, map to it.
+ // Otherwise create a new variable and link it to the existing.
+ if (FoundDef && D->isThisDeclarationADefinition())
+ return Importer.MapImported(D, FoundDef);
}
+ } else {
+ return make_error<ASTImportError>(ASTImportError::NameConflict);
}
- } else {
- TemplateArgumentListInfo ToTAInfo;
- if (const ASTTemplateArgumentListInfo *Args = D->getTemplateArgsInfo()) {
- if (Error Err = ImportTemplateArgumentListInfo(*Args, ToTAInfo))
- return std::move(Err);
- }
+ }
- using PartVarSpecDecl = VarTemplatePartialSpecializationDecl;
- // Create a new specialization.
- if (auto *FromPartial = dyn_cast<PartVarSpecDecl>(D)) {
- // Import TemplateArgumentListInfo
- TemplateArgumentListInfo ArgInfos;
- const auto *FromTAArgsAsWritten = FromPartial->getTemplateArgsAsWritten();
- // NOTE: FromTAArgsAsWritten and template parameter list are non-null.
- if (Error Err = ImportTemplateArgumentListInfo(
- *FromTAArgsAsWritten, ArgInfos))
- return std::move(Err);
+ VarTemplateSpecializationDecl *D2 = nullptr;
- auto ToTPListOrErr = import(FromPartial->getTemplateParameters());
- if (!ToTPListOrErr)
- return ToTPListOrErr.takeError();
+ TemplateArgumentListInfo ToTAInfo;
+ if (const ASTTemplateArgumentListInfo *Args = D->getTemplateArgsInfo()) {
+ if (Error Err = ImportTemplateArgumentListInfo(*Args, ToTAInfo))
+ return std::move(Err);
+ }
- PartVarSpecDecl *ToPartial;
- if (GetImportedOrCreateDecl(ToPartial, D, Importer.getToContext(), DC,
- *BeginLocOrErr, *IdLocOrErr, *ToTPListOrErr,
- VarTemplate, QualType(), nullptr,
- D->getStorageClass(), TemplateArgs, ArgInfos))
- return ToPartial;
+ using PartVarSpecDecl = VarTemplatePartialSpecializationDecl;
+ // Create a new specialization.
+ if (auto *FromPartial = dyn_cast<PartVarSpecDecl>(D)) {
+ // Import TemplateArgumentListInfo
+ TemplateArgumentListInfo ArgInfos;
+ const auto *FromTAArgsAsWritten = FromPartial->getTemplateArgsAsWritten();
+ // NOTE: FromTAArgsAsWritten and template parameter list are non-null.
+ if (Error Err =
+ ImportTemplateArgumentListInfo(*FromTAArgsAsWritten, ArgInfos))
+ return std::move(Err);
- if (Expected<PartVarSpecDecl *> ToInstOrErr = import(
- FromPartial->getInstantiatedFromMember()))
- ToPartial->setInstantiatedFromMember(*ToInstOrErr);
- else
- return ToInstOrErr.takeError();
-
- if (FromPartial->isMemberSpecialization())
- ToPartial->setMemberSpecialization();
-
- D2 = ToPartial;
-
- // FIXME: Use this update if VarTemplatePartialSpecializationDecl is fixed
- // to adopt template parameters.
- // updateLookupTableForTemplateParameters(**ToTPListOrErr);
- } else { // Full specialization
- if (GetImportedOrCreateDecl(D2, D, Importer.getToContext(), DC,
- *BeginLocOrErr, *IdLocOrErr, VarTemplate,
- QualType(), nullptr, D->getStorageClass(),
- TemplateArgs))
- return D2;
- }
+ auto ToTPListOrErr = import(FromPartial->getTemplateParameters());
+ if (!ToTPListOrErr)
+ return ToTPListOrErr.takeError();
- QualType T;
- if (Error Err = importInto(T, D->getType()))
- return std::move(Err);
- D2->setType(T);
+ PartVarSpecDecl *ToPartial;
+ if (GetImportedOrCreateDecl(ToPartial, D, Importer.getToContext(), DC,
+ *BeginLocOrErr, *IdLocOrErr, *ToTPListOrErr,
+ VarTemplate, QualType(), nullptr,
+ D->getStorageClass(), TemplateArgs, ArgInfos))
+ return ToPartial;
- auto TInfoOrErr = import(D->getTypeSourceInfo());
- if (!TInfoOrErr)
- return TInfoOrErr.takeError();
- D2->setTypeSourceInfo(*TInfoOrErr);
+ if (Expected<PartVarSpecDecl *> ToInstOrErr =
+ import(FromPartial->getInstantiatedFromMember()))
+ ToPartial->setInstantiatedFromMember(*ToInstOrErr);
+ else
+ return ToInstOrErr.takeError();
- if (D->getPointOfInstantiation().isValid()) {
- if (ExpectedSLoc POIOrErr = import(D->getPointOfInstantiation()))
- D2->setPointOfInstantiation(*POIOrErr);
- else
- return POIOrErr.takeError();
- }
+ if (FromPartial->isMemberSpecialization())
+ ToPartial->setMemberSpecialization();
+
+ D2 = ToPartial;
- D2->setSpecializationKind(D->getSpecializationKind());
- D2->setTemplateArgsInfo(ToTAInfo);
+ // FIXME: Use this update if VarTemplatePartialSpecializationDecl is fixed
+ // to adopt template parameters.
+ // updateLookupTableForTemplateParameters(**ToTPListOrErr);
+ } else { // Full specialization
+ if (GetImportedOrCreateDecl(D2, D, Importer.getToContext(), DC,
+ *BeginLocOrErr, *IdLocOrErr, VarTemplate,
+ QualType(), nullptr, D->getStorageClass(),
+ TemplateArgs))
+ return D2;
+ }
- // Add this specialization to the class template.
- VarTemplate->AddSpecialization(D2, InsertPos);
+ QualType T;
+ if (Error Err = importInto(T, D->getType()))
+ return std::move(Err);
+ D2->setType(T);
- // Import the qualifier, if any.
- if (auto LocOrErr = import(D->getQualifierLoc()))
- D2->setQualifierInfo(*LocOrErr);
+ auto TInfoOrErr = import(D->getTypeSourceInfo());
+ if (!TInfoOrErr)
+ return TInfoOrErr.takeError();
+ D2->setTypeSourceInfo(*TInfoOrErr);
+
+ if (D->getPointOfInstantiation().isValid()) {
+ if (ExpectedSLoc POIOrErr = import(D->getPointOfInstantiation()))
+ D2->setPointOfInstantiation(*POIOrErr);
else
- return LocOrErr.takeError();
+ return POIOrErr.takeError();
+ }
- if (D->isConstexpr())
- D2->setConstexpr(true);
+ D2->setSpecializationKind(D->getSpecializationKind());
+ D2->setTemplateArgsInfo(ToTAInfo);
- // Add the specialization to this context.
- D2->setLexicalDeclContext(LexicalDC);
- LexicalDC->addDeclInternal(D2);
+ if (auto LocOrErr = import(D->getQualifierLoc()))
+ D2->setQualifierInfo(*LocOrErr);
+ else
+ return LocOrErr.takeError();
- D2->setAccess(D->getAccess());
- }
+ if (D->isConstexpr())
+ D2->setConstexpr(true);
+
+ D2->setAccess(D->getAccess());
if (Error Err = ImportInitializer(D, D2))
return std::move(Err);
+ if (FoundSpecialization)
+ D2->setPreviousDecl(FoundSpecialization->getMostRecentDecl());
+
+ VarTemplate->AddSpecialization(D2, InsertPos);
+
+ addDeclToContexts(D, D2);
+
+ // Import the rest of the chain. I.e. import all subsequent declarations.
+ for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt) {
+ ExpectedDecl RedeclOrErr = import(*RedeclIt);
+ if (!RedeclOrErr)
+ return RedeclOrErr.takeError();
+ }
+
return D2;
}
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp
index a9e0d1698a9178..bd4e0b3fa3a12b 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1319,9 +1319,6 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
VarDecl *D1, VarDecl *D2) {
- if (D1->getStorageClass() != D2->getStorageClass())
- return false;
-
IdentifierInfo *Name1 = D1->getIdentifier();
IdentifierInfo *Name2 = D2->getIdentifier();
if (!::IsStructurallyEquivalent(Name1, Name2))
@@ -1330,6 +1327,16 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
if (!IsStructurallyEquivalent(Context, D1->getType(), D2->getType()))
return false;
+ // Compare storage class and initializer only if none or both are a
+ // definition. Like a forward-declaration matches a class definition, variable
+ // declarations that are not definitions should match with the definitions.
+ if (D1->isThisDeclarationADefinition() !=
+ D2->isThisDeclarationADefinition())
+ return true;
+
+ if (D1->getStorageClass() != D2->getStorageClass())
+ return false;
+
return IsStructurallyEquivalent(Context, D1->getInit(), D2->getInit());
}
diff --git a/clang/unittests/AST/ASTImporterGenericRedeclTest.cpp b/clang/unittests/AST/ASTImporterGenericRedeclTest.cpp
index 1206fac15d4a24..24e7fb6373f7f0 100644
--- a/clang/unittests/AST/ASTImporterGenericRedeclTest.cpp
+++ b/clang/unittests/AST/ASTImporterGenericRedeclTest.cpp
@@ -80,7 +80,6 @@ struct VariableTemplate {
static constexpr auto *Definition =
R"(
template <class T> T X;
- template <> int X<int>;
)";
// There is no matcher for varTemplateDecl so use a work-around.
BindableMatcher<Decl> getPattern() {
@@ -118,19 +117,41 @@ struct ClassTemplateSpec {
using DeclTy = ClassTemplateSpecializationDecl;
static constexpr auto *Prototype =
R"(
- template <class T> class X;
- template <> class X<int>;
- )";
+ template <class T> class X;
+ template <> class X<int>;
+ )";
static constexpr auto *Definition =
R"(
- template <class T> class X;
- template <> class X<int> {};
- )";
+ template <class T> class X;
+ template <> class X<int> {};
+ )";
BindableMatcher<Decl> getPattern() {
return classTemplateSpecializationDecl(hasName("X"), unless(isImplicit()));
}
};
+const internal::VariadicDynCastAllOfMatcher<Decl, VarTemplateDecl>
+ varTemplateDecl;
+const internal::VariadicDynCastAllOfMatcher<Decl, VarTemplateSpecializationDecl>
+ varTemplateSpecializationDecl;
+
+struct VariableTemplateSpec {
+ using DeclTy = VarTemplateSpecializationDecl;
+ static constexpr auto *Prototype =
+ R"(
+ template <class T> extern T X;
+ template <> extern int X<int>;
+ )";
+ static constexpr auto *Definition =
+ R"(
+ template <class T> T X;
+ template <> int X<int>;
+ )";
+ BindableMatcher<Decl> getPattern() {
+ return varTemplateSpecializationDecl(hasName("X"), unless(isImplicit()));
+ }
+};
+
template <typename TypeParam>
struct RedeclChain : ASTImporterOptionSpecificTestBase {
@@ -451,6 +472,9 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(
RedeclChain, ClassTemplateSpec, ,
PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(
+ RedeclChain, VariableTemplateSpec, ,
+ PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
DefinitionShouldBeImportedAsADefinition)
@@ -470,6 +494,9 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
DefinitionShouldBeImportedAsADefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, ClassTemplateSpec, ,
DefinitionShouldBeImportedAsADefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(
+ RedeclChain, VariableTemplateSpec, ,
+ DefinitionShouldBeImportedAsADefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
ImportPrototypeAfterImportedPrototype)
@@ -489,6 +516,8 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
ImportPrototypeAfterImportedPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, ClassTemplateSpec, ,
ImportPrototypeAfterImportedPrototype)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplateSpec, ,
+ ImportPrototypeAfterImportedPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
ImportDefinitionAfterImportedPrototype)
@@ -508,6 +537,8 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
ImportDefinitionAfterImportedPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, ClassTemplateSpec, ,
ImportDefinitionAfterImportedPrototype)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplateSpec, ,
+ ImportDefinitionAfterImportedPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
ImportPrototypeAfterImportedDefinition)
@@ -527,6 +558,8 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
ImportPrototypeAfterImportedDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, ClassTemplateSpec, ,
ImportPrototypeAfterImportedDefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplateSpec, ,
+ ImportPrototypeAfterImportedDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
ImportPrototypes)
@@ -545,6 +578,8 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, ClassTemplateSpec, ,
ImportPrototypes)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
ImportPrototypes)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplateSpec, ,
+ ImportPrototypes)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
ImportDefinitions)
@@ -563,6 +598,8 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, ClassTemplateSpec, ,
ImportDefinitions)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
ImportDefinitions)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplateSpec, ,
+ ImportDefinitions)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
ImportDefinitionThenPrototype)
@@ -582,6 +619,8 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
ImportDefinitionThenPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, ClassTemplateSpec, ,
ImportDefinitionThenPrototype)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplateSpec, ,
+ ImportDefinitionThenPrototype)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
ImportPrototypeThenDefinition)
@@ -601,6 +640,8 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
ImportPrototypeThenDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, ClassTemplateSpec, ,
ImportPrototypeThenDefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplateSpec, ,
+ ImportPrototypeThenDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
WholeRedeclChainIsImportedAtOnce)
@@ -612,6 +653,8 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplate, ,
WholeRedeclChainIsImportedAtOnce)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
WholeRedeclChainIsImportedAtOnce)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplateSpec, ,
+ WholeRedeclChainIsImportedAtOnce)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, Function, ,
ImportPrototypeThenProtoAndDefinition)
@@ -623,6 +666,8 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplate, ,
ImportPrototypeThenProtoAndDefinition)
ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, FunctionTemplateSpec, ,
ImportPrototypeThenProtoAndDefinition)
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_SUITE(RedeclChain, VariableTemplateSpec, ,
+ ImportPrototypeThenProtoAndDefinition)
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, RedeclChainFunction,
DefaultTestValuesForRunOptions);
@@ -642,6 +687,8 @@ INSTANT...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/78284
More information about the cfe-commits
mailing list