[ASTImporter] Fix lexical DC for templated decls; support VarTemplatePartialSpecDecl

[a.sidorin]

Summary

Also minor refactoring in related functions was done.

[xazax.hun]

Looks good to me. Only found a few nits.

lib/AST/ASTImporter.cpp

4296 ↗

(On Diff #133189)

auto * to not repeat type.

4399 ↗

(On Diff #133189)

The formatting might be a bit off here.

[a.sidorin]

lib/AST/ASTImporter.cpp

4296 ↗

(On Diff #133189)

I usually prefer to keep the type if it doesn't give a large space win because it hurts readability a bit. From VarDecl *, we can instantly find the type; for auto, we have to look forward. (Yes, VarTemplatePartialSpecializationDecl has to be replaced immediately :) ).
Other issue is that QtCreator I use for development still doesn't have an autocompletion for auto types. However, LLVM says: "do use auto with initializers like cast<Foo>(...)", so I'll change this.

[a.sidorin]

Fix style issues found on review.

[martong]

lib/AST/ASTImporter.cpp

2858 ↗

(On Diff #133626)

In case of class templates, the explicit instantiation is the member of the DeclContext. It does not belong to the DeclContext only in case of implicit instantiations.
I suppose the same is true for template variables as well.
In our code base we fixed it by checking on the TSK_ kind, see https://github.com/Ericsson/clang/pull/270/files.

@xazax.hun perhaps we should open source some of our fixes as well?

4455 ↗

(On Diff #133626)

This is related to my other comment, perhaps we should not add D2 to the DeclContext unconditionnally. I think only implicit instantiations should be added. See
https://github.com/Ericsson/clang/pull/270/files

[martong]

Just ignore my previous comments, the issue with explicit instantiations could be fixed in a separate independent patch. All is good.

[a.sidorin]

lib/AST/ASTImporter.cpp

2858 ↗

(On Diff #133626)

This code handles templated declarations, not template instantiations. Every TemplateDecl has an underlying templated declaration that is used while instantiating the template; it is not the same as template specialization. And it is not visible from the DeclContext it refers to via lookup. Or am I misunderstanding something?
Yes, the issue with implicit instantiations still persists but it is not the target for this patch. I have took a look at the patch; it looks like it fixes a separate issue (and I welcome you to post the patch here!).

cfe/trunk/lib/AST/ASTImporter.cpp

 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/Support/MemoryBuffer.h"
-#include <deque>
 
 namespace clang {
   class ASTNodeImporter : public TypeVisitor<ASTNodeImporter, QualType>,
                           public DeclVisitor<ASTNodeImporter, Decl *>,
                           public StmtVisitor<ASTNodeImporter, Stmt *> {
     ASTImporter &Importer;
 
   public:
bool ASTNodeImporter::ImportTemplateArgumentListInfo(
   TemplateArgumentListInfo ToTAInfo(Importer.Import(FromLAngleLoc),
                                     Importer.Import(FromRAngleLoc));
   if (ImportTemplateArgumentListInfo(Container, ToTAInfo))
     return true;
   Result = ToTAInfo;
   return false;
 }
 
+template <>
+bool ASTNodeImporter::ImportTemplateArgumentListInfo<TemplateArgumentListInfo>(
+    const TemplateArgumentListInfo &From, TemplateArgumentListInfo &Result) {
+  return ImportTemplateArgumentListInfo(
+      From.getLAngleLoc(), From.getRAngleLoc(), From.arguments(), Result);
+}
+
+template <>
+bool ASTNodeImporter::ImportTemplateArgumentListInfo<
+    ASTTemplateArgumentListInfo>(const ASTTemplateArgumentListInfo &From,
+                                 TemplateArgumentListInfo &Result) {
+  return ImportTemplateArgumentListInfo(From.LAngleLoc, From.RAngleLoc,
+                                        From.arguments(), Result);
+}
+
 bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord, 
                                         RecordDecl *ToRecord, bool Complain) {
   // Eliminate a potential failure point where we attempt to re-import
   // something we're trying to import while completing ToRecord.
   Decl *ToOrigin = Importer.GetOriginalDecl(ToRecord);
   if (ToOrigin) {
     RecordDecl *ToOriginRecord = dyn_cast<RecordDecl>(ToOrigin);
     if (ToOriginRecord)
Decl *ASTNodeImporter::VisitTypedefNameDecl(TypedefNameDecl *D, bool IsAlias) {
   if (T.isNull())
     return nullptr;
 
   // Create the new typedef node.
   TypeSourceInfo *TInfo = Importer.Import(D->getTypeSourceInfo());
   SourceLocation StartL = Importer.Import(D->getLocStart());
   TypedefNameDecl *ToTypedef;
   if (IsAlias)
-    ToTypedef = TypeAliasDecl::Create(Importer.getToContext(), DC,
-                                      StartL, Loc,
-                                      Name.getAsIdentifierInfo(),
-                                      TInfo);
+    ToTypedef = TypeAliasDecl::Create(Importer.getToContext(), DC, StartL, Loc,
+                                      Name.getAsIdentifierInfo(), TInfo);
   else
     ToTypedef = TypedefDecl::Create(Importer.getToContext(), DC,
                                     StartL, Loc,
                                     Name.getAsIdentifierInfo(),
                                     TInfo);
 
   ToTypedef->setAccess(D->getAccess());
   ToTypedef->setLexicalDeclContext(LexicalDC);
   Importer.Imported(D, ToTypedef);
+
+  // Templated declarations should not appear in DeclContext.
+  TypeAliasDecl *FromAlias = IsAlias ? cast<TypeAliasDecl>(D) : nullptr;
+  if (!FromAlias || !FromAlias->getDescribedAliasTemplate())
     LexicalDC->addDeclInternal(ToTypedef);
 
   return ToTypedef;
 }
 
 Decl *ASTNodeImporter::VisitTypedefDecl(TypedefDecl *D) {
   return VisitTypedefNameDecl(D, /*IsAlias=*/false);
 }
 
 Decl *ASTNodeImporter::VisitTypeAliasDecl(TypeAliasDecl *D) {
   return VisitTypedefNameDecl(D, /*IsAlias=*/true);
 }
 
 Decl *ASTNodeImporter::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
   // Import the major distinguishing characteristics of this typedef.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
   SourceLocation Loc;
-  NamedDecl *ToD;
-  if (ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
+  NamedDecl *FoundD;
+  if (ImportDeclParts(D, DC, LexicalDC, Name, FoundD, Loc))
     return nullptr;
-  if (ToD)
-    return ToD;
+  if (FoundD)
+    return FoundD;
 
   // If this typedef is not in block scope, determine whether we've
   // seen a typedef with the same name (that we can merge with) or any
   // other entity by that name (which name lookup could conflict with).
   if (!DC->isFunctionOrMethod()) {
     SmallVector<NamedDecl *, 4> ConflictingDecls;
     unsigned IDNS = Decl::IDNS_Ordinary;
     SmallVector<NamedDecl *, 2> FoundDecls;
if (!DC->isFunctionOrMethod()) {
     }
   }
 
   TemplateParameterList *Params = ImportTemplateParameterList(
         D->getTemplateParameters());
   if (!Params)
     return nullptr;
 
-  NamedDecl *TemplDecl = cast_or_null<NamedDecl>(
+  auto *TemplDecl = cast_or_null<TypeAliasDecl>(
         Importer.Import(D->getTemplatedDecl()));
   if (!TemplDecl)
     return nullptr;
 
   TypeAliasTemplateDecl *ToAlias = TypeAliasTemplateDecl::Create(
         Importer.getToContext(), DC, Loc, Name, Params, TemplDecl);
 
+  TemplDecl->setDescribedAliasTemplate(ToAlias);
+
   ToAlias->setAccess(D->getAccess());
   ToAlias->setLexicalDeclContext(LexicalDC);
   Importer.Imported(D, ToAlias);
   LexicalDC->addDeclInternal(ToAlias);
-  return ToD;
+  return ToAlias;
 }
 
 Decl *ASTNodeImporter::VisitLabelDecl(LabelDecl *D) {
   // Import the major distinguishing characteristics of this label.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
   SourceLocation Loc;
   NamedDecl *ToD;
if (ImportTemplateArguments(TemplArgs.data(), TemplArgs.size(),
                                 ToTemplArgs))
       return true;
 
     TemplateArgumentList *ToTAList = TemplateArgumentList::CreateCopy(
           Importer.getToContext(), ToTemplArgs);
 
     TemplateArgumentListInfo ToTAInfo;
     const auto *FromTAArgsAsWritten = FTSInfo->TemplateArgumentsAsWritten;
-    if (FromTAArgsAsWritten) {
-      if (ImportTemplateArgumentListInfo(
-              FromTAArgsAsWritten->LAngleLoc, FromTAArgsAsWritten->RAngleLoc,
-              FromTAArgsAsWritten->arguments(), ToTAInfo))
+    if (FromTAArgsAsWritten)
+      if (ImportTemplateArgumentListInfo(*FromTAArgsAsWritten, ToTAInfo))
         return true;
-    }
 
     SourceLocation POI = Importer.Import(FTSInfo->getPointOfInstantiation());
 
     ToFD->setFunctionTemplateSpecialization(
         Template, ToTAList, /* InsertPos= */ nullptr,
         TSK, FromTAArgsAsWritten ? &ToTAInfo : nullptr, POI);
     return false;
   }
VarDecl *ToVar = VarDecl::Create(Importer.getToContext(), DC,
                                    Importer.Import(D->getInnerLocStart()),
                                    Loc, Name.getAsIdentifierInfo(),
                                    T, TInfo,
                                    D->getStorageClass());
   ToVar->setQualifierInfo(Importer.Import(D->getQualifierLoc()));
   ToVar->setAccess(D->getAccess());
   ToVar->setLexicalDeclContext(LexicalDC);
   Importer.Imported(D, ToVar);
+
+  // Templated declarations should never appear in the enclosing DeclContext.
+  if (!D->getDescribedVarTemplate())
     LexicalDC->addDeclInternal(ToVar);
 
   if (!D->isFileVarDecl() &&
       D->isUsed())
     ToVar->setIsUsed();
 
   // Merge the initializer.
   if (ImportDefinition(D, ToVar))
     return nullptr;
if (D2) {
     }
   } else {
     // Create a new specialization.
     if (ClassTemplatePartialSpecializationDecl *PartialSpec =
         dyn_cast<ClassTemplatePartialSpecializationDecl>(D)) {
 
       // Import TemplateArgumentListInfo
       TemplateArgumentListInfo ToTAInfo;
-      auto &ASTTemplateArgs = *PartialSpec->getTemplateArgsAsWritten();
-      if (ImportTemplateArgumentListInfo(ASTTemplateArgs.arguments(), ToTAInfo))
+      const auto &ASTTemplateArgs = *PartialSpec->getTemplateArgsAsWritten();
+      if (ImportTemplateArgumentListInfo(ASTTemplateArgs, ToTAInfo))
         return nullptr;
 
       QualType CanonInjType = Importer.Import(
             PartialSpec->getInjectedSpecializationType());
       if (CanonInjType.isNull())
         return nullptr;
       CanonInjType = CanonInjType.getCanonicalType();
 
Decl *ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) {
   VarDecl *DTemplated = D->getTemplatedDecl();
 
   // Import the type.
   QualType T = Importer.Import(DTemplated->getType());
   if (T.isNull())
     return nullptr;
 
   // Create the declaration that is being templated.
-  SourceLocation StartLoc = Importer.Import(DTemplated->getLocStart());
-  SourceLocation IdLoc = Importer.Import(DTemplated->getLocation());
-  TypeSourceInfo *TInfo = Importer.Import(DTemplated->getTypeSourceInfo());
-  VarDecl *D2Templated = VarDecl::Create(Importer.getToContext(), DC, StartLoc,
-                                         IdLoc, Name.getAsIdentifierInfo(), T,
-                                         TInfo, DTemplated->getStorageClass());
-  D2Templated->setAccess(DTemplated->getAccess());
-  D2Templated->setQualifierInfo(Importer.Import(DTemplated->getQualifierLoc()));
-  D2Templated->setLexicalDeclContext(LexicalDC);
-
-  // Importer.Imported(DTemplated, D2Templated);
-  // LexicalDC->addDeclInternal(D2Templated);
-
-  // Merge the initializer.
-  if (ImportDefinition(DTemplated, D2Templated))
+  auto *ToTemplated = dyn_cast_or_null<VarDecl>(Importer.Import(DTemplated));
+  if (!ToTemplated)
     return nullptr;
 
   // Create the variable template declaration itself.
   TemplateParameterList *TemplateParams =
       ImportTemplateParameterList(D->getTemplateParameters());
   if (!TemplateParams)
     return nullptr;
 
-  VarTemplateDecl *D2 = VarTemplateDecl::Create(
-      Importer.getToContext(), DC, Loc, Name, TemplateParams, D2Templated);
-  D2Templated->setDescribedVarTemplate(D2);
-
-  D2->setAccess(D->getAccess());
-  D2->setLexicalDeclContext(LexicalDC);
-  LexicalDC->addDeclInternal(D2);
+  VarTemplateDecl *ToVarTD = VarTemplateDecl::Create(
+      Importer.getToContext(), DC, Loc, Name, TemplateParams, ToTemplated);
+  ToTemplated->setDescribedVarTemplate(ToVarTD);
+
+  ToVarTD->setAccess(D->getAccess());
+  ToVarTD->setLexicalDeclContext(LexicalDC);
+  LexicalDC->addDeclInternal(ToVarTD);
 
   // Note the relationship between the variable templates.
-  Importer.Imported(D, D2);
-  Importer.Imported(DTemplated, D2Templated);
+  Importer.Imported(D, ToVarTD);
+  Importer.Imported(DTemplated, ToTemplated);
 
   if (DTemplated->isThisDeclarationADefinition() &&
-      !D2Templated->isThisDeclarationADefinition()) {
+      !ToTemplated->isThisDeclarationADefinition()) {
     // FIXME: Import definition!
   }
 
-  return D2;
+  return ToVarTD;
 }
 
 Decl *ASTNodeImporter::VisitVarTemplateSpecializationDecl(
     VarTemplateSpecializationDecl *D) {
   // If this record has a definition in the translation unit we're coming from,
   // but this particular declaration is not that definition, import the
   // definition and map to that.
   VarDecl *Definition = D->getDefinition();
if (VarDecl *FoundDef = D2->getDefinition()) {
       }
     }
   } else {
 
     // Import the type.
     QualType T = Importer.Import(D->getType());
     if (T.isNull())
       return nullptr;
+
     TypeSourceInfo *TInfo = Importer.Import(D->getTypeSourceInfo());
+    if (D->getTypeSourceInfo() && !TInfo)
+      return nullptr;
 
+    TemplateArgumentListInfo ToTAInfo;
+    if (ImportTemplateArgumentListInfo(D->getTemplateArgsInfo(), ToTAInfo))
+      return nullptr;
+
+    using PartVarSpecDecl = VarTemplatePartialSpecializationDecl;
     // Create a new specialization.
+    if (auto *FromPartial = dyn_cast<PartVarSpecDecl>(D)) {
+      // Import TemplateArgumentListInfo
+      TemplateArgumentListInfo ArgInfos;
+      const auto *FromTAArgsAsWritten = FromPartial->getTemplateArgsAsWritten();
+      // NOTE: FromTAArgsAsWritten and template parameter list are non-null.
+      if (ImportTemplateArgumentListInfo(*FromTAArgsAsWritten, ArgInfos))
+        return nullptr;
+
+      TemplateParameterList *ToTPList = ImportTemplateParameterList(
+            FromPartial->getTemplateParameters());
+      if (!ToTPList)
+        return nullptr;
+
+      auto *ToPartial = PartVarSpecDecl::Create(
+          Importer.getToContext(), DC, StartLoc, IdLoc, ToTPList, VarTemplate,
+          T, TInfo, D->getStorageClass(), TemplateArgs, ArgInfos);
+
+      auto *FromInst = FromPartial->getInstantiatedFromMember();
+      auto *ToInst = cast_or_null<PartVarSpecDecl>(Importer.Import(FromInst));
+      if (FromInst && !ToInst)
+        return nullptr;
+
+      ToPartial->setInstantiatedFromMember(ToInst);
+      if (FromPartial->isMemberSpecialization())
+        ToPartial->setMemberSpecialization();
+
+      D2 = ToPartial;
+
+    } else { // Full specialization
       D2 = VarTemplateSpecializationDecl::Create(
           Importer.getToContext(), DC, StartLoc, IdLoc, VarTemplate, T, TInfo,
           D->getStorageClass(), TemplateArgs);
+    }
+
+    SourceLocation POI = D->getPointOfInstantiation();
+    if (POI.isValid())
+      D2->setPointOfInstantiation(Importer.Import(POI));
+
     D2->setSpecializationKind(D->getSpecializationKind());
-    D2->setTemplateArgsInfo(D->getTemplateArgsInfo());
+    D2->setTemplateArgsInfo(ToTAInfo);
 
     // Add this specialization to the class template.
     VarTemplate->AddSpecialization(D2, InsertPos);
 
     // Import the qualifier, if any.
     D2->setQualifierInfo(Importer.Import(D->getQualifierLoc()));
 
+    if (D->isConstexpr())
+      D2->setConstexpr(true);
+
     // Add the specialization to this context.
     D2->setLexicalDeclContext(LexicalDC);
     LexicalDC->addDeclInternal(D2);
+
+    D2->setAccess(D->getAccess());
   }
+
   Importer.Imported(D, D2);
 
-  if (D->isThisDeclarationADefinition() && ImportDefinition(D, D2))
+  // NOTE: isThisDeclarationADefinition() can return DeclarationOnly even if
+  // declaration has initializer. Should this be fixed in the AST?.. Anyway,
+  // we have to check the declaration for initializer - otherwise, it won't be
+  // imported.
+  if ((D->isThisDeclarationADefinition() || D->hasInit()) &&
+      ImportDefinition(D, D2))
     return nullptr;
 
   return D2;
 }
 
 Decl *ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;

cfe/trunk/test/ASTMerge/var-cpp/Inputs/var1.cpp

+
+template <typename T>
+constexpr T my_pi = T(3.1415926535897932385L);  // variable template
+
+template <> constexpr char my_pi<char> = '3';   // variable template specialization
+
+template <typename T>
+struct Wrapper {
+  template <typename U> static constexpr U my_const = U(1);
+   // Variable template partial specialization with member variable.
+  template <typename U> static constexpr U *my_const<const U *> = (U *)(0);
+};
+
+constexpr char a[] = "hello";
+
+template <> template <>
+constexpr const char *Wrapper<float>::my_const<const char *> = a;

cfe/trunk/test/ASTMerge/var-cpp/test.cpp

+// RUN: %clang_cc1 -emit-pch -std=c++17 -o %t.1.ast %S/Inputs/var1.cpp
+// RUN: %clang_cc1 -std=c++17 -ast-merge %t.1.ast -fsyntax-only %s 2>&1
+
+static_assert(my_pi<double> == (double)3.1415926535897932385L);
+static_assert(my_pi<char> == '3');
+
+static_assert(Wrapper<int>::my_const<float> == 1.f);
+static_assert(Wrapper<char>::my_const<const float *> == nullptr);
+static_assert(Wrapper<float>::my_const<const char *> == a);

cfe/trunk/unittests/AST/ASTImporterTest.cpp

testImport("template <typename T> struct C { T t; };"
              "void instantiate() { declToImport<int>(); }",
              Lang_CXX, "", Lang_CXX, Verifier,
              functionTemplateDecl(has(functionDecl(
                  has(compoundStmt(has(cxxDependentScopeMemberExpr())))))));
 }
 
 TEST(ImportType, ImportTypeAliasTemplate) {
   MatchVerifier<Decl> Verifier;
-  testImport("template <int K>"
-             "struct dummy { static const int i = K; };"
-             "template <int K> using dummy2 = dummy<K>;"
-             "int declToImport() { return dummy2<3>::i; }",
-             Lang_CXX11, "", Lang_CXX11, Verifier,
-             functionDecl(
-               hasBody(
-                 compoundStmt(
-                   has(
-                     returnStmt(
-                       has(
-                         implicitCastExpr(
-                           has(
-                             declRefExpr())))))))));
+  testImport(
+      "template <int K>"
+          hasBody(compoundStmt(
+              has(returnStmt(has(implicitCastExpr(has(declRefExpr()))))))),
+          unless(hasAncestor(translationUnitDecl(has(typeAliasDecl()))))));
+}
+
+const internal::VariadicDynCastAllOfMatcher<Decl, VarTemplateSpecializationDecl>
+    varTemplateSpecializationDecl;
+
+TEST(ImportDecl, ImportVarTemplate) {
+  MatchVerifier<Decl> Verifier;
+  testImport(
+      "template <typename T>"
+      "T pi = T(3.1415926535897932385L);"
+      "void declToImport() { pi<int>; }",
+      Lang_CXX11, "", Lang_CXX11, Verifier,
+      functionDecl(
+          hasBody(has(declRefExpr(to(varTemplateSpecializationDecl())))),
+          unless(hasAncestor(translationUnitDecl(has(varDecl(
+              hasName("pi"), unless(varTemplateSpecializationDecl()))))))));
 }
 
 TEST(ImportType, ImportPackExpansion) {
   MatchVerifier<Decl> Verifier;
   testImport("template <typename... Args>"
              "struct dummy {"
              "  dummy(Args... args) {}"
              "  static const int i = 4;"