[clang] [clang][ASTImporter] Fix import of template parameter default values. (PR #100100)

Balázs Kéri via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 29 01:59:18 PDT 2024


https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/100100

>From e4440b82f3d1fe5c7cafbad87da0e266d35a619e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Tue, 23 Jul 2024 11:20:22 +0200
Subject: [PATCH 1/2] [clang][ASTImporter] Fix import of template parameter
 default values.

Default values of template parameters (non-type, type, template)
were not correctly handled in the "inherited" case. This occurs
if the first declaration contains the default value but a next
one not. The default value is "inherited" from the first.

In ASTImporter it was only possible to set the inherited status
after the template object was created with the template parameters
that were imported without handling the inherited case. The
import function of the template parameter contains not enough
information (previous declaration) to set the inherited-from status.
After the template was created, default value of the parameters
that should be inherited is reset to inherited mode.
---
 clang/lib/AST/ASTImporter.cpp           |  34 +++++++
 clang/unittests/AST/ASTImporterTest.cpp | 125 ++++++++++++++++++++++++
 2 files changed, 159 insertions(+)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 08ef09d353afc..1d9ea714780ce 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -359,6 +359,31 @@ namespace clang {
           Params, Importer.getToContext().getTranslationUnitDecl());
     }
 
+    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);
+        }
+      }
+    }
+
+    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));
+      }
+    }
+
   public:
     explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
 
@@ -6138,6 +6163,9 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
     }
 
     D2->setPreviousDecl(Recent);
+
+    updateTemplateParametersInheritedFrom(*(Recent->getTemplateParameters()),
+                                          **TemplateParamsOrErr);
   }
 
   return D2;
@@ -6452,6 +6480,9 @@ ExpectedDecl ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) {
         ToTemplated->setPreviousDecl(PrevTemplated);
     }
     ToVarTD->setPreviousDecl(Recent);
+
+    updateTemplateParametersInheritedFrom(*(Recent->getTemplateParameters()),
+                                          **TemplateParamsOrErr);
   }
 
   return ToVarTD;
@@ -6724,6 +6755,9 @@ 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 6d987cc7e9ec6..ed64593ae2f0b 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9783,6 +9783,128 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportExistingEmptyAnonymousEnums) {
   EXPECT_EQ(ImportedE2, ToE1);
 }
 
+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 testImport(RedeclarableTemplateDecl *FromD) {
+    RedeclarableTemplateDecl *ToD = Import(FromD, Lang_CXX14);
+    checkTemplateParams(ToD);
+  }
+
+  const char *CodeFunction =
+      R"(
+      template <class> struct X;
+
+      template <int A = 2, typename B = int, template<class> class C = X>
+      void f();
+      template <int A, typename B, template<class> class C>
+      void f();
+      template <int A, typename B, template<class> class C>
+      void f() {}
+      )";
+
+  const char *CodeClass =
+      R"(
+      template <class> struct X;
+
+      template <int A = 2, typename B = int, template<class> class C = X>
+      struct S;
+      template <int A, typename B, template<class> class C>
+      struct S;
+      template <int A, typename B, template<class> class C>
+      struct S {};
+      )";
+
+  const char *CodeVar =
+      R"(
+      template <class> struct X;
+
+      template <int A = 2, typename B = int, template<class> class C = X>
+      extern int V;
+      template <int A, typename B, template<class> class C>
+      extern int V;
+      template <int A, typename B, template<class> class C>
+      int V = A;
+      )";
+};
+
+TEST_P(ImportTemplateParmDeclDefaultValue, ImportFunctionTemplate) {
+  Decl *FromTU = getTuDecl(CodeFunction, Lang_CXX14);
+  auto *FromLastD = LastDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU, functionTemplateDecl(hasName("f")));
+  testImport(FromLastD);
+}
+
+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);
+}
+
+TEST_P(ImportTemplateParmDeclDefaultValue, ImportClassTemplate) {
+  Decl *FromTU = getTuDecl(CodeClass, Lang_CXX14);
+  auto *FromLastD = LastDeclMatcher<ClassTemplateDecl>().match(
+      FromTU, classTemplateDecl(hasName("S")));
+  testImport(FromLastD);
+}
+
+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);
+}
+
+TEST_P(ImportTemplateParmDeclDefaultValue, ImportVarTemplate) {
+  Decl *FromTU = getTuDecl(CodeVar, Lang_CXX14);
+  auto *FromLastD = LastDeclMatcher<VarTemplateDecl>().match(
+      FromTU, varTemplateDecl(hasName("V")));
+  testImport(FromLastD);
+}
+
+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);
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
                          DefaultTestValuesForRunOptions);
 
@@ -9866,6 +9988,9 @@ INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportInjectedClassNameType,
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportMatrixType,
                          DefaultTestValuesForRunOptions);
 
+INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportTemplateParmDeclDefaultValue,
+                         DefaultTestValuesForRunOptions);
+
 // FIXME: Make ImportOpenCLPipe test work.
 // INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportOpenCLPipe,
 //                          DefaultTestValuesForRunOptions);

>From 24204afd6fdda9297a8224c57b4be570354fb580 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Wed, 24 Jul 2024 15:40:33 +0200
Subject: [PATCH 2/2] added explanation comment

---
 clang/lib/AST/ASTImporter.cpp | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 1d9ea714780ce..b205697d09c28 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -371,6 +371,29 @@ namespace clang {
       }
     }
 
+    // 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).
+    //
+    // 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.
     void updateTemplateParametersInheritedFrom(
         const TemplateParameterList &RecentParams,
         TemplateParameterList &NewParams) {



More information about the cfe-commits mailing list