[clang] [clang-tools-extra] [include-cleaner] Use FoundDecl only for using-shadow-decls (PR #82615)

kadir çetinkaya via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 22 05:19:37 PST 2024


https://github.com/kadircet created https://github.com/llvm/llvm-project/pull/82615

None

>From 1ae249119856ae2d8c1d2d4a3c3e311fa3d66dd0 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet at google.com>
Date: Thu, 22 Feb 2024 13:22:11 +0100
Subject: [PATCH 1/2] Reland "[clang] Preserve found-decl when constructing
 VarTemplateIds"

Update include-cleaner tests. Now that we have proper found-decls set up
for VarTemplates, in case of instationtations we point to primary
templates and not specializations. To be changed in a follow-up patch.
---
 .../include-cleaner/unittests/WalkASTTest.cpp  | 16 ++++++++--------
 clang/include/clang/Sema/Sema.h                |  2 +-
 clang/lib/Sema/SemaTemplate.cpp                | 18 ++++++++----------
 clang/test/AST/ast-dump-using.cpp              |  7 +++++++
 4 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index bdfc24b8edee38..0be5db36b1fc51 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -200,24 +200,24 @@ TEST(WalkAST, VarTemplates) {
   EXPECT_THAT(testWalk(R"cpp(
     template <typename T> T $explicit^Foo = 0;)cpp",
                        "int z = ^Foo<int>;"),
-              ElementsAre(Decl::VarTemplateSpecialization));
+              ElementsAre(Decl::VarTemplate));
   EXPECT_THAT(testWalk(R"cpp(
-    template<typename T> T Foo = 0;
-    template<> int $explicit^Foo<int> = 1;)cpp",
+    template<typename T> T $explicit^Foo = 0;
+    template<> int Foo<int> = 1;)cpp",
                        "int x = ^Foo<int>;"),
-              ElementsAre(Decl::VarTemplateSpecialization));
+              ElementsAre(Decl::VarTemplate));
   // FIXME: This points at implicit specialization, instead we should point to
   // explicit partial specializaiton pattern.
   EXPECT_THAT(testWalk(R"cpp(
-    template<typename T> T Foo = 0;
-    template<typename T> T* $explicit^Foo<T*> = nullptr;)cpp",
+    template<typename T> T $explicit^Foo = 0;
+    template<typename T> T* Foo<T*> = nullptr;)cpp",
                        "int *x = ^Foo<int *>;"),
-              ElementsAre(Decl::VarTemplateSpecialization));
+              ElementsAre(Decl::VarTemplate));
   EXPECT_THAT(testWalk(R"cpp(
     template<typename T> T $explicit^Foo = 0;
     template int Foo<int>;)cpp",
                        "int x = ^Foo<int>;"),
-              ElementsAre(Decl::VarTemplateSpecialization));
+              ElementsAre(Decl::VarTemplate));
 }
 TEST(WalkAST, FunctionTemplates) {
   // Explicit instantiation and (partial) specialization references primary
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fcccac10f4733a..e457694e4625db 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8540,7 +8540,7 @@ class Sema final {
   /// if the arguments are dependent.
   ExprResult CheckVarTemplateId(const CXXScopeSpec &SS,
                                 const DeclarationNameInfo &NameInfo,
-                                VarTemplateDecl *Template,
+                                VarTemplateDecl *Template, NamedDecl *FoundD,
                                 SourceLocation TemplateLoc,
                                 const TemplateArgumentListInfo *TemplateArgs);
 
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 1a975a8d0a0df5..7d3d665194add1 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4958,11 +4958,10 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,
   return Decl;
 }
 
-ExprResult
-Sema::CheckVarTemplateId(const CXXScopeSpec &SS,
-                         const DeclarationNameInfo &NameInfo,
-                         VarTemplateDecl *Template, SourceLocation TemplateLoc,
-                         const TemplateArgumentListInfo *TemplateArgs) {
+ExprResult Sema::CheckVarTemplateId(
+    const CXXScopeSpec &SS, const DeclarationNameInfo &NameInfo,
+    VarTemplateDecl *Template, NamedDecl *FoundD, SourceLocation TemplateLoc,
+    const TemplateArgumentListInfo *TemplateArgs) {
 
   DeclResult Decl = CheckVarTemplateId(Template, TemplateLoc, NameInfo.getLoc(),
                                        *TemplateArgs);
@@ -4978,8 +4977,7 @@ Sema::CheckVarTemplateId(const CXXScopeSpec &SS,
                                        NameInfo.getLoc());
 
   // Build an ordinary singleton decl ref.
-  return BuildDeclarationNameExpr(SS, NameInfo, Var,
-                                  /*FoundD=*/nullptr, TemplateArgs);
+  return BuildDeclarationNameExpr(SS, NameInfo, Var, FoundD, TemplateArgs);
 }
 
 void Sema::diagnoseMissingTemplateArguments(TemplateName Name,
@@ -5066,9 +5064,9 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec &SS,
   bool KnownDependent = false;
   // In C++1y, check variable template ids.
   if (R.getAsSingle<VarTemplateDecl>()) {
-    ExprResult Res = CheckVarTemplateId(SS, R.getLookupNameInfo(),
-                                        R.getAsSingle<VarTemplateDecl>(),
-                                        TemplateKWLoc, TemplateArgs);
+    ExprResult Res = CheckVarTemplateId(
+        SS, R.getLookupNameInfo(), R.getAsSingle<VarTemplateDecl>(),
+        R.getRepresentativeDecl(), TemplateKWLoc, TemplateArgs);
     if (Res.isInvalid() || Res.isUsable())
       return Res;
     // Result is dependent. Carry on to build an UnresolvedLookupEpxr.
diff --git a/clang/test/AST/ast-dump-using.cpp b/clang/test/AST/ast-dump-using.cpp
index 5a4e910ffb8654..8e5c60d3aabf4a 100644
--- a/clang/test/AST/ast-dump-using.cpp
+++ b/clang/test/AST/ast-dump-using.cpp
@@ -2,6 +2,7 @@
 
 namespace a {
 struct S;
+template <typename T> T x = {};
 }
 namespace b {
 using a::S;
@@ -21,4 +22,10 @@ typedef S e; // check the same UsingType is reused.
 // CHECK-NEXT:   `-UsingType [[TYPE_ADDR]] 'a::S' sugar
 // CHECK-NEXT:     |-UsingShadow [[SHADOW_ADDR]] 'S'
 // CHECK-NEXT:     `-RecordType {{.*}} 'a::S'
+using a::x;
+
+void foo() {
+  x<int> = 3;
+  // CHECK: DeclRefExpr {{.*}} 'x' {{.*}} (UsingShadow {{.*}} 'x')
+}
 }

>From 3910281c9983b7ecd062b128654f2cc528b12cd7 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet at google.com>
Date: Thu, 22 Feb 2024 13:52:42 +0100
Subject: [PATCH 2/2] [include-cleaner] Use FoundDecl only for
 using-shadow-decls

---
 .../include-cleaner/lib/WalkAST.cpp           |  5 +++
 .../include-cleaner/unittests/WalkASTTest.cpp | 34 +++++++++++--------
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index 6c4d9b7862d915..277e6ec5b08900 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -128,6 +128,11 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
     auto *FD = DRE->getFoundDecl();
+    // Prefer the underlying decl if FoundDecl isn't a shadow decl, e.g:
+    // - For templates, found-decl is always primary template, but we want the
+    // specializaiton itself.
+    if (!llvm::isa<UsingShadowDecl>(FD))
+      FD = DRE->getDecl();
     // For refs to non-meber-like decls, use the found decl.
     // For member-like decls, we should have a reference from the qualifier to
     // the container decl instead, which is preferred as it'll handle
diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index 0be5db36b1fc51..e238dc3d902bbe 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -200,24 +200,26 @@ TEST(WalkAST, VarTemplates) {
   EXPECT_THAT(testWalk(R"cpp(
     template <typename T> T $explicit^Foo = 0;)cpp",
                        "int z = ^Foo<int>;"),
-              ElementsAre(Decl::VarTemplate));
+              ElementsAre(Decl::VarTemplateSpecialization));
   EXPECT_THAT(testWalk(R"cpp(
-    template<typename T> T $explicit^Foo = 0;
-    template<> int Foo<int> = 1;)cpp",
+    template<typename T> T Foo = 0;
+    template<> int $explicit^Foo<int> = 1;)cpp",
                        "int x = ^Foo<int>;"),
-              ElementsAre(Decl::VarTemplate));
+              ElementsAre(Decl::VarTemplateSpecialization));
   // FIXME: This points at implicit specialization, instead we should point to
   // explicit partial specializaiton pattern.
   EXPECT_THAT(testWalk(R"cpp(
-    template<typename T> T $explicit^Foo = 0;
-    template<typename T> T* Foo<T*> = nullptr;)cpp",
+    template<typename T> T Foo = 0;
+    template<typename T> T* $explicit^Foo<T*> = nullptr;)cpp",
                        "int *x = ^Foo<int *>;"),
-              ElementsAre(Decl::VarTemplate));
+              ElementsAre(Decl::VarTemplateSpecialization));
+  // Implicit specializations through explicit instantiations has source
+  // locations pointing at the primary template.
   EXPECT_THAT(testWalk(R"cpp(
     template<typename T> T $explicit^Foo = 0;
     template int Foo<int>;)cpp",
                        "int x = ^Foo<int>;"),
-              ElementsAre(Decl::VarTemplate));
+              ElementsAre(Decl::VarTemplateSpecialization));
 }
 TEST(WalkAST, FunctionTemplates) {
   // Explicit instantiation and (partial) specialization references primary
@@ -239,18 +241,19 @@ TEST(WalkAST, FunctionTemplates) {
   EXPECT_THAT(testWalk(R"cpp(
     template <typename T> void $explicit^foo() {})cpp",
                        "auto x = []{ ^foo<int>(); };"),
-              ElementsAre(Decl::FunctionTemplate));
-  // FIXME: DeclRefExpr points at primary template, not the specialization.
+              ElementsAre(Decl::Function));
   EXPECT_THAT(testWalk(R"cpp(
-    template<typename T> void $explicit^foo() {}
-    template<> void foo<int>(){})cpp",
+    template<typename T> void foo() {}
+    template<> void $explicit^foo<int>(){})cpp",
                        "auto x = []{ ^foo<int>(); };"),
-              ElementsAre(Decl::FunctionTemplate));
+              ElementsAre(Decl::Function));
+  // The decl is actually the specialization, but explicit instantations point
+  // at the primary template.
   EXPECT_THAT(testWalk(R"cpp(
     template<typename T> void $explicit^foo() {};
     template void foo<int>();)cpp",
                        "auto x = [] { ^foo<int>(); };"),
-              ElementsAre(Decl::FunctionTemplate));
+              ElementsAre(Decl::Function));
 }
 TEST(WalkAST, TemplateSpecializationsFromUsingDecl) {
   // Class templates
@@ -548,7 +551,8 @@ TEST(WalkAST, Concepts) {
   testWalk(Concept, "template<typename T> void func() requires ^Foo<T> {}");
   testWalk(Concept, "void func(^Foo auto x) {}");
   // FIXME: Foo should be explicitly referenced.
-  testWalk("template<typename T> concept Foo = true;", "void func() { ^Foo auto x = 1; }");
+  testWalk("template<typename T> concept Foo = true;",
+           "void func() { ^Foo auto x = 1; }");
 }
 
 TEST(WalkAST, FriendDecl) {



More information about the cfe-commits mailing list