[PATCH] D146202: [clang] Fix a UsingTemplate regression after 3e78fa860235431323aaf08c8fa922d75a7cfffa

2023-03-16 Thread Haojian Wu via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f5fdc22a26c: [clang] Fix a UsingTemplate regression after… 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146202/new/

https://reviews.llvm.org/D146202

Files:
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
  clang/lib/AST/TemplateName.cpp
  clang/test/AST/ast-dump-using-template.cpp


Index: clang/test/AST/ast-dump-using-template.cpp
===
--- clang/test/AST/ast-dump-using-template.cpp
+++ clang/test/AST/ast-dump-using-template.cpp
@@ -9,8 +9,11 @@
  public:
S(T);
 };
+template struct S2 { S2(T); };
+template  S2(T t) -> S2;
 }
 using ns::S;
+using ns::S2;
 
 // TemplateName in TemplateSpecializationType.
 template
@@ -36,3 +39,10 @@
 // CHECK-NEXT:  |-DeclRefExpr {{.*}}
 // CHECK-NEXT:  `-ElaboratedType {{.*}} 'S' sugar
 // CHECK-NEXT:`-DeducedTemplateSpecializationType {{.*}} 'ns::S' 
sugar using
+
+S2 DeducedTemplateSpecializationT2(123);
+using D = decltype(DeducedTemplateSpecializationT2);
+// CHECK:  DecltypeType {{.*}}
+// CHECK-NEXT:  |-DeclRefExpr {{.*}}
+// CHECK-NEXT:  `-ElaboratedType {{.*}} 'S2' sugar
+// CHECK-NEXT:`-DeducedTemplateSpecializationType {{.*}} 'S2' sugar 
using
Index: clang/lib/AST/TemplateName.cpp
===
--- clang/lib/AST/TemplateName.cpp
+++ clang/lib/AST/TemplateName.cpp
@@ -282,7 +282,9 @@
 }
 
 void TemplateName::Profile(llvm::FoldingSetNodeID ) {
-  if (auto *TD = getAsTemplateDecl())
+  if (const auto* USD = getAsUsingShadowDecl())
+ID.AddPointer(USD->getCanonicalDecl());
+  else if (const auto *TD = getAsTemplateDecl())
 ID.AddPointer(TD->getCanonicalDecl());
   else
 ID.AddPointer(Storage.getOpaqueValue());
Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -171,6 +171,13 @@
   namespace ns {template  struct S {}; }
   using ns::$explicit^S;)cpp",
"^S x;");
+  testWalk(R"cpp(
+  namespace ns {
+template  struct S { S(T);};
+template  S(T t) -> S;
+  }
+  using ns::$explicit^S;)cpp",
+  "^S x(123);");
   testWalk("template struct $explicit^S {};",
R"cpp(
   template  typename> struct X {};


Index: clang/test/AST/ast-dump-using-template.cpp
===
--- clang/test/AST/ast-dump-using-template.cpp
+++ clang/test/AST/ast-dump-using-template.cpp
@@ -9,8 +9,11 @@
  public:
S(T);
 };
+template struct S2 { S2(T); };
+template  S2(T t) -> S2;
 }
 using ns::S;
+using ns::S2;
 
 // TemplateName in TemplateSpecializationType.
 template
@@ -36,3 +39,10 @@
 // CHECK-NEXT:  |-DeclRefExpr {{.*}}
 // CHECK-NEXT:  `-ElaboratedType {{.*}} 'S' sugar
 // CHECK-NEXT:`-DeducedTemplateSpecializationType {{.*}} 'ns::S' sugar using
+
+S2 DeducedTemplateSpecializationT2(123);
+using D = decltype(DeducedTemplateSpecializationT2);
+// CHECK:  DecltypeType {{.*}}
+// CHECK-NEXT:  |-DeclRefExpr {{.*}}
+// CHECK-NEXT:  `-ElaboratedType {{.*}} 'S2' sugar
+// CHECK-NEXT:`-DeducedTemplateSpecializationType {{.*}} 'S2' sugar using
Index: clang/lib/AST/TemplateName.cpp
===
--- clang/lib/AST/TemplateName.cpp
+++ clang/lib/AST/TemplateName.cpp
@@ -282,7 +282,9 @@
 }
 
 void TemplateName::Profile(llvm::FoldingSetNodeID ) {
-  if (auto *TD = getAsTemplateDecl())
+  if (const auto* USD = getAsUsingShadowDecl())
+ID.AddPointer(USD->getCanonicalDecl());
+  else if (const auto *TD = getAsTemplateDecl())
 ID.AddPointer(TD->getCanonicalDecl());
   else
 ID.AddPointer(Storage.getOpaqueValue());
Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -171,6 +171,13 @@
   namespace ns {template  struct S {}; }
   using ns::$explicit^S;)cpp",
"^S x;");
+  testWalk(R"cpp(
+  namespace ns {
+template  struct S { S(T);};
+template  S(T t) -> S;
+  }
+  using ns::$explicit^S;)cpp",
+  "^S x(123);");
   testWalk("template struct $explicit^S {};",
R"cpp(
   template  typename> struct X {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146202: [clang] Fix a UsingTemplate regression after 3e78fa860235431323aaf08c8fa922d75a7cfffa

2023-03-16 Thread Haojian Wu via Phabricator via cfe-commits 
hokein added a comment.

Thanks for the review.

In D146202#4198904 , @ChuanqiXu wrote:

> Yeah, this one should be correct. Modules techniques use `*::Profile` method 
> to judge if two entities are the same extensively. So it would be best to add 
> a test case for modules like we did in 
> https://reviews.llvm.org/rG3e78fa860235431323aaf08c8fa922d75a7cfffa. And it 
> doesn't matter if you are not interested, I'll take it later then.

I'm landing the patch as-is now. It would be nice for you to add a related test 
for modules afterwards (since I don't have much knowledge about clang module 
stuff).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146202/new/

https://reviews.llvm.org/D146202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146202: [clang] Fix a UsingTemplate regression after 3e78fa860235431323aaf08c8fa922d75a7cfffa

2023-03-16 Thread Chuanqi Xu via Phabricator via cfe-commits 
ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

Yeah, this one should be correct. Modules techniques use `*::Profile` method to 
judge if two entities are the same extensively. So it would be best to add a 
test case for modules like we did in 
https://reviews.llvm.org/rG3e78fa860235431323aaf08c8fa922d75a7cfffa. And it 
doesn't matter if you are not interested, I'll take it later then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146202/new/

https://reviews.llvm.org/D146202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146202: [clang] Fix a UsingTemplate regression after 3e78fa860235431323aaf08c8fa922d75a7cfffa

2023-03-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet accepted this revision.
kadircet added a subscriber: ChuanqiXu.
kadircet added a comment.
This revision is now accepted and ready to land.

FWIW, I believe this patch does the right thing by marking the 
`DeducedTemplateSpecializationType` as `using`. It's explicitly introduced into 
the global namespace through the using decl, and even before 
3e78fa860235431323aaf08c8fa922d75a7cfffa 
 we 
weren't marking them as such.

But I am still not sure about the implications of this in the modules world. So 
adding @ChuanqiXu as a reviewer in case he has more insights.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146202/new/

https://reviews.llvm.org/D146202

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146202: [clang] Fix a UsingTemplate regression after 3e78fa860235431323aaf08c8fa922d75a7cfffa

2023-03-16 Thread Haojian Wu via Phabricator via cfe-commits 
hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a project: All.
hokein requested review of this revision.
Herald added projects: clang, clang-tools-extra.

TemplateName::getAsTemplateDecl() returns the underlying TemplateDecl
for a UsingTemplate kind template name. We should respect that in the
Profile method otherwise we might desugar the template name unexpectedly
(e.g. for template argument deduction with deduciton guides).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146202

Files:
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
  clang/lib/AST/TemplateName.cpp
  clang/test/AST/ast-dump-using-template.cpp


Index: clang/test/AST/ast-dump-using-template.cpp
===
--- clang/test/AST/ast-dump-using-template.cpp
+++ clang/test/AST/ast-dump-using-template.cpp
@@ -9,8 +9,11 @@
  public:
S(T);
 };
+template struct S2 { S2(T); };
+template  S2(T t) -> S2;
 }
 using ns::S;
+using ns::S2;
 
 // TemplateName in TemplateSpecializationType.
 template
@@ -36,3 +39,10 @@
 // CHECK-NEXT:  |-DeclRefExpr {{.*}}
 // CHECK-NEXT:  `-ElaboratedType {{.*}} 'S' sugar
 // CHECK-NEXT:`-DeducedTemplateSpecializationType {{.*}} 'ns::S' 
sugar using
+
+S2 DeducedTemplateSpecializationT2(123);
+using D = decltype(DeducedTemplateSpecializationT2);
+// CHECK:  DecltypeType {{.*}}
+// CHECK-NEXT:  |-DeclRefExpr {{.*}}
+// CHECK-NEXT:  `-ElaboratedType {{.*}} 'S2' sugar
+// CHECK-NEXT:`-DeducedTemplateSpecializationType {{.*}} 'S2' sugar 
using
Index: clang/lib/AST/TemplateName.cpp
===
--- clang/lib/AST/TemplateName.cpp
+++ clang/lib/AST/TemplateName.cpp
@@ -282,7 +282,9 @@
 }
 
 void TemplateName::Profile(llvm::FoldingSetNodeID ) {
-  if (auto *TD = getAsTemplateDecl())
+  if (const auto* USD = getAsUsingShadowDecl())
+ID.AddPointer(USD->getCanonicalDecl());
+  else if (const auto *TD = getAsTemplateDecl())
 ID.AddPointer(TD->getCanonicalDecl());
   else
 ID.AddPointer(Storage.getOpaqueValue());
Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -171,6 +171,13 @@
   namespace ns {template  struct S {}; }
   using ns::$explicit^S;)cpp",
"^S x;");
+  testWalk(R"cpp(
+  namespace ns {
+template  struct S { S(T);};
+template  S(T t) -> S;
+  }
+  using ns::$explicit^S;)cpp",
+  "^S x(123);");
   testWalk("template struct $explicit^S {};",
R"cpp(
   template  typename> struct X {};


Index: clang/test/AST/ast-dump-using-template.cpp
===
--- clang/test/AST/ast-dump-using-template.cpp
+++ clang/test/AST/ast-dump-using-template.cpp
@@ -9,8 +9,11 @@
  public:
S(T);
 };
+template struct S2 { S2(T); };
+template  S2(T t) -> S2;
 }
 using ns::S;
+using ns::S2;
 
 // TemplateName in TemplateSpecializationType.
 template
@@ -36,3 +39,10 @@
 // CHECK-NEXT:  |-DeclRefExpr {{.*}}
 // CHECK-NEXT:  `-ElaboratedType {{.*}} 'S' sugar
 // CHECK-NEXT:`-DeducedTemplateSpecializationType {{.*}} 'ns::S' sugar using
+
+S2 DeducedTemplateSpecializationT2(123);
+using D = decltype(DeducedTemplateSpecializationT2);
+// CHECK:  DecltypeType {{.*}}
+// CHECK-NEXT:  |-DeclRefExpr {{.*}}
+// CHECK-NEXT:  `-ElaboratedType {{.*}} 'S2' sugar
+// CHECK-NEXT:`-DeducedTemplateSpecializationType {{.*}} 'S2' sugar using
Index: clang/lib/AST/TemplateName.cpp
===
--- clang/lib/AST/TemplateName.cpp
+++ clang/lib/AST/TemplateName.cpp
@@ -282,7 +282,9 @@
 }
 
 void TemplateName::Profile(llvm::FoldingSetNodeID ) {
-  if (auto *TD = getAsTemplateDecl())
+  if (const auto* USD = getAsUsingShadowDecl())
+ID.AddPointer(USD->getCanonicalDecl());
+  else if (const auto *TD = getAsTemplateDecl())
 ID.AddPointer(TD->getCanonicalDecl());
   else
 ID.AddPointer(Storage.getOpaqueValue());
Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -171,6 +171,13 @@
   namespace ns {template  struct S {}; }
   using ns::$explicit^S;)cpp",
"^S x;");
+  testWalk(R"cpp(
+  namespace ns {
+template  struct S { S(T);};
+template  S(T t) -> S;
+  }
+  using ns::$explicit^S;)cpp",
+  "^S x(123);");
   testWalk("template struct $explicit^S {};",
R"cpp(
   template  typename> struct X {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org