[clang][AST] Set correct DeclContext in ASTImporter lookup table for template params.

[balazske]

Summary

Template parameters are created in ASTImporter with the translation unit as DeclContext.
The DeclContext is later updated (by the create function of template classes).
ASTImporterLookupTable was not updated after these changes of the DC. The patch
adds update of the DeclContext in ASTImporterLookupTable.

[shafik]

clang/lib/AST/ASTImporter.cpp

6049

Is this case covered in the tests?

[balazske]

Add test for deduction guides.

[balazske]

clang/lib/AST/ASTImporter.cpp

6049

This test was left out but added now. But test CTADImplicit does fail if the import of FunctionTemplateDecl is handled similarly as the other cases.

[balazske]

clang/unittests/AST/ASTImporterTest.cpp

4482

This should not have been reformatted.

[balazske]

Added AST dump without clang-format into test code.

[martong]

clang/lib/AST/ASTImporter.cpp

6005–6006

Do you think that the alternative approach that is used with TypedefNameDecl could work here?

// Do not import the DeclContext, we will import it once the TypedefNameDecl
// is created.
if (Error Err = ImportDeclParts(D, Name, ToD, Loc))
  return std::move(Err);

With that perhaps we could avoid the hassle with the update. Could you please investigate that?

6055

typo: is is -> it is

[balazske]

clang/lib/AST/ASTImporter.cpp

6005–6006

The problem is not with the DC of the FunctionTemplateDecl, it is with the template parameters. It looks not safe to pass nullptr as DeclContext for these, this is only possible if the import is done. (Probably the solution in TypedefNameDecl is not perfect: If something exists already in the "To TU" that was not imported, specially a namespace that contains a typedef with same name, it may not be found before the import.)

[martong]

Ok, looks good to me! Thanks!

[balazske]

Fixed typo error in comment.

clang/lib/AST/ASTImporter.cpp

void addDeclToContexts(Decl *FromD, Decl *ToD) {
             if (ND == FromNamed) {
               ToDC->makeDeclVisibleInContext(ToNamed);
               break;
             }
         }
       }
     }
 
+    void updateLookupTableForTemplateParameters(TemplateParameterList &Params,
+                                                DeclContext *OldDC) {
+      ASTImporterLookupTable *LT = Importer.SharedState->getLookupTable();
+      if (!LT)
+        return;
+
+      for (NamedDecl *TP : Params)
+        LT->update(TP, OldDC);
+    }
+
+    void updateLookupTableForTemplateParameters(TemplateParameterList &Params) {
+      updateLookupTableForTemplateParameters(
+          Params, Importer.getToContext().getTranslationUnitDecl());
+    }
+
   public:
     explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
 
     using TypeVisitor<ASTNodeImporter, ExpectedType>::Visit;
     using DeclVisitor<ASTNodeImporter, ExpectedDecl>::Visit;
     using StmtVisitor<ASTNodeImporter, ExpectedStmt>::Visit;
 
     // Importing types
if (GetImportedOrCreateDecl(ToAlias, D, Importer.getToContext(), DC, Loc,
                               Name, ToTemplateParameters, ToTemplatedDecl))
     return ToAlias;
 
   ToTemplatedDecl->setDescribedAliasTemplate(ToAlias);
 
   ToAlias->setAccess(D->getAccess());
   ToAlias->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(ToAlias);
+  if (DC != Importer.getToContext().getTranslationUnitDecl())
+    updateLookupTableForTemplateParameters(*ToTemplateParameters);
   return ToAlias;
 }
 
 ExpectedDecl ASTNodeImporter::VisitLabelDecl(LabelDecl *D) {
   // Import the major distinguishing characteristics of this label.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
   SourceLocation Loc;
if (GetImportedOrCreateDecl(D2, D, Importer.getToContext(), DC, Loc, Name,
     return D2;
 
   ToTemplated->setDescribedClassTemplate(D2);
 
   D2->setAccess(D->getAccess());
   D2->setLexicalDeclContext(LexicalDC);
 
   addDeclToContexts(D, D2);
+  updateLookupTableForTemplateParameters(**TemplateParamsOrErr);
 
   if (FoundByLookup) {
     auto *Recent =
         const_cast<ClassTemplateDecl *>(FoundByLookup->getMostRecentDecl());
 
     // It is possible that during the import of the class template definition
     // we start the import of a fwd friend decl of the very same class template
     // and we add the fwd friend decl to the lookup table. But the ToTemplated
if (PartialSpec) {
     // Update InsertPos, because preceding import calls may have invalidated
     // it by adding new specializations.
     auto *PartSpec2 = cast<ClassTemplatePartialSpecializationDecl>(D2);
     if (!ClassTemplate->findPartialSpecialization(TemplateArgs, ToTPList,
                                                   InsertPos))
       // Add this partial specialization to the class template.
       ClassTemplate->AddPartialSpecialization(PartSpec2, InsertPos);
 
+    updateLookupTableForTemplateParameters(*ToTPList);
   } else { // Not a partial specialization.
     if (GetImportedOrCreateDecl(
             D2, D, Importer.getToContext(), D->getTagKind(), DC,
             *BeginLocOrErr, *IdLocOrErr, ClassTemplate, TemplateArgs,
             PrevDecl))
       return D2;
 
     // Update InsertPos, because preceding import calls may have invalidated
if (GetImportedOrCreateDecl(ToVarTD, D, Importer.getToContext(), DC, Loc,
                               Name, *TemplateParamsOrErr, ToTemplated))
     return ToVarTD;
 
   ToTemplated->setDescribedVarTemplate(ToVarTD);
 
   ToVarTD->setAccess(D->getAccess());
   ToVarTD->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(ToVarTD);
+  if (DC != Importer.getToContext().getTranslationUnitDecl())
+    updateLookupTableForTemplateParameters(**TemplateParamsOrErr);
 
   if (FoundByLookup) {
     auto *Recent =
         const_cast<VarTemplateDecl *>(FoundByLookup->getMostRecentDecl());
     if (!ToTemplated->getPreviousDecl()) {
       auto *PrevTemplated =
           FoundByLookup->getTemplatedDecl()->getMostRecentDecl();
       if (ToTemplated != PrevTemplated)
if (auto *FromPartial = dyn_cast<PartVarSpecDecl>(D)) {
       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,
                                   T, *TInfoOrErr,
                                   D->getStorageClass(), TemplateArgs))
         return D2;
     }
 

 
 ExpectedDecl
 ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
   SourceLocation Loc;
   NamedDecl *ToD;
 
   if (Error Err = ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
     return std::move(Err);
 
   if (ToD)
     return ToD;
 
   const FunctionTemplateDecl *FoundByLookup = nullptr;
 
   // Try to find a function in our own ("to") context with the same name, same
   // type, and in the same context as the function we're importing.
for (auto *FoundDecl : FoundDecls) {
         }
       }
     }
   }
 
   auto ParamsOrErr = import(D->getTemplateParameters());
   if (!ParamsOrErr)
     return ParamsOrErr.takeError();
+  TemplateParameterList *Params = *ParamsOrErr;
 
   FunctionDecl *TemplatedFD;
   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();
+
   FunctionTemplateDecl *ToFunc;
   if (GetImportedOrCreateDecl(ToFunc, D, Importer.getToContext(), DC, Loc, Name,
-                              *ParamsOrErr, TemplatedFD))
+                              Params, TemplatedFD))
     return ToFunc;
 
   TemplatedFD->setDescribedFunctionTemplate(ToFunc);
 
   ToFunc->setAccess(D->getAccess());
   ToFunc->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(ToFunc);
+  updateLookupTableForTemplateParameters(*Params, OldParamDC);
 
   if (FoundByLookup) {
     auto *Recent =
         const_cast<FunctionTemplateDecl *>(FoundByLookup->getMostRecentDecl());
     if (!TemplatedFD->getPreviousDecl()) {
       assert(FoundByLookup->getTemplatedDecl() &&
              "Found decl must have its templated decl set");
       auto *PrevTemplated =

clang/unittests/AST/ASTImporterTest.cpp

auto *FromD = FirstDeclMatcher<FunctionDecl>().match(FromTU,
       functionDecl(hasName("f"), isExplicitTemplateSpecialization()));
   ASSERT_EQ(FromD->getNumTemplateParameterLists(), 1u);
 
   auto *ToD = Import(FromD, Lang_CXX03);
   // The template parameter list should exist.
   EXPECT_EQ(ToD->getNumTemplateParameterLists(), 1u);
 }
 
+const internal::VariadicDynCastAllOfMatcher<Decl, VarTemplateDecl>
+    varTemplateDecl;
+
+const internal::VariadicDynCastAllOfMatcher<
+    Decl, VarTemplatePartialSpecializationDecl>
+    varTemplatePartialSpecializationDecl;
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+       FunctionTemplateParameterDeclContext) {
+  constexpr auto Code =
+      R"(
+      template<class T>
+      void f() {};
+      )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+
+  auto *FromD = FirstDeclMatcher<FunctionTemplateDecl>().match(
+      FromTU, functionTemplateDecl(hasName("f")));
+
+  ASSERT_EQ(FromD->getTemplateParameters()->getParam(0)->getDeclContext(),
+            FromD->getTemplatedDecl());
+
+  auto *ToD = Import(FromD, Lang_CXX11);
+  EXPECT_EQ(ToD->getTemplateParameters()->getParam(0)->getDeclContext(),
+            ToD->getTemplatedDecl());
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+      ToD->getTemplatedDecl(), ToD->getTemplateParameters()->getParam(0)));
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ClassTemplateParameterDeclContext) {
+  constexpr auto Code =
+      R"(
+      template<class T1, class T2>
+      struct S {};
+      template<class T2>
+      struct S<int, T2> {};
+      )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+
+  auto *FromD = FirstDeclMatcher<ClassTemplateDecl>().match(
+      FromTU, classTemplateDecl(hasName("S")));
+  auto *FromDPart =
+      FirstDeclMatcher<ClassTemplatePartialSpecializationDecl>().match(
+          FromTU, classTemplatePartialSpecializationDecl(hasName("S")));
+
+  ASSERT_EQ(FromD->getTemplateParameters()->getParam(0)->getDeclContext(),
+            FromD->getTemplatedDecl());
+  ASSERT_EQ(FromDPart->getTemplateParameters()->getParam(0)->getDeclContext(),
+            FromDPart);
+
+  auto *ToD = Import(FromD, Lang_CXX11);
+  auto *ToDPart = Import(FromDPart, Lang_CXX11);
+
+  EXPECT_EQ(ToD->getTemplateParameters()->getParam(0)->getDeclContext(),
+            ToD->getTemplatedDecl());
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+      ToD->getTemplatedDecl(), ToD->getTemplateParameters()->getParam(0)));
+
+  EXPECT_EQ(ToDPart->getTemplateParameters()->getParam(0)->getDeclContext(),
+            ToDPart);
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+      ToDPart, ToDPart->getTemplateParameters()->getParam(0)));
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+       CXXDeductionGuideTemplateParameterDeclContext) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      template <typename T> struct A {
+        A(T);
+      };
+      A a{(int)0};
+      )",
+      Lang_CXX17, "input.cc");
+// clang-format off
+/*
+|-ClassTemplateDecl 0x1fe5000 <input.cc:2:7, line:4:7> line:2:36 A
+| |-TemplateTypeParmDecl 0x1fe4eb0 <col:17, col:26> col:26 referenced typename depth 0 index 0 T
+| |-CXXRecordDecl 0x1fe4f70 <col:29, line:4:7> line:2:36 struct A definition
+
+|-FunctionTemplateDecl 0x1fe5860 <line:2:7, line:3:12> col:9 implicit <deduction guide for A>
+| |-TemplateTypeParmDecl 0x1fe4eb0 <line:2:17, col:26> col:26 referenced typename depth 0 index 0 T
+| |-CXXDeductionGuideDecl 0x1fe57a8 <line:3:9, col:12> col:9 implicit <deduction guide for A> 'auto (T) -> A<T>'
+| | `-ParmVarDecl 0x1fe56b0 <col:11> col:12 'T'
+| `-CXXDeductionGuideDecl 0x20515d8 <col:9, col:12> col:9 implicit used <deduction guide for A> 'auto (int) -> A<int>'
+|   |-TemplateArgument type 'int'
+|   | `-BuiltinType 0x20587e0 'int'
+|   `-ParmVarDecl 0x2051388 <col:11> col:12 'int':'int'
+`-FunctionTemplateDecl 0x1fe5a78 <line:2:7, col:36> col:36 implicit <deduction guide for A>
+  |-TemplateTypeParmDecl 0x1fe4eb0 <col:17, col:26> col:26 referenced typename depth 0 index 0 T
+  `-CXXDeductionGuideDecl 0x1fe59c0 <col:36> col:36 implicit <deduction guide for A> 'auto (A<T>) -> A<T>'
+    `-ParmVarDecl 0x1fe5958 <col:36> col:36 'A<T>'
+*/
+// clang-format on
+  auto *FromD1 = FirstDeclMatcher<CXXDeductionGuideDecl>().match(
+      FromTU, cxxDeductionGuideDecl());
+  auto *FromD2 = LastDeclMatcher<CXXDeductionGuideDecl>().match(
+      FromTU, cxxDeductionGuideDecl());
+
+  NamedDecl *P1 =
+      FromD1->getDescribedFunctionTemplate()->getTemplateParameters()->getParam(
+          0);
+  NamedDecl *P2 =
+      FromD2->getDescribedFunctionTemplate()->getTemplateParameters()->getParam(
+          0);
+  DeclContext *DC = P1->getDeclContext();
+
+  ASSERT_EQ(P1, P2);
+  ASSERT_TRUE(DC == FromD1 || DC == FromD2);
+
+  auto *ToD1 = Import(FromD1, Lang_CXX17);
+  auto *ToD2 = Import(FromD2, Lang_CXX17);
+  ASSERT_TRUE(ToD1 && ToD2);
+
+  P1 = ToD1->getDescribedFunctionTemplate()->getTemplateParameters()->getParam(
+      0);
+  P2 = ToD2->getDescribedFunctionTemplate()->getTemplateParameters()->getParam(
+      0);
+  DC = P1->getDeclContext();
+
+  EXPECT_EQ(P1, P2);
+  EXPECT_TRUE(DC == ToD1 || DC == ToD2);
+
+  ASTImporterLookupTable *Tbl = SharedStatePtr->getLookupTable();
+  if (Tbl->contains(ToD1, P1)) {
+    EXPECT_FALSE(Tbl->contains(ToD2, P1));
+  } else {
+    EXPECT_TRUE(Tbl->contains(ToD2, P1));
+  }
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateParameterDeclContext) {
+  constexpr auto Code =
+      R"(
+      template<class T1, class T2>
+      int X1;
+      template<class T2>
+      int X1<int, T2>;
+
+      namespace Ns {
+        template<class T1, class T2>
+        int X2;
+        template<class T2>
+        int X2<int, T2>;
+      }
+      )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX14);
+
+  auto *FromD1 = FirstDeclMatcher<VarTemplateDecl>().match(
+      FromTU, varTemplateDecl(hasName("X1")));
+  auto *FromD1Part =
+      FirstDeclMatcher<VarTemplatePartialSpecializationDecl>().match(
+          FromTU, varTemplatePartialSpecializationDecl(hasName("X1")));
+  auto *FromD2 = FirstDeclMatcher<VarTemplateDecl>().match(
+      FromTU, varTemplateDecl(hasName("X2")));
+  auto *FromD2Part =
+      FirstDeclMatcher<VarTemplatePartialSpecializationDecl>().match(
+          FromTU, varTemplatePartialSpecializationDecl(hasName("X2")));
+
+  ASSERT_EQ(FromD1->getTemplateParameters()->getParam(0)->getDeclContext(),
+            FromD1->getDeclContext());
+  ASSERT_EQ(FromD2->getTemplateParameters()->getParam(0)->getDeclContext(),
+            FromD2->getDeclContext());
+
+  ASSERT_EQ(FromD1Part->getTemplateParameters()->getParam(0)->getDeclContext(),
+            FromD1Part->getDeclContext());
+  // FIXME: VarTemplatePartialSpecializationDecl does not update ("adopt")
+  // template parameter decl context
+  // ASSERT_EQ(FromD2Part->getTemplateParameters()->getParam(0)->getDeclContext(),
+  // FromD2Part->getDeclContext());
+
+  auto *ToD1 = Import(FromD1, Lang_CXX14);
+  auto *ToD2 = Import(FromD2, Lang_CXX14);
+
+  auto *ToD1Part = Import(FromD1Part, Lang_CXX14);
+  auto *ToD2Part = Import(FromD2Part, Lang_CXX14);
+
+  EXPECT_EQ(ToD1->getTemplateParameters()->getParam(0)->getDeclContext(),
+            ToD1->getDeclContext());
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+      ToD1->getDeclContext(), ToD1->getTemplateParameters()->getParam(0)));
+  EXPECT_EQ(ToD2->getTemplateParameters()->getParam(0)->getDeclContext(),
+            ToD2->getDeclContext());
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+      ToD2->getDeclContext(), ToD2->getTemplateParameters()->getParam(0)));
+
+  EXPECT_EQ(ToD1Part->getTemplateParameters()->getParam(0)->getDeclContext(),
+            ToD1Part->getDeclContext());
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+      ToD1Part->getDeclContext(),
+      ToD1Part->getTemplateParameters()->getParam(0)));
+  // EXPECT_EQ(ToD2Part->getTemplateParameters()->getParam(0)->getDeclContext(),
+  // ToD2Part->getDeclContext());
+  // EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+  //     ToD2Part->getDeclContext(),
+  //     ToD2Part->getTemplateParameters()->getParam(0)));
+  (void)ToD2Part;
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+       TypeAliasTemplateParameterDeclContext) {
+  constexpr auto Code =
+      R"(
+      template<class T1, class T2>
+      struct S {};
+      template<class T> using S1 = S<T, int>;
+      namespace Ns {
+        template<class T> using S2 = S<T, int>;
+      }
+      )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+
+  auto *FromD1 = FirstDeclMatcher<TypeAliasTemplateDecl>().match(
+      FromTU, typeAliasTemplateDecl(hasName("S1")));
+  auto *FromD2 = FirstDeclMatcher<TypeAliasTemplateDecl>().match(
+      FromTU, typeAliasTemplateDecl(hasName("S2")));
+
+  ASSERT_EQ(FromD1->getTemplateParameters()->getParam(0)->getDeclContext(),
+            FromD1->getDeclContext());
+  ASSERT_EQ(FromD2->getTemplateParameters()->getParam(0)->getDeclContext(),
+            FromD2->getDeclContext());
+
+  auto *ToD1 = Import(FromD1, Lang_CXX11);
+  auto *ToD2 = Import(FromD2, Lang_CXX11);
+
+  EXPECT_EQ(ToD1->getTemplateParameters()->getParam(0)->getDeclContext(),
+            ToD1->getDeclContext());
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+      ToD1->getDeclContext(), ToD1->getTemplateParameters()->getParam(0)));
+  EXPECT_EQ(ToD2->getTemplateParameters()->getParam(0)->getDeclContext(),
+            ToD2->getDeclContext());
+  EXPECT_TRUE(SharedStatePtr->getLookupTable()->contains(
+      ToD2->getDeclContext(), ToD2->getTemplateParameters()->getParam(0)));
+}
+
 struct ASTImporterLookupTableTest : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ASTImporterLookupTableTest, OneDecl) {
   auto *ToTU = getToTuDecl("int a;", Lang_CXX03);
   auto *D = FirstDeclMatcher<VarDecl>().match(ToTU, varDecl(hasName("a")));
   ASTImporterLookupTable LT(*ToTU);
   auto Res = LT.lookup(ToTU, D->getDeclName());
   ASSERT_EQ(Res.size(), 1u);