https://github.com/balazske updated
https://github.com/llvm/llvm-project/pull/101836
From 2e98fc222566c5e746ade4ccaba23de3b59e0a5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Sat, 3 Aug 2024 18:10:34 +0200
Subject: [PATCH 1/2] [clang][ASTImporter] New fix for default template
parameter values.
Commit e4440b8 added a change that introduced new crash in an
incorrectly handled case. This is fixed here.
---
clang/lib/AST/ASTImporter.cpp | 12 ++-
clang/unittests/AST/ASTImporterTest.cpp | 97 +++++++++++++++++++++++++
2 files changed, 106 insertions(+), 3 deletions(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 103235547f482e..7e4a92ccbe40f7 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5972,7 +5972,11 @@
ASTNodeImporter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
import(D->getDefaultArgument());
if (!ToDefaultArgOrErr)
return ToDefaultArgOrErr.takeError();
- ToD->setDefaultArgument(ToD->getASTContext(), *ToDefaultArgOrErr);
+ // The import process can trigger import of the parent template which can
+ // set the default argument value (to "inherited").
+ // In this case do nothing here.
+ if (!ToD->hasDefaultArgument())
+ ToD->setDefaultArgument(ToD->getASTContext(), *ToDefaultArgOrErr);
}
return ToD;
@@ -6004,7 +6008,8 @@
ASTNodeImporter::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
import(D->getDefaultArgument());
if (!ToDefaultArgOrErr)
return ToDefaultArgOrErr.takeError();
- ToD->setDefaultArgument(Importer.getToContext(), *ToDefaultArgOrErr);
+ if (!ToD->hasDefaultArgument())
+ ToD->setDefaultArgument(Importer.getToContext(), *ToDefaultArgOrErr);
}
return ToD;
@@ -6041,7 +6046,8 @@
ASTNodeImporter::VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
import(D->getDefaultArgument());
if (!ToDefaultArgOrErr)
return ToDefaultArgOrErr.takeError();
- ToD->setDefaultArgument(Importer.getToContext(), *ToDefaultArgOrErr);
+ if (!ToD->hasDefaultArgument())
+ ToD->setDefaultArgument(Importer.getToContext(), *ToDefaultArgOrErr);
}
return ToD;
diff --git a/clang/unittests/AST/ASTImporterTest.cpp
b/clang/unittests/AST/ASTImporterTest.cpp
index 57242ff49fe3b8..4c41171deec46a 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9919,6 +9919,103 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
ImportExistingVarTemplate) {
testImport(FromLastD);
}
+TEST_P(ImportTemplateParmDeclDefaultValue,
+ ImportParentTemplateDuringNonTypeTemplateParmDecl) {
+ // This wants to provoke that during import of 'Y' in "typename T = Y"
+ // (before this import returns) the later definition of 'X' is imported
fully.
+ const char *Code =
+ R"(
+ struct Z;
+
+ struct Y {
+ Z *z;
+ static const int x = 1;
+ };
+
+ template <int P = Y::x>
+ struct X;
+
+ template <int P>
+ struct X {
+ static const int A = 1;
+ };
+
+ struct Z {
+ template<int P>
+ void f(int A = X<P>::A);
+ };
+ )";
+
+ Decl *FromTU = getTuDecl(Code, Lang_CXX14);
+ auto *FromD = FirstDeclMatcher<NonTypeTemplateParmDecl>().match(
+ FromTU, nonTypeTemplateParmDecl(hasName("P")));
+ auto *ToD = Import(FromD, Lang_CXX14);
+ EXPECT_TRUE(ToD);
+}
+
+TEST_P(ImportTemplateParmDeclDefaultValue,
+ ImportParentTemplateDuringTemplateTypeParmDecl) {
+ const char *Code =
+ R"(
+ struct Z;
+
+ struct Y {
+ Z *z;
+ };
+
+ template <typename T = Y>
+ struct X;
+
+ template <typename T>
+ struct X {
+ static const int A = 1;
+ };
+
+ struct Z {
+ template<typename T>
+ void f(int A = X<T>::A);
+ };
+ )";
+
+ Decl *FromTU = getTuDecl(Code, Lang_CXX14);
+ auto *FromD = FirstDeclMatcher<TemplateTypeParmDecl>().match(
+ FromTU, templateTypeParmDecl(hasName("T")));
+ auto *ToD = Import(FromD, Lang_CXX14);
+ EXPECT_TRUE(ToD);
+}
+
+TEST_P(ImportTemplateParmDeclDefaultValue,
+ ImportParentTemplateDuringTemplateTemplateParmDecl) {
+ const char *Code =
+ R"(
+ struct Z;
+
+ template <int>
+ struct Y {
+ Z *z;
+ };
+
+ template <template <int> class T = Y>
+ struct X;
+
+ template <template <int> class T>
+ struct X {
+ static const int A = 1;
+ };
+
+ struct Z {
+ template <template <int> class T>
+ void f(int A = X<T>::A);
+ };
+ )";
+
+ Decl *FromTU = getTuDecl(Code, Lang_CXX14);
+ auto *FromD = FirstDeclMatcher<TemplateTemplateParmDecl>().match(
+ FromTU, templateTemplateParmDecl(hasName("T")));
+ auto *ToD = Import(FromD, Lang_CXX14);
+ EXPECT_TRUE(ToD);
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
From 70ef4f2579bbb91f7babdf62feef0e5c9173fa31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Fri, 16 Aug 2024 09:58:49 +0200
Subject: [PATCH 2/2] added more tests, updated comments
---
clang/lib/AST/ASTImporter.cpp | 30 +++++-----
clang/unittests/AST/ASTImporterTest.cpp | 76 ++++++++++++++++++-------
2 files changed, 70 insertions(+), 36 deletions(-)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 7e4a92ccbe40f7..f8865baa80ae8c 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -384,16 +384,10 @@ namespace clang {
// following declarations have a reference to the original default value
// through the "inherited" value. This value should be set for all imported
// template parameters that have a previous declaration (also a previous
- // template declaration).
- //
- // In the `Visit*ParmDecl` functions the default value of these template
- // arguments is always imported. At that location the previous declaration
- // is not easily accessible, it is not possible to call
- // `setInheritedDefaultArgument` at that place.
- // `updateTemplateParametersInheritedFrom` is called later when the already
- // imported default value is erased and changed to "inherited".
- // It is important to change the mode to "inherited" otherwise false
- // structural in-equivalences could be detected.
+ // template declaration). The chain of parameter default value inheritances
+ // will have the same order as the chain of previous declarations, in the
+ // "To" AST. The "From" AST may have a different order because import does
+ // not happen sequentially.
void updateTemplateParametersInheritedFrom(
const TemplateParameterList &RecentParams,
TemplateParameterList &NewParams) {
@@ -5934,8 +5928,8 @@
ASTNodeImporter::VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D) {
ExpectedDecl
ASTNodeImporter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
// For template arguments, we adopt the translation unit as our declaration
- // context. This context will be fixed when the actual template declaration
- // is created.
+ // context. This context will be fixed when (during) the actual template
+ // declaration is created.
ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
if (!BeginLocOrErr)
@@ -5968,13 +5962,19 @@
ASTNodeImporter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
}
if (D->hasDefaultArgument()) {
+ // Default argument can be "inherited" when it has a reference to the
+ // previous declaration (of the default argument) which is stored only
once.
+ // Here we import the default argument in any case, and the inherited state
+ // is updated later after the parent template was created. If the
+ // inherited-from object would be imported here it causes more difficulties
+ // (parent template may not be created yet and import loops can occur).
Expected<TemplateArgumentLoc> ToDefaultArgOrErr =
import(D->getDefaultArgument());
if (!ToDefaultArgOrErr)
return ToDefaultArgOrErr.takeError();
- // The import process can trigger import of the parent template which can
- // set the default argument value (to "inherited").
- // In this case do nothing here.
+ // The just called import process can trigger import of the parent template
+ // which can update the default argument value to "inherited". This should
+ // not be changed.
if (!ToD->hasDefaultArgument())
ToD->setDefaultArgument(ToD->getASTContext(), *ToDefaultArgOrErr);
}
diff --git a/clang/unittests/AST/ASTImporterTest.cpp
b/clang/unittests/AST/ASTImporterTest.cpp
index 4c41171deec46a..73a5cb541e4d29 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9837,6 +9837,37 @@ struct ImportTemplateParmDeclDefaultValue
checkTemplateParams(ToD);
}
+ // In these tests the ASTImporter visit function for second template
parameter
+ // (called 'T2') is called recursively from visit function of the first
+ // parameter (called 'T1') (look at the codes). The default value is
specified
+ // at 'T1' and inherited at 'T2'. But because during the import the second
+ // template is created first, it is added to the "To" AST first. This way the
+ // declaration chain is reversed with the import. The import ensures
currently
+ // that the first occurrence will have the default template argument in place
+ // and the second will have it inherited from the first. This is reversed
+ // compared to the original.
+ // FIXME: This reversal is probably an unexpected property of the new "To"
+ // AST. Source locations can become wrong, for example default argument that
+ // was specified at 'T2' appears at 'T1' after the import but with the
+ // original source location. This can affect source ranges of parent
+ // declarations.
+ template <class TemplateParmDeclT>
+ void testImportTemplateParmDeclReversed(TemplateParmDeclT *FromD,
+ TemplateParmDeclT *FromDInherited) {
+ ASSERT_FALSE(FromD->getDefaultArgStorage().isInherited());
+ ASSERT_TRUE(FromDInherited->getDefaultArgStorage().isInherited());
+
+ auto *ToD = Import(FromD, Lang_CXX14);
+ EXPECT_TRUE(ToD);
+
+ auto *ToDInherited = Import(FromDInherited, Lang_CXX14);
+ EXPECT_TRUE(ToDInherited);
+
+ EXPECT_TRUE(ToD->getDefaultArgStorage().isInherited());
+ EXPECT_FALSE(ToDInherited->getDefaultArgStorage().isInherited());
+ EXPECT_EQ(ToD->getDefaultArgStorage().getInheritedFrom(), ToDInherited);
+ }
+
const char *CodeFunction =
R"(
template <class> struct X;
@@ -9920,9 +9951,7 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
ImportExistingVarTemplate) {
}
TEST_P(ImportTemplateParmDeclDefaultValue,
- ImportParentTemplateDuringNonTypeTemplateParmDecl) {
- // This wants to provoke that during import of 'Y' in "typename T = Y"
- // (before this import returns) the later definition of 'X' is imported
fully.
+ ImportNonTypeTemplateParmDeclReversed) {
const char *Code =
R"(
struct Z;
@@ -9932,10 +9961,10 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
static const int x = 1;
};
- template <int P = Y::x>
+ template <int P1 = Y::x>
struct X;
- template <int P>
+ template <int P2>
struct X {
static const int A = 1;
};
@@ -9948,13 +9977,14 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
Decl *FromTU = getTuDecl(Code, Lang_CXX14);
auto *FromD = FirstDeclMatcher<NonTypeTemplateParmDecl>().match(
- FromTU, nonTypeTemplateParmDecl(hasName("P")));
- auto *ToD = Import(FromD, Lang_CXX14);
- EXPECT_TRUE(ToD);
+ FromTU, nonTypeTemplateParmDecl(hasName("P1")));
+ auto *FromDInherited = FirstDeclMatcher<NonTypeTemplateParmDecl>().match(
+ FromTU, nonTypeTemplateParmDecl(hasName("P2")));
+
+ testImportTemplateParmDeclReversed(FromD, FromDInherited);
}
-TEST_P(ImportTemplateParmDeclDefaultValue,
- ImportParentTemplateDuringTemplateTypeParmDecl) {
+TEST_P(ImportTemplateParmDeclDefaultValue, ImportTemplateTypeParmDeclReversed)
{
const char *Code =
R"(
struct Z;
@@ -9963,10 +9993,10 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
Z *z;
};
- template <typename T = Y>
+ template <typename T1 = Y>
struct X;
- template <typename T>
+ template <typename T2>
struct X {
static const int A = 1;
};
@@ -9979,13 +10009,15 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
Decl *FromTU = getTuDecl(Code, Lang_CXX14);
auto *FromD = FirstDeclMatcher<TemplateTypeParmDecl>().match(
- FromTU, templateTypeParmDecl(hasName("T")));
- auto *ToD = Import(FromD, Lang_CXX14);
- EXPECT_TRUE(ToD);
+ FromTU, templateTypeParmDecl(hasName("T1")));
+ auto *FromDInherited = FirstDeclMatcher<TemplateTypeParmDecl>().match(
+ FromTU, templateTypeParmDecl(hasName("T2")));
+
+ testImportTemplateParmDeclReversed(FromD, FromDInherited);
}
TEST_P(ImportTemplateParmDeclDefaultValue,
- ImportParentTemplateDuringTemplateTemplateParmDecl) {
+ ImportTemplateTemplateParmDeclReversed) {
const char *Code =
R"(
struct Z;
@@ -9995,10 +10027,10 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
Z *z;
};
- template <template <int> class T = Y>
+ template <template <int> class T1 = Y>
struct X;
- template <template <int> class T>
+ template <template <int> class T2>
struct X {
static const int A = 1;
};
@@ -10011,9 +10043,11 @@ TEST_P(ImportTemplateParmDeclDefaultValue,
Decl *FromTU = getTuDecl(Code, Lang_CXX14);
auto *FromD = FirstDeclMatcher<TemplateTemplateParmDecl>().match(
- FromTU, templateTemplateParmDecl(hasName("T")));
- auto *ToD = Import(FromD, Lang_CXX14);
- EXPECT_TRUE(ToD);
+ FromTU, templateTemplateParmDecl(hasName("T1")));
+ auto *FromDInherited = FirstDeclMatcher<TemplateTemplateParmDecl>().match(
+ FromTU, templateTemplateParmDecl(hasName("T2")));
+
+ testImportTemplateParmDeclReversed(FromD, FromDInherited);
}
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
