[clang] 341a30a - [clang][ASTImporter] Update lookup table correctly at deduction guides.

Gabor Marton via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 6 11:45:53 PST 2021


Author: Balázs Kéri
Date: 2021-12-06T20:40:16+01:00
New Revision: 341a30a4ba4bdb537916986cbc0bac8b5b31d8f6

URL: https://github.com/llvm/llvm-project/commit/341a30a4ba4bdb537916986cbc0bac8b5b31d8f6
DIFF: https://github.com/llvm/llvm-project/commit/341a30a4ba4bdb537916986cbc0bac8b5b31d8f6.diff

LOG: [clang][ASTImporter] Update lookup table correctly at deduction guides.

Declaration context of template parameters of a FunctionTemplateDecl
may be different for each one parameter if the template is a
deduction guide. This case is handled correctly after this change.

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D114418

Added: 
    

Modified: 
    clang/include/clang/AST/ASTImporterLookupTable.h
    clang/lib/AST/ASTImporter.cpp
    clang/lib/AST/ASTImporterLookupTable.cpp
    clang/unittests/AST/ASTImporterTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ASTImporterLookupTable.h b/clang/include/clang/AST/ASTImporterLookupTable.h
index 47dca20338393..918c2b9e350cd 100644
--- a/clang/include/clang/AST/ASTImporterLookupTable.h
+++ b/clang/include/clang/AST/ASTImporterLookupTable.h
@@ -75,6 +75,10 @@ class ASTImporterLookupTable {
   // The function should be called when the old context is definitely 
diff erent
   // from the new.
   void update(NamedDecl *ND, DeclContext *OldDC);
+  // Same as 'update' but allow if 'ND' is not in the table or the old context
+  // is the same as the new.
+  // FIXME: The old redeclaration context is not handled.
+  void updateForced(NamedDecl *ND, DeclContext *OldDC);
   using LookupResult = DeclList;
   LookupResult lookup(DeclContext *DC, DeclarationName Name) const;
   // Check if the `ND` is within the lookup table (with its current name) in

diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 710e40bbb4b72..13490f1cf89d7 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6066,20 +6066,24 @@ ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
   if (Error Err = importInto(TemplatedFD, D->getTemplatedDecl()))
     return std::move(Err);
 
-  // Template parameters of the ClassTemplateDecl and FunctionTemplateDecl are
-  // shared, if the FunctionTemplateDecl is a deduction guide for the class.
-  // At import the ClassTemplateDecl object is always created first (FIXME: is
-  // this really true?) because the dependency, then the FunctionTemplateDecl.
-  // The DeclContext of the template parameters is changed when the
-  // FunctionTemplateDecl is created, but was set already when the class
-  // template was created. So here it is not the TU (default value) any more.
-  // FIXME: The DeclContext of the parameters is now set finally to the
-  // CXXDeductionGuideDecl object that was imported later. This may not be the
-  // same that is in the original AST, specially if there are multiple deduction
-  // guides.
-  DeclContext *OldParamDC = nullptr;
-  if (Params->size() > 0)
-    OldParamDC = Params->getParam(0)->getDeclContext();
+  // At creation of the template the template parameters are "adopted"
+  // (DeclContext is changed). After this possible change the lookup table
+  // must be updated.
+  // At deduction guides the DeclContext of the template parameters may be
+  // 
diff erent from what we would expect, it may be the class template, or a
+  // probably 
diff erent CXXDeductionGuideDecl. This may come from the fact that
+  // the template parameter objects may be shared between deduction guides or
+  // the class template, and at creation of multiple FunctionTemplateDecl
+  // objects (for deduction guides) the same parameters are re-used. The
+  // "adoption" happens multiple times with 
diff erent parent, even recursively
+  // for TemplateTemplateParmDecl. The same happens at import when the
+  // FunctionTemplateDecl objects are created, but in 
diff erent order.
+  // In this way the DeclContext of these template parameters is not necessarily
+  // the same as in the "from" context.
+  SmallVector<DeclContext *, 2> OldParamDC;
+  OldParamDC.reserve(Params->size());
+  llvm::transform(*Params, std::back_inserter(OldParamDC),
+                  [](NamedDecl *ND) { return ND->getDeclContext(); });
 
   FunctionTemplateDecl *ToFunc;
   if (GetImportedOrCreateDecl(ToFunc, D, Importer.getToContext(), DC, Loc, Name,
@@ -6091,7 +6095,12 @@ ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
   ToFunc->setAccess(D->getAccess());
   ToFunc->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(ToFunc);
-  updateLookupTableForTemplateParameters(*Params, OldParamDC);
+
+  ASTImporterLookupTable *LT = Importer.SharedState->getLookupTable();
+  if (LT && !OldParamDC.empty()) {
+    for (unsigned int I = 0; I < OldParamDC.size(); ++I)
+      LT->updateForced(Params->getParam(I), OldParamDC[I]);
+  }
 
   if (FoundByLookup) {
     auto *Recent =

diff  --git a/clang/lib/AST/ASTImporterLookupTable.cpp b/clang/lib/AST/ASTImporterLookupTable.cpp
index ef42561c6f941..b7d17a5e92d00 100644
--- a/clang/lib/AST/ASTImporterLookupTable.cpp
+++ b/clang/lib/AST/ASTImporterLookupTable.cpp
@@ -140,6 +140,11 @@ void ASTImporterLookupTable::update(NamedDecl *ND, DeclContext *OldDC) {
   add(ND);
 }
 
+void ASTImporterLookupTable::updateForced(NamedDecl *ND, DeclContext *OldDC) {
+  LookupTable[OldDC][ND->getDeclName()].remove(ND);
+  add(ND);
+}
+
 ASTImporterLookupTable::LookupResult
 ASTImporterLookupTable::lookup(DeclContext *DC, DeclarationName Name) const {
   auto DCI = LookupTable.find(DC->getPrimaryContext());

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 29b1500fcd76d..83ccdb5caeaff 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -7329,6 +7329,143 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportUsingShadowList) {
   EXPECT_EQ(*ShadowI, ToUsingShadowF2);
 }
 
+AST_MATCHER_P(FunctionTemplateDecl, templateParameterCountIs, unsigned, Cnt) {
+  return Node.getTemplateParameters()->size() == Cnt;
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportDeductionGuide) {
+  TranslationUnitDecl *FromTU = getTuDecl(
+      R"(
+      template<class> class A { };
+      template<class T> class B {
+          template<class T1, typename = A<T>> B(T1);
+      };
+      template<class T>
+      B(T, T) -> B<int>;
+      )",
+      Lang_CXX17);
+
+  // Get the implicit deduction guide for (non-default) constructor of 'B'.
+  auto *FromDGCtor = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU, functionTemplateDecl(templateParameterCountIs(3)));
+  // Implicit deduction guide for copy constructor of 'B'.
+  auto *FromDGCopyCtor = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU, functionTemplateDecl(templateParameterCountIs(1), isImplicit()));
+  // User defined deduction guide.
+  auto *FromDGOther = FirstDeclMatcher<CXXDeductionGuideDecl>().match(
+      FromTU, cxxDeductionGuideDecl(unless(isImplicit())));
+
+  TemplateParameterList *FromDGCtorTP = FromDGCtor->getTemplateParameters();
+  // Don't know why exactly but this is the DeclContext here.
+  EXPECT_EQ(FromDGCtorTP->getParam(0)->getDeclContext(),
+            FromDGCopyCtor->getTemplatedDecl());
+  EXPECT_EQ(FromDGCtorTP->getParam(1)->getDeclContext(),
+            FromDGCtor->getTemplatedDecl());
+  EXPECT_EQ(FromDGCtorTP->getParam(2)->getDeclContext(),
+            FromDGCtor->getTemplatedDecl());
+  EXPECT_EQ(
+      FromDGCopyCtor->getTemplateParameters()->getParam(0)->getDeclContext(),
+      FromDGCopyCtor->getTemplatedDecl());
+  EXPECT_EQ(FromDGOther->getDescribedTemplate()
+                ->getTemplateParameters()
+                ->getParam(0)
+                ->getDeclContext(),
+            FromDGOther);
+
+  auto *ToDGCtor = Import(FromDGCtor, Lang_CXX17);
+  auto *ToDGCopyCtor = Import(FromDGCopyCtor, Lang_CXX17);
+  auto *ToDGOther = Import(FromDGOther, Lang_CXX17);
+  ASSERT_TRUE(ToDGCtor);
+  ASSERT_TRUE(ToDGCopyCtor);
+  ASSERT_TRUE(ToDGOther);
+
+  TemplateParameterList *ToDGCtorTP = ToDGCtor->getTemplateParameters();
+  EXPECT_EQ(ToDGCtorTP->getParam(0)->getDeclContext(),
+            ToDGCopyCtor->getTemplatedDecl());
+  EXPECT_EQ(ToDGCtorTP->getParam(1)->getDeclContext(),
+            ToDGCtor->getTemplatedDecl());
+  EXPECT_EQ(ToDGCtorTP->getParam(2)->getDeclContext(),
+            ToDGCtor->getTemplatedDecl());
+  EXPECT_EQ(
+      ToDGCopyCtor->getTemplateParameters()->getParam(0)->getDeclContext(),
+      ToDGCopyCtor->getTemplatedDecl());
+  EXPECT_EQ(ToDGOther->getDescribedTemplate()
+                ->getTemplateParameters()
+                ->getParam(0)
+                ->getDeclContext(),
+            ToDGOther);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportDeductionGuideDifferentOrder) {
+  // This test demonstrates that the DeclContext of the imported object is
+  // dependent on the order of import. The test is an exact copy of the previous
+  // one except at the indicated locations.
+  TranslationUnitDecl *FromTU = getTuDecl(
+      R"(
+      template<class> class A { };
+      template<class T> class B {
+          template<class T1, typename = A<T>> B(T1);
+      };
+      template<class T>
+      B(T, T) -> B<int>;
+      )",
+      Lang_CXX17);
+
+  // Get the implicit deduction guide for (non-default) constructor of 'B'.
+  auto *FromDGCtor = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU, functionTemplateDecl(templateParameterCountIs(3)));
+  // Implicit deduction guide for copy constructor of 'B'.
+  auto *FromDGCopyCtor = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU, functionTemplateDecl(templateParameterCountIs(1), isImplicit()));
+  // User defined deduction guide.
+  auto *FromDGOther = FirstDeclMatcher<CXXDeductionGuideDecl>().match(
+      FromTU, cxxDeductionGuideDecl(unless(isImplicit())));
+
+  TemplateParameterList *FromDGCtorTP = FromDGCtor->getTemplateParameters();
+  // Don't know why exactly but this is the DeclContext here.
+  EXPECT_EQ(FromDGCtorTP->getParam(0)->getDeclContext(),
+            FromDGCopyCtor->getTemplatedDecl());
+  EXPECT_EQ(FromDGCtorTP->getParam(1)->getDeclContext(),
+            FromDGCtor->getTemplatedDecl());
+  EXPECT_EQ(FromDGCtorTP->getParam(2)->getDeclContext(),
+            FromDGCtor->getTemplatedDecl());
+  EXPECT_EQ(
+      FromDGCopyCtor->getTemplateParameters()->getParam(0)->getDeclContext(),
+      FromDGCopyCtor->getTemplatedDecl());
+  EXPECT_EQ(FromDGOther->getDescribedTemplate()
+                ->getTemplateParameters()
+                ->getParam(0)
+                ->getDeclContext(),
+            FromDGOther);
+
+  // Here the import of 'ToDGCopyCtor' and 'ToDGCtor' is reversed relative to
+  // the previous test.
+  auto *ToDGCopyCtor = Import(FromDGCopyCtor, Lang_CXX17);
+  auto *ToDGCtor = Import(FromDGCtor, Lang_CXX17);
+  auto *ToDGOther = Import(FromDGOther, Lang_CXX17);
+  ASSERT_TRUE(ToDGCtor);
+  ASSERT_TRUE(ToDGCopyCtor);
+  ASSERT_TRUE(ToDGOther);
+
+  TemplateParameterList *ToDGCtorTP = ToDGCtor->getTemplateParameters();
+  // Next line: DeclContext is 
diff erent relative to the previous test.
+  EXPECT_EQ(ToDGCtorTP->getParam(0)->getDeclContext(),
+            ToDGCtor->getTemplatedDecl());
+  EXPECT_EQ(ToDGCtorTP->getParam(1)->getDeclContext(),
+            ToDGCtor->getTemplatedDecl());
+  EXPECT_EQ(ToDGCtorTP->getParam(2)->getDeclContext(),
+            ToDGCtor->getTemplatedDecl());
+  // Next line: DeclContext is 
diff erent relative to the previous test.
+  EXPECT_EQ(
+      ToDGCopyCtor->getTemplateParameters()->getParam(0)->getDeclContext(),
+      ToDGCtor->getTemplatedDecl());
+  EXPECT_EQ(ToDGOther->getDescribedTemplate()
+                ->getTemplateParameters()
+                ->getParam(0)
+                ->getDeclContext(),
+            ToDGOther);
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
                          DefaultTestValuesForRunOptions);
 


        


More information about the cfe-commits mailing list