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/3] [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/3] 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, From fcb886c3a367882bd317cc7bf7038d981713bccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Fri, 30 Aug 2024 10:34:04 +0200 Subject: [PATCH 3/3] import inherited default value in original order --- clang/lib/AST/ASTImporter.cpp | 120 ++++------ clang/unittests/AST/ASTImporterTest.cpp | 281 +++++++++++++++--------- 2 files changed, 215 insertions(+), 186 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index f8865baa80ae8c..40319a4d99dde1 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -360,45 +360,42 @@ namespace clang { } template <typename TemplateParmDeclT> - void tryUpdateTemplateParmDeclInheritedFrom(NamedDecl *RecentParm, - NamedDecl *NewParm) { - if (auto *ParmT = dyn_cast<TemplateParmDeclT>(RecentParm)) { - if (ParmT->hasDefaultArgument()) { - auto *P = cast<TemplateParmDeclT>(NewParm); - P->removeDefaultArgument(); - P->setInheritedDefaultArgument(Importer.ToContext, ParmT); + Error importTemplateParameterDefaultArgument(const TemplateParmDeclT *D, + TemplateParmDeclT *ToD) { + Error Err = Error::success(); + if (D->hasDefaultArgument()) { + if (D->defaultArgumentWasInherited()) { + auto *ToInheritedFrom = const_cast<TemplateParmDeclT *>( + importChecked(Err, D->getDefaultArgStorage().getInheritedFrom())); + if (Err) + return std::move(Err); + if (!ToInheritedFrom->hasDefaultArgument()) { + // Resolve possible circular dependency between default value of the + // template argument and the template declaration. + const auto ToInheritedDefaultArg = + importChecked(Err, D->getDefaultArgStorage() + .getInheritedFrom() + ->getDefaultArgument()); + if (Err) + return std::move(Err); + ToInheritedFrom->setDefaultArgument(Importer.getToContext(), + ToInheritedDefaultArg); + } + ToD->setInheritedDefaultArgument(ToD->getASTContext(), + ToInheritedFrom); + } else { + Expected<TemplateArgumentLoc> ToDefaultArgOrErr = + import(D->getDefaultArgument()); + if (!ToDefaultArgOrErr) + return ToDefaultArgOrErr.takeError(); + // Default argument could have been set in the + // '!ToInheritedFrom->hasDefaultArgument()' branch above. + if (!ToD->hasDefaultArgument()) + ToD->setDefaultArgument(Importer.getToContext(), + *ToDefaultArgOrErr); } } - } - - // Update the parameter list `NewParams` of a template declaration - // by "inheriting" default argument values from `RecentParams`, - // which is the parameter list of an earlier declaration of the - // same template. (Note that "inheriting" default argument values - // is not related to object-oriented inheritance.) - // - // In the clang AST template parameters (NonTypeTemplateParmDec, - // TemplateTypeParmDecl, TemplateTemplateParmDecl) have a reference to the - // default value, if one is specified at the first declaration. The default - // value can be specified only once. The template parameters of the - // 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). 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) { - for (auto [Idx, Param] : enumerate(RecentParams)) { - tryUpdateTemplateParmDeclInheritedFrom<NonTypeTemplateParmDecl>( - Param, NewParams.getParam(Idx)); - tryUpdateTemplateParmDeclInheritedFrom<TemplateTypeParmDecl>( - Param, NewParams.getParam(Idx)); - tryUpdateTemplateParmDeclInheritedFrom<TemplateTemplateParmDecl>( - Param, NewParams.getParam(Idx)); - } + return Err; } public: @@ -5961,23 +5958,8 @@ ASTNodeImporter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) { ToD->setTypeConstraint(ToConceptRef, ToIDC); } - 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 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); - } + if (Error Err = importTemplateParameterDefaultArgument(D, ToD)) + return Err; return ToD; } @@ -6003,14 +5985,9 @@ ASTNodeImporter::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) { D->isParameterPack(), ToTypeSourceInfo)) return ToD; - if (D->hasDefaultArgument()) { - Expected<TemplateArgumentLoc> ToDefaultArgOrErr = - import(D->getDefaultArgument()); - if (!ToDefaultArgOrErr) - return ToDefaultArgOrErr.takeError(); - if (!ToD->hasDefaultArgument()) - ToD->setDefaultArgument(Importer.getToContext(), *ToDefaultArgOrErr); - } + Err = importTemplateParameterDefaultArgument(D, ToD); + if (Err) + return Err; return ToD; } @@ -6041,14 +6018,8 @@ ASTNodeImporter::VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) { *TemplateParamsOrErr)) return ToD; - if (D->hasDefaultArgument()) { - Expected<TemplateArgumentLoc> ToDefaultArgOrErr = - import(D->getDefaultArgument()); - if (!ToDefaultArgOrErr) - return ToDefaultArgOrErr.takeError(); - if (!ToD->hasDefaultArgument()) - ToD->setDefaultArgument(Importer.getToContext(), *ToDefaultArgOrErr); - } + if (Error Err = importTemplateParameterDefaultArgument(D, ToD)) + return Err; return ToD; } @@ -6186,9 +6157,6 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) { } D2->setPreviousDecl(Recent); - - updateTemplateParametersInheritedFrom(*(Recent->getTemplateParameters()), - **TemplateParamsOrErr); } return D2; @@ -6503,9 +6471,6 @@ ExpectedDecl ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) { ToTemplated->setPreviousDecl(PrevTemplated); } ToVarTD->setPreviousDecl(Recent); - - updateTemplateParametersInheritedFrom(*(Recent->getTemplateParameters()), - **TemplateParamsOrErr); } return ToVarTD; @@ -6778,9 +6743,6 @@ ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) { TemplatedFD->setPreviousDecl(PrevTemplated); } ToFunc->setPreviousDecl(Recent); - - updateTemplateParametersInheritedFrom(*(Recent->getTemplateParameters()), - *Params); } return ToFunc; diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 73a5cb541e4d29..ad3193f03cf522 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -9800,62 +9800,59 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportMultipleAnonymousEnumDecls) { struct ImportTemplateParmDeclDefaultValue : public ASTImporterOptionSpecificTestBase { protected: - void checkTemplateParams(RedeclarableTemplateDecl *D) { - auto *CanD = cast<RedeclarableTemplateDecl>(D->getCanonicalDecl()); - auto *CanNonTypeP = cast<NonTypeTemplateParmDecl>( - CanD->getTemplateParameters()->getParam(0)); - auto *CanTypeP = - cast<TemplateTypeParmDecl>(CanD->getTemplateParameters()->getParam(1)); - auto *CanTemplateP = cast<TemplateTemplateParmDecl>( - CanD->getTemplateParameters()->getParam(2)); - EXPECT_FALSE(CanNonTypeP->getDefaultArgStorage().isInherited()); - EXPECT_FALSE(CanTypeP->getDefaultArgStorage().isInherited()); - EXPECT_FALSE(CanTemplateP->getDefaultArgStorage().isInherited()); - for (Decl *Redecl : D->redecls()) { - auto *ReD = cast<RedeclarableTemplateDecl>(Redecl); - if (ReD != CanD) { - auto *NonTypeP = cast<NonTypeTemplateParmDecl>( - ReD->getTemplateParameters()->getParam(0)); - auto *TypeP = cast<TemplateTypeParmDecl>( - ReD->getTemplateParameters()->getParam(1)); - auto *TemplateP = cast<TemplateTemplateParmDecl>( - ReD->getTemplateParameters()->getParam(2)); - EXPECT_TRUE(NonTypeP->getDefaultArgStorage().isInherited()); - EXPECT_TRUE(TypeP->getDefaultArgStorage().isInherited()); - EXPECT_TRUE(TemplateP->getDefaultArgStorage().isInherited()); - EXPECT_EQ(NonTypeP->getDefaultArgStorage().getInheritedFrom(), - CanNonTypeP); - EXPECT_EQ(TypeP->getDefaultArgStorage().getInheritedFrom(), CanTypeP); - EXPECT_EQ(TemplateP->getDefaultArgStorage().getInheritedFrom(), - CanTemplateP); - } + void checkTemplateParams(RedeclarableTemplateDecl *D, + RedeclarableTemplateDecl *InheritedFromD) { + auto *NonTypeP = + cast<NonTypeTemplateParmDecl>(D->getTemplateParameters()->getParam(0)); + auto *TypeP = + cast<TemplateTypeParmDecl>(D->getTemplateParameters()->getParam(1)); + auto *TemplateP = + cast<TemplateTemplateParmDecl>(D->getTemplateParameters()->getParam(2)); + if (InheritedFromD) { + EXPECT_TRUE(NonTypeP->getDefaultArgStorage().isInherited()); + EXPECT_TRUE(TypeP->getDefaultArgStorage().isInherited()); + EXPECT_TRUE(TemplateP->getDefaultArgStorage().isInherited()); + EXPECT_EQ(NonTypeP->getDefaultArgStorage().getInheritedFrom(), + InheritedFromD->getTemplateParameters()->getParam(0)); + EXPECT_EQ(TypeP->getDefaultArgStorage().getInheritedFrom(), + InheritedFromD->getTemplateParameters()->getParam(1)); + EXPECT_EQ(TemplateP->getDefaultArgStorage().getInheritedFrom(), + InheritedFromD->getTemplateParameters()->getParam(2)); + } else { + EXPECT_FALSE(NonTypeP->getDefaultArgStorage().isInherited()); + EXPECT_FALSE(TypeP->getDefaultArgStorage().isInherited()); + EXPECT_FALSE(TemplateP->getDefaultArgStorage().isInherited()); } } - void testImport(RedeclarableTemplateDecl *FromD) { - RedeclarableTemplateDecl *ToD = Import(FromD, Lang_CXX14); - checkTemplateParams(ToD); + void testImport(RedeclarableTemplateDecl *FromD1, + RedeclarableTemplateDecl *FromD2, + RedeclarableTemplateDecl *FromD3, + RedeclarableTemplateDecl *ToExistingD1) { + auto *ToD1 = Import(FromD1, Lang_CXX14); + auto *ToD2 = Import(FromD2, Lang_CXX14); + auto *ToD3 = Import(FromD3, Lang_CXX14); + checkTemplateParams(ToD1, nullptr); + checkTemplateParams(ToD2, ToD1); + checkTemplateParams(ToD3, ToExistingD1 ? ToExistingD1 : ToD1); } - // 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. + // In these tests a circular dependency is created between the template + // parameter default value and the template declaration (with the same + // template parameter). template <class TemplateParmDeclT> - void testImportTemplateParmDeclReversed(TemplateParmDeclT *FromD, - TemplateParmDeclT *FromDInherited) { - ASSERT_FALSE(FromD->getDefaultArgStorage().isInherited()); - ASSERT_TRUE(FromDInherited->getDefaultArgStorage().isInherited()); + void + testTemplateParmDeclCircularDependency(ClassTemplateDecl *FromD, + ClassTemplateDecl *FromDInherited) { + auto GetTemplateParm = + [](ClassTemplateDecl *D) -> const TemplateParmDeclT * { + return dyn_cast<TemplateParmDeclT>( + D->getTemplateParameters()->getParam(0)); + }; + + ASSERT_FALSE(GetTemplateParm(FromD)->getDefaultArgStorage().isInherited()); + ASSERT_TRUE( + GetTemplateParm(FromDInherited)->getDefaultArgStorage().isInherited()); auto *ToD = Import(FromD, Lang_CXX14); EXPECT_TRUE(ToD); @@ -9863,9 +9860,15 @@ struct ImportTemplateParmDeclDefaultValue 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); + EXPECT_FALSE(GetTemplateParm(ToD)->getDefaultArgStorage().isInherited()); + EXPECT_TRUE( + GetTemplateParm(ToDInherited)->getDefaultArgStorage().isInherited()); + EXPECT_EQ(GetTemplateParm(ToDInherited) + ->getDefaultArgStorage() + .getInheritedFrom(), + GetTemplateParm(ToD)); + + EXPECT_EQ(ToD->getPreviousDecl(), ToDInherited); } const char *CodeFunction = @@ -9873,85 +9876,145 @@ struct ImportTemplateParmDeclDefaultValue template <class> struct X; template <int A = 2, typename B = int, template<class> class C = X> - void f(); + void test(); template <int A, typename B, template<class> class C> - void f(); + void test(); template <int A, typename B, template<class> class C> - void f() {} + void test() {} )"; const char *CodeClass = R"( + namespace N { template <class> struct X; template <int A = 2, typename B = int, template<class> class C = X> - struct S; + struct test; template <int A, typename B, template<class> class C> - struct S; + struct test; template <int A, typename B, template<class> class C> - struct S {}; + struct test {}; + } )"; const char *CodeVar = R"( + namespace N { template <class> struct X; template <int A = 2, typename B = int, template<class> class C = X> - extern int V; + extern int test; template <int A, typename B, template<class> class C> - extern int V; + extern int test; template <int A, typename B, template<class> class C> - int V = A; + int test = A; + } )"; }; -TEST_P(ImportTemplateParmDeclDefaultValue, ImportFunctionTemplate) { - Decl *FromTU = getTuDecl(CodeFunction, Lang_CXX14); - auto *FromLastD = LastDeclMatcher<FunctionTemplateDecl>().match( +TEST_P(ImportTemplateParmDeclDefaultValue, InvisibleInheritedFrom) { + const char *ToCode = + R"( + template <int P = 1> + void f() {} + )"; + TranslationUnitDecl *ToTU = getToTuDecl(ToCode, Lang_CXX14); + auto *ToFDef = FirstDeclMatcher<FunctionTemplateDecl>().match( + ToTU, functionTemplateDecl(hasName("f"))); + + const char *FromCode = + R"( + template <int P = 1> + void f() {} + template <int P> + void f(); + )"; + TranslationUnitDecl *FromTU = getTuDecl(FromCode, Lang_CXX14); + auto *FromFDef = FirstDeclMatcher<FunctionTemplateDecl>().match( + FromTU, functionTemplateDecl(hasName("f"))); + auto *FromF = LastDeclMatcher<FunctionTemplateDecl>().match( FromTU, functionTemplateDecl(hasName("f"))); - testImport(FromLastD); + + auto *ToFDefImported = Import(FromFDef, Lang_CXX14); + EXPECT_EQ(ToFDefImported, ToFDef); + auto *ToF = Import(FromF, Lang_CXX14); + EXPECT_NE(ToF, ToFDef); + const auto *Parm = dyn_cast<NonTypeTemplateParmDecl>( + ToF->getTemplateParameters()->getParam(0)); + EXPECT_TRUE(Parm->defaultArgumentWasInherited()); + // FIXME: This behavior may be confusing: + // Default value is not inherited from the existing declaration, instead a new + // is created at import that is similar to the existing but not reachable from + // the AST. + EXPECT_NE(Parm->getDefaultArgStorage().getInheritedFrom(), + ToFDef->getTemplateParameters()->getParam(0)); +} + +TEST_P(ImportTemplateParmDeclDefaultValue, ImportFunctionTemplate) { + TranslationUnitDecl *FromTU = getTuDecl(CodeFunction, Lang_CXX14); + auto *D3 = LastDeclMatcher<FunctionTemplateDecl>().match( + FromTU, functionTemplateDecl(hasName("test") /*, hasBody(stmt())*/)); + auto *D2 = dyn_cast<FunctionTemplateDecl>(D3->getPreviousDecl()); + auto *D1 = dyn_cast<FunctionTemplateDecl>(D2->getPreviousDecl()); + testImport(D1, D2, D3, nullptr); } TEST_P(ImportTemplateParmDeclDefaultValue, ImportExistingFunctionTemplate) { - getToTuDecl(CodeFunction, Lang_CXX14); - Decl *FromTU = getTuDecl(CodeFunction, Lang_CXX14); - auto *FromLastD = LastDeclMatcher<FunctionTemplateDecl>().match( - FromTU, functionTemplateDecl(hasName("f"))); - testImport(FromLastD); + TranslationUnitDecl *ToTU = getToTuDecl(CodeFunction, Lang_CXX14); + auto *ToD1 = FirstDeclMatcher<FunctionTemplateDecl>().match( + ToTU, functionTemplateDecl(hasName("test"))); + TranslationUnitDecl *FromTU = getTuDecl(CodeFunction, Lang_CXX14); + auto *D3 = LastDeclMatcher<FunctionTemplateDecl>().match( + FromTU, functionTemplateDecl(hasName("test"))); + auto *D2 = dyn_cast<FunctionTemplateDecl>(D3->getPreviousDecl()); + auto *D1 = dyn_cast<FunctionTemplateDecl>(D2->getPreviousDecl()); + testImport(D1, D2, D3, ToD1); } TEST_P(ImportTemplateParmDeclDefaultValue, ImportClassTemplate) { - Decl *FromTU = getTuDecl(CodeClass, Lang_CXX14); - auto *FromLastD = LastDeclMatcher<ClassTemplateDecl>().match( - FromTU, classTemplateDecl(hasName("S"))); - testImport(FromLastD); + TranslationUnitDecl *FromTU = getTuDecl(CodeClass, Lang_CXX14); + auto *D3 = LastDeclMatcher<ClassTemplateDecl>().match( + FromTU, classTemplateDecl(hasName("test"))); + auto *D2 = dyn_cast<ClassTemplateDecl>(D3->getPreviousDecl()); + auto *D1 = dyn_cast<ClassTemplateDecl>(D2->getPreviousDecl()); + testImport(D1, D2, D3, nullptr); } TEST_P(ImportTemplateParmDeclDefaultValue, ImportExistingClassTemplate) { - getToTuDecl(CodeClass, Lang_CXX14); - Decl *FromTU = getTuDecl(CodeClass, Lang_CXX14); - auto *FromLastD = LastDeclMatcher<ClassTemplateDecl>().match( - FromTU, classTemplateDecl(hasName("S"))); - testImport(FromLastD); + TranslationUnitDecl *ToTU = getToTuDecl(CodeClass, Lang_CXX14); + auto *ToD1 = FirstDeclMatcher<ClassTemplateDecl>().match( + ToTU, classTemplateDecl(hasName("test"))); + TranslationUnitDecl *FromTU = getTuDecl(CodeClass, Lang_CXX14); + auto *D3 = LastDeclMatcher<ClassTemplateDecl>().match( + FromTU, classTemplateDecl(hasName("test"))); + auto *D2 = dyn_cast<ClassTemplateDecl>(D3->getPreviousDecl()); + auto *D1 = dyn_cast<ClassTemplateDecl>(D2->getPreviousDecl()); + testImport(D1, D2, D3, ToD1); } TEST_P(ImportTemplateParmDeclDefaultValue, ImportVarTemplate) { - Decl *FromTU = getTuDecl(CodeVar, Lang_CXX14); - auto *FromLastD = LastDeclMatcher<VarTemplateDecl>().match( - FromTU, varTemplateDecl(hasName("V"))); - testImport(FromLastD); + TranslationUnitDecl *FromTU = getTuDecl(CodeVar, Lang_CXX14); + auto *D3 = LastDeclMatcher<VarTemplateDecl>().match( + FromTU, varTemplateDecl(hasName("test"))); + auto *D2 = dyn_cast<VarTemplateDecl>(D3->getPreviousDecl()); + auto *D1 = dyn_cast<VarTemplateDecl>(D2->getPreviousDecl()); + testImport(D1, D2, D3, nullptr); } TEST_P(ImportTemplateParmDeclDefaultValue, ImportExistingVarTemplate) { - getToTuDecl(CodeVar, Lang_CXX14); - Decl *FromTU = getTuDecl(CodeVar, Lang_CXX14); - auto *FromLastD = LastDeclMatcher<VarTemplateDecl>().match( - FromTU, varTemplateDecl(hasName("V"))); - testImport(FromLastD); + TranslationUnitDecl *ToTU = getToTuDecl(CodeVar, Lang_CXX14); + auto *ToD1 = FirstDeclMatcher<VarTemplateDecl>().match( + ToTU, varTemplateDecl(hasName("test"))); + TranslationUnitDecl *FromTU = getTuDecl(CodeVar, Lang_CXX14); + auto *D3 = LastDeclMatcher<VarTemplateDecl>().match( + FromTU, varTemplateDecl(hasName("test"))); + auto *D2 = dyn_cast<VarTemplateDecl>(D3->getPreviousDecl()); + auto *D1 = dyn_cast<VarTemplateDecl>(D2->getPreviousDecl()); + testImport(D1, D2, D3, ToD1); } TEST_P(ImportTemplateParmDeclDefaultValue, - ImportNonTypeTemplateParmDeclReversed) { + NonTypeTemplateParmDeclCircularDependency) { const char *Code = R"( struct Z; @@ -9976,15 +10039,17 @@ TEST_P(ImportTemplateParmDeclDefaultValue, )"; Decl *FromTU = getTuDecl(Code, Lang_CXX14); - auto *FromD = FirstDeclMatcher<NonTypeTemplateParmDecl>().match( - FromTU, nonTypeTemplateParmDecl(hasName("P1"))); - auto *FromDInherited = FirstDeclMatcher<NonTypeTemplateParmDecl>().match( - FromTU, nonTypeTemplateParmDecl(hasName("P2"))); + auto *FromD = FirstDeclMatcher<ClassTemplateDecl>().match( + FromTU, classTemplateDecl(hasName("X"))); + auto *FromDInherited = LastDeclMatcher<ClassTemplateDecl>().match( + FromTU, classTemplateDecl(hasName("X"))); - testImportTemplateParmDeclReversed(FromD, FromDInherited); + testTemplateParmDeclCircularDependency<NonTypeTemplateParmDecl>( + FromD, FromDInherited); } -TEST_P(ImportTemplateParmDeclDefaultValue, ImportTemplateTypeParmDeclReversed) { +TEST_P(ImportTemplateParmDeclDefaultValue, + TemplateTypeParmDeclCircularDependency) { const char *Code = R"( struct Z; @@ -10008,16 +10073,17 @@ TEST_P(ImportTemplateParmDeclDefaultValue, ImportTemplateTypeParmDeclReversed) { )"; Decl *FromTU = getTuDecl(Code, Lang_CXX14); - auto *FromD = FirstDeclMatcher<TemplateTypeParmDecl>().match( - FromTU, templateTypeParmDecl(hasName("T1"))); - auto *FromDInherited = FirstDeclMatcher<TemplateTypeParmDecl>().match( - FromTU, templateTypeParmDecl(hasName("T2"))); + auto *FromD = FirstDeclMatcher<ClassTemplateDecl>().match( + FromTU, classTemplateDecl(hasName("X"))); + auto *FromDInherited = LastDeclMatcher<ClassTemplateDecl>().match( + FromTU, classTemplateDecl(hasName("X"))); - testImportTemplateParmDeclReversed(FromD, FromDInherited); + testTemplateParmDeclCircularDependency<TemplateTypeParmDecl>(FromD, + FromDInherited); } TEST_P(ImportTemplateParmDeclDefaultValue, - ImportTemplateTemplateParmDeclReversed) { + TemplateTemplateParmDeclCircularDependency) { const char *Code = R"( struct Z; @@ -10042,12 +10108,13 @@ TEST_P(ImportTemplateParmDeclDefaultValue, )"; Decl *FromTU = getTuDecl(Code, Lang_CXX14); - auto *FromD = FirstDeclMatcher<TemplateTemplateParmDecl>().match( - FromTU, templateTemplateParmDecl(hasName("T1"))); - auto *FromDInherited = FirstDeclMatcher<TemplateTemplateParmDecl>().match( - FromTU, templateTemplateParmDecl(hasName("T2"))); + auto *FromD = FirstDeclMatcher<ClassTemplateDecl>().match( + FromTU, classTemplateDecl(hasName("X"))); + auto *FromDInherited = LastDeclMatcher<ClassTemplateDecl>().match( + FromTU, classTemplateDecl(hasName("X"))); - testImportTemplateParmDeclReversed(FromD, FromDInherited); + testTemplateParmDeclCircularDependency<TemplateTemplateParmDecl>( + 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