[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-03-04 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian closed 
https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-03-04 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian updated 
https://github.com/llvm/llvm-project/pull/79683

>From 7d05842505dbcabcc54cb365006c794ab9371983 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski 
Date: Tue, 16 Jan 2024 08:05:33 -0500
Subject: [PATCH] Reapply "[Clang][Sema] Diagnose function/variable templates
 that shadow their own template parameters (#78274)"

---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticSemaKinds.td|  3 ++
 clang/include/clang/Sema/Scope.h  |  7 
 clang/include/clang/Sema/Sema.h   | 16 -
 clang/lib/Sema/Scope.cpp  |  3 ++
 clang/lib/Sema/SemaDecl.cpp   | 31 +---
 clang/lib/Sema/SemaDeclCXX.cpp| 13 +++
 clang/lib/Sema/SemaTemplate.cpp   | 36 +--
 .../test/CXX/temp/temp.res/temp.local/p6.cpp  | 22 +---
 9 files changed, 88 insertions(+), 45 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c239c97f7afc91..b06de6c3c6e5d3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -42,6 +42,8 @@ C/C++ Language Potentially Breaking Changes
 
 C++ Specific Potentially Breaking Changes
 -
+- Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
+  This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
 ABI Changes in This Version
 ---
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 91105d4231f06a..8ea194ceadb5d1 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4977,6 +4977,9 @@ def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
 def ext_template_param_shadow : ExtWarn<
   err_template_param_shadow.Summary>, InGroup;
+def ext_compat_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Summary>, InGroup<
+  DiagGroup<"strict-primary-template-shadow">>, DefaultError;
 def note_template_param_here : Note<"template parameter is declared here">;
 def note_template_param_external : Note<
   "template parameter from hidden source: %0">;
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 536c12cb9d64e1..099c2739e8603a 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -206,6 +206,10 @@ class Scope {
   /// other template parameter scopes as parents.
   Scope *TemplateParamParent;
 
+  /// DeclScopeParent - This is a direct link to the immediately containing
+  /// DeclScope, i.e. scope which can contain declarations.
+  Scope *DeclParent;
+
   /// DeclsInScope - This keeps track of all declarations in this scope.  When
   /// the declaration is added to the scope, it is set as the current
   /// declaration for the identifier in the IdentifierTable.  When the scope is
@@ -305,6 +309,9 @@ class Scope {
   Scope *getTemplateParamParent() { return TemplateParamParent; }
   const Scope *getTemplateParamParent() const { return TemplateParamParent; }
 
+  Scope *getDeclParent() { return DeclParent; }
+  const Scope *getDeclParent() const { return DeclParent; }
+
   /// Returns the depth of this scope. The translation-unit has scope depth 0.
   unsigned getDepth() const { return Depth; }
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ef4b93fac95ce5..29baf542f6cbf5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8386,7 +8386,21 @@ class Sema final {
   TemplateSpecializationKind TSK,
   bool Complain = true);
 
-  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl);
+  /// DiagnoseTemplateParameterShadow - Produce a diagnostic complaining
+  /// that the template parameter 'PrevDecl' is being shadowed by a new
+  /// declaration at location Loc. Returns true to indicate that this is
+  /// an error, and false otherwise.
+  ///
+  /// \param Loc The location of the declaration that shadows a template
+  ///parameter.
+  ///
+  /// \param PrevDecl The template parameter that the declaration shadows.
+  ///
+  /// \param SupportedForCompatibility Whether to issue the diagnostic as
+  ///a warning for compatibility with older versions of clang.
+  ///Ignored when MSVC compatibility is enabled.
+  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool SupportedForCompatibility = false);
   TemplateDecl *AdjustDeclIfTemplate(Decl *);
 
   NamedDecl *ActOnTypeParameter(Scope *S, bool Typename,
diff --git a/clang/lib/Sema/Scope.cpp b/clang/lib/Sema/Scope.cpp
index

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-03-04 Thread via cfe-commits 

https://github.com/cor3ntin approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-26 Thread Krystian Stasiowski via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

sdkrystian wrote:

@AaronBallman should I still wait for @cor3ntin to reply?

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-20 Thread Krystian Stasiowski via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

sdkrystian wrote:

Ping @cor3ntin 

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-15 Thread Krystian Stasiowski via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

sdkrystian wrote:

I applied the suggested changes, so this should be good to go once cor3ntin 
responds 

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-15 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian updated 
https://github.com/llvm/llvm-project/pull/79683

>From 0d4a6d155b5d70970b63f4337507098ea938f627 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski 
Date: Tue, 16 Jan 2024 08:05:33 -0500
Subject: [PATCH] Reapply "[Clang][Sema] Diagnose function/variable templates
 that shadow their own template parameters (#78274)"

---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticSemaKinds.td|  3 ++
 clang/include/clang/Sema/Scope.h  |  7 
 clang/include/clang/Sema/Sema.h   | 16 -
 clang/lib/Sema/Scope.cpp  |  3 ++
 clang/lib/Sema/SemaDecl.cpp   | 31 +---
 clang/lib/Sema/SemaDeclCXX.cpp| 13 +++
 clang/lib/Sema/SemaTemplate.cpp   | 36 +--
 .../test/CXX/temp/temp.res/temp.local/p6.cpp  | 22 +---
 9 files changed, 88 insertions(+), 45 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9473867c1f231f..71654bc7bef345 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -42,6 +42,8 @@ C/C++ Language Potentially Breaking Changes
 
 C++ Specific Potentially Breaking Changes
 -
+- Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
+  This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
 ABI Changes in This Version
 ---
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1c0ebbe4e68343..e71218c390f722 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4966,6 +4966,9 @@ def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
 def ext_template_param_shadow : ExtWarn<
   err_template_param_shadow.Summary>, InGroup;
+def ext_compat_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Summary>, InGroup<
+  DiagGroup<"strict-primary-template-shadow">>, DefaultError;
 def note_template_param_here : Note<"template parameter is declared here">;
 def note_template_param_external : Note<
   "template parameter from hidden source: %0">;
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 9e81706cd2aa1d..27c73b537a8636 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -200,6 +200,10 @@ class Scope {
   /// other template parameter scopes as parents.
   Scope *TemplateParamParent;
 
+  /// DeclScopeParent - This is a direct link to the immediately containing
+  /// DeclScope, i.e. scope which can contain declarations.
+  Scope *DeclParent;
+
   /// DeclsInScope - This keeps track of all declarations in this scope.  When
   /// the declaration is added to the scope, it is set as the current
   /// declaration for the identifier in the IdentifierTable.  When the scope is
@@ -299,6 +303,9 @@ class Scope {
   Scope *getTemplateParamParent() { return TemplateParamParent; }
   const Scope *getTemplateParamParent() const { return TemplateParamParent; }
 
+  Scope *getDeclParent() { return DeclParent; }
+  const Scope *getDeclParent() const { return DeclParent; }
+
   /// Returns the depth of this scope. The translation-unit has scope depth 0.
   unsigned getDepth() const { return Depth; }
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 59eab0185ae632..711e6f17440a65 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8315,7 +8315,21 @@ class Sema final {
   TemplateSpecializationKind TSK,
   bool Complain = true);
 
-  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl);
+  /// DiagnoseTemplateParameterShadow - Produce a diagnostic complaining
+  /// that the template parameter 'PrevDecl' is being shadowed by a new
+  /// declaration at location Loc. Returns true to indicate that this is
+  /// an error, and false otherwise.
+  ///
+  /// \param Loc The location of the declaration that shadows a template
+  ///parameter.
+  ///
+  /// \param PrevDecl The template parameter that the declaration shadows.
+  ///
+  /// \param SupportedForCompatibility Whether to issue the diagnostic as
+  ///a warning for compatibility with older versions of clang.
+  ///Ignored when MSVC compatibility is enabled.
+  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool SupportedForCompatibility = false);
   TemplateDecl *AdjustDeclIfTemplate(Decl *);
 
   NamedDecl *ActOnTypeParameter(Scope *S, bool Typename,
diff --git a/clang/lib/Sema/Scope.cpp b/clang/lib/Sema/Scope.cpp
index

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-15 Thread Aaron Ballman via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

AaronBallman wrote:

> @AaronBallman In VS20222 the javadocs show up even when they are attached to 
> the definition... but I'll move them to the declaration to be on the safe 
> side :)

For me, that only happens if the definition is in the same TU as the use. When 
I tried outside of SemaTemplate.cpp (such as the uses in SemaDecl.cpp), no docs 
appeared in the tooltip.

> While I mostly agree with changing the name to SupportedAsExtension, I don't 
> think the change to the diagnostic selection logic is quite right. When MSVC 
> compatibility is enabled, template parameter shadowing is always allowed 
> (read: there are no cases where shadowing would be an error with MSVC 
> compatibility enabled), so we should issue the MSVC specific diagnostic 
> regardless of SupportedAsExtension.

Good point!

> Perhaps we name it SupportedForCompatibility instead?

I could be okay with that; @cor3ntin WDYT?

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-15 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian edited 
https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-15 Thread Krystian Stasiowski via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

sdkrystian wrote:

@AaronBallman In VS20222 the javadocs show up even when they are attached to 
the definition... but I'll move them to the declaration to be on the safe side 
:)

While I agree with changing the name to `SupportedAsExtension`, I don't think 
the change to the diagnostic selection logic is quite right. When MSVC 
compatibility **is** enabled, template parameter shadowing is _always_ allowed 
(read: there are no cases where shadowing would be an error with MSVC 
compatibility enabled), so we should issue the MSVC specific diagnostic 
regardless of `SupportedAsExtension`.  `SupportedAsExtension` should be used to 
select between the error/extension diagnostic only when MSVC compatibility **is 
not** enabled. Perhaps we name it `SupportedForCompatibility` instead? 



https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-15 Thread Aaron Ballman via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

AaronBallman wrote:

The javadocs don't help too much because they're in the source file rather than 
attached to the declaration (so they don't show up in all IDEs, like Visual 
Studio). We should move them to the header file.

I would recommend we change the logic slightly and name the parameter 
`SupportedAsExtension`. Then the diagnostic selection becomes 
`SupportedAsExtension ? (MSVC ? msvc_ext_diag : generic_ext_diag) : err_diag` 
and it's easier for the caller to reason about whether this shadowing should be 
supported as an extension or not in any given case.

WDYT?

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-12 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

Ping @cor3ntin 

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-02-05 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

Ping @cor3ntin 

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-30 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

@cor3ntin Addressed review comments... Please approve if it all checks out :)

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-30 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian updated 
https://github.com/llvm/llvm-project/pull/79683

>From a9a6b6ea71ef57eabd136d3b00a9dad0011a86e5 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski 
Date: Tue, 16 Jan 2024 08:05:33 -0500
Subject: [PATCH] Reapply "[Clang][Sema] Diagnose function/variable templates
 that shadow their own template parameters (#78274)"

---
 clang/docs/ReleaseNotes.rst   |  2 +
 .../clang/Basic/DiagnosticSemaKinds.td|  3 ++
 clang/include/clang/Sema/Scope.h  |  7 
 clang/include/clang/Sema/Sema.h   |  3 +-
 clang/lib/Sema/Scope.cpp  |  3 ++
 clang/lib/Sema/SemaDecl.cpp   | 31 --
 clang/lib/Sema/SemaDeclCXX.cpp| 13 ++
 clang/lib/Sema/SemaTemplate.cpp   | 40 ---
 .../test/CXX/temp/temp.res/temp.local/p6.cpp  | 22 --
 9 files changed, 83 insertions(+), 41 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9473867c1f231..71654bc7bef34 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -42,6 +42,8 @@ C/C++ Language Potentially Breaking Changes
 
 C++ Specific Potentially Breaking Changes
 -
+- Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
+  This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
 ABI Changes in This Version
 ---
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1c0ebbe4e6834..e71218c390f72 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4966,6 +4966,9 @@ def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
 def ext_template_param_shadow : ExtWarn<
   err_template_param_shadow.Summary>, InGroup;
+def ext_compat_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Summary>, InGroup<
+  DiagGroup<"strict-primary-template-shadow">>, DefaultError;
 def note_template_param_here : Note<"template parameter is declared here">;
 def note_template_param_external : Note<
   "template parameter from hidden source: %0">;
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 9e81706cd2aa1..27c73b537a863 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -200,6 +200,10 @@ class Scope {
   /// other template parameter scopes as parents.
   Scope *TemplateParamParent;
 
+  /// DeclScopeParent - This is a direct link to the immediately containing
+  /// DeclScope, i.e. scope which can contain declarations.
+  Scope *DeclParent;
+
   /// DeclsInScope - This keeps track of all declarations in this scope.  When
   /// the declaration is added to the scope, it is set as the current
   /// declaration for the identifier in the IdentifierTable.  When the scope is
@@ -299,6 +303,9 @@ class Scope {
   Scope *getTemplateParamParent() { return TemplateParamParent; }
   const Scope *getTemplateParamParent() const { return TemplateParamParent; }
 
+  Scope *getDeclParent() { return DeclParent; }
+  const Scope *getDeclParent() const { return DeclParent; }
+
   /// Returns the depth of this scope. The translation-unit has scope depth 0.
   unsigned getDepth() const { return Depth; }
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 59eab0185ae63..c016c9cf6dccd 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8315,7 +8315,8 @@ class Sema final {
   TemplateSpecializationKind TSK,
   bool Complain = true);
 
-  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl);
+  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning = false);
   TemplateDecl *AdjustDeclIfTemplate(Decl *);
 
   NamedDecl *ActOnTypeParameter(Scope *S, bool Typename,
diff --git a/clang/lib/Sema/Scope.cpp b/clang/lib/Sema/Scope.cpp
index 4570d8c615fe5..a8c318dcc5327 100644
--- a/clang/lib/Sema/Scope.cpp
+++ b/clang/lib/Sema/Scope.cpp
@@ -37,6 +37,7 @@ void Scope::setFlags(Scope *parent, unsigned flags) {
 FnParent   = parent->FnParent;
 BlockParent= parent->BlockParent;
 TemplateParamParent = parent->TemplateParamParent;
+DeclParent = parent->DeclParent;
 MSLastManglingParent = parent->MSLastManglingParent;
 MSCurManglingNumber = getMSLastManglingNumber();
 if ((Flags & (FnScope | ClassScope | BlockScope | TemplateParamScope |
@@ -52,6 +53,7 @@ void Scope::setFlags(Scope *parent, unsigned flags) {
 PrototypeIndex = 0;
 MSLastManglingParent = FnParent = BlockParent = nullptr;

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-30 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian updated 
https://github.com/llvm/llvm-project/pull/79683

>From 60e72f20f5bf74a3e62688cfcaa2bc623f0faff1 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski 
Date: Tue, 16 Jan 2024 08:05:33 -0500
Subject: [PATCH] Reapply "[Clang][Sema] Diagnose function/variable templates
 that shadow their own template parameters (#78274)"

---
 clang/docs/ReleaseNotes.rst   |  2 +
 .../clang/Basic/DiagnosticSemaKinds.td|  3 ++
 clang/include/clang/Sema/Scope.h  |  7 
 clang/include/clang/Sema/Sema.h   |  3 +-
 clang/lib/Sema/Scope.cpp  |  3 ++
 clang/lib/Sema/SemaDecl.cpp   | 32 +--
 clang/lib/Sema/SemaDeclCXX.cpp| 13 ++
 clang/lib/Sema/SemaTemplate.cpp   | 40 ---
 .../test/CXX/temp/temp.res/temp.local/p6.cpp  | 22 --
 9 files changed, 84 insertions(+), 41 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9473867c1f231..71654bc7bef34 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -42,6 +42,8 @@ C/C++ Language Potentially Breaking Changes
 
 C++ Specific Potentially Breaking Changes
 -
+- Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
+  This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
 ABI Changes in This Version
 ---
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1c0ebbe4e6834..e71218c390f72 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4966,6 +4966,9 @@ def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
 def ext_template_param_shadow : ExtWarn<
   err_template_param_shadow.Summary>, InGroup;
+def ext_compat_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Summary>, InGroup<
+  DiagGroup<"strict-primary-template-shadow">>, DefaultError;
 def note_template_param_here : Note<"template parameter is declared here">;
 def note_template_param_external : Note<
   "template parameter from hidden source: %0">;
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 9e81706cd2aa1..27c73b537a863 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -200,6 +200,10 @@ class Scope {
   /// other template parameter scopes as parents.
   Scope *TemplateParamParent;
 
+  /// DeclScopeParent - This is a direct link to the immediately containing
+  /// DeclScope, i.e. scope which can contain declarations.
+  Scope *DeclParent;
+
   /// DeclsInScope - This keeps track of all declarations in this scope.  When
   /// the declaration is added to the scope, it is set as the current
   /// declaration for the identifier in the IdentifierTable.  When the scope is
@@ -299,6 +303,9 @@ class Scope {
   Scope *getTemplateParamParent() { return TemplateParamParent; }
   const Scope *getTemplateParamParent() const { return TemplateParamParent; }
 
+  Scope *getDeclParent() { return DeclParent; }
+  const Scope *getDeclParent() const { return DeclParent; }
+
   /// Returns the depth of this scope. The translation-unit has scope depth 0.
   unsigned getDepth() const { return Depth; }
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 59eab0185ae63..c016c9cf6dccd 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8315,7 +8315,8 @@ class Sema final {
   TemplateSpecializationKind TSK,
   bool Complain = true);
 
-  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl);
+  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning = false);
   TemplateDecl *AdjustDeclIfTemplate(Decl *);
 
   NamedDecl *ActOnTypeParameter(Scope *S, bool Typename,
diff --git a/clang/lib/Sema/Scope.cpp b/clang/lib/Sema/Scope.cpp
index 4570d8c615fe5..a8c318dcc5327 100644
--- a/clang/lib/Sema/Scope.cpp
+++ b/clang/lib/Sema/Scope.cpp
@@ -37,6 +37,7 @@ void Scope::setFlags(Scope *parent, unsigned flags) {
 FnParent   = parent->FnParent;
 BlockParent= parent->BlockParent;
 TemplateParamParent = parent->TemplateParamParent;
+DeclParent = parent->DeclParent;
 MSLastManglingParent = parent->MSLastManglingParent;
 MSCurManglingNumber = getMSLastManglingNumber();
 if ((Flags & (FnScope | ClassScope | BlockScope | TemplateParamScope |
@@ -52,6 +53,7 @@ void Scope::setFlags(Scope *parent, unsigned flags) {
 PrototypeIndex = 0;
 MSLastManglingParent = FnParent = BlockParent = nullptr;

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread Krystian Stasiowski via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

sdkrystian wrote:

I can add javadocs for the parameters that explain the semantics of 
`IssueWarning` and the motivating use-case for it. Is that a good compromise?

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

cor3ntin wrote:

I don't care much about the exact name, although i agree with you my suggestion 
may not be great.  It can be `ShadowsNameOfFunctionOrVariableItIsAttachedTo` or 
whatever name you think is best describing of what it does. `IssueWarning` is a 
bit _too_ generic, or rather it forces us to think about warning in multiple 
places and especially someone calling `DiagnoseTemplateParameterShadow` should 
understand what the bool parameter does.

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread Krystian Stasiowski via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

sdkrystian wrote:

I think the more generic `IssueWarning` is less ambiguous. We only issue the 
warning for variable/function templates the shadow _their own_ template 
parameters. For example:
```cpp
template
struct A
{
template void U(); // warning
template void T(); // error
void T(); // error
};
```
`ShadowsFunctionOrVariableName` establishes the expectation that its value will 
be `true` for all the above function declarations (its only `true` for the 
first one).

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread Krystian Stasiowski via cfe-commits 


@@ -6535,6 +6543,12 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
   if (getLangOpts().CPlusPlus)
 CheckExtraCXXDefaultArguments(D);
 
+  // The scope passed in may not be a decl scope.  Zip up the scope tree until
+  // we find one that is.
+  while ((S->getFlags() & Scope::DeclScope) == 0 ||
+ (S->getFlags() & Scope::TemplateParamScope) != 0)
+S = S->getParent();

sdkrystian wrote:

I was planning to do that in a separate PR... but I could just throw in here I 
suppose 

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {
   assert(PrevDecl->isTemplateParameter() && "Not a template parameter");
 
   // C++ [temp.local]p4:
   //   A template-parameter shall not be redeclared within its
   //   scope (including nested scopes).
   //
   // Make this a warning when MSVC compatibility is requested.
-  unsigned DiagId = getLangOpts().MSVCCompat ? diag::ext_template_param_shadow
- : diag::err_template_param_shadow;
+  unsigned DiagId = getLangOpts().MSVCCompat
+? diag::ext_template_param_shadow
+: (IssueWarning ? 
diag::ext_compat_template_param_shadow

cor3ntin wrote:

A comment there too would not hurt

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread via cfe-commits 


@@ -6535,6 +6543,12 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
   if (getLangOpts().CPlusPlus)
 CheckExtraCXXDefaultArguments(D);
 
+  // The scope passed in may not be a decl scope.  Zip up the scope tree until
+  // we find one that is.
+  while ((S->getFlags() & Scope::DeclScope) == 0 ||
+ (S->getFlags() & Scope::TemplateParamScope) != 0)
+S = S->getParent();

cor3ntin wrote:

We do that in at least 3 places (search for `(S->getFlags() & 
Scope::DeclScope)` )
Can we add a `Scope::getInnermostEnclosingDeclarativeScope()` or something like 
that ?

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread via cfe-commits 


@@ -885,16 +885,19 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {

cor3ntin wrote:

Can this be called `ShadowsFunctionOrVariableName` or something like that ? It 
might be clearer

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian updated 
https://github.com/llvm/llvm-project/pull/79683

>From 391254191932c8ca325346978de5c57b233cbab8 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski 
Date: Tue, 16 Jan 2024 08:05:33 -0500
Subject: [PATCH] Reapply "[Clang][Sema] Diagnose function/variable templates
 that shadow their own template parameters (#78274)"

---
 clang/docs/ReleaseNotes.rst   |  2 ++
 .../clang/Basic/DiagnosticSemaKinds.td|  3 ++
 clang/include/clang/Sema/Sema.h   |  3 +-
 clang/lib/Sema/SemaDecl.cpp   | 34 +--
 clang/lib/Sema/SemaTemplate.cpp   |  9 +++--
 .../test/CXX/temp/temp.res/temp.local/p6.cpp  | 22 +---
 6 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5330cd9caad8011..0fd8f69930ddfcf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -42,6 +42,8 @@ C/C++ Language Potentially Breaking Changes
 
 C++ Specific Potentially Breaking Changes
 -
+- Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
+  This error can be disabled via `-Wno-strict-primary-template-shadow` for 
compatibility with previous versions of clang.
 
 ABI Changes in This Version
 ---
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 649de9f47c46195..4e3b20548913ab5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4966,6 +4966,9 @@ def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
 def ext_template_param_shadow : ExtWarn<
   err_template_param_shadow.Summary>, InGroup;
+def ext_compat_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Summary>, InGroup<
+  DiagGroup<"strict-primary-template-shadow">>, DefaultError;
 def note_template_param_here : Note<"template parameter is declared here">;
 def note_template_param_external : Note<
   "template parameter from hidden source: %0">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1f1cbd11ff73581..d0fb4f09916eda4 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8256,7 +8256,8 @@ class Sema final {
   TemplateSpecializationKind TSK,
   bool Complain = true);
 
-  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl);
+  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning = false);
   TemplateDecl *AdjustDeclIfTemplate(Decl *);
 
   NamedDecl *ActOnTypeParameter(Scope *S, bool Typename,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f9bf1d14bdc4f68..2e10a3057cfbee6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6377,12 +6377,6 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
   } else if (DiagnoseUnexpandedParameterPack(NameInfo, UPPC_DeclarationType))
 return nullptr;
 
-  // The scope passed in may not be a decl scope.  Zip up the scope tree until
-  // we find one that is.
-  while ((S->getFlags() & Scope::DeclScope) == 0 ||
- (S->getFlags() & Scope::TemplateParamScope) != 0)
-S = S->getParent();
-
   DeclContext *DC = CurContext;
   if (D.getCXXScopeSpec().isInvalid())
 D.setInvalidType();
@@ -6506,12 +6500,26 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
 RemoveUsingDecls(Previous);
   }
 
-  if (Previous.isSingleResult() &&
-  Previous.getFoundDecl()->isTemplateParameter()) {
+  // if (Previous.isSingleResult() &&
+  //Previous.getFoundDecl()->isTemplateParameter()) {
+  if (auto *TPD = Previous.getAsSingle();
+  TPD && TPD->isTemplateParameter()) {
+// Older versions of clang allowed the names of function/variable templates
+// to shadow the names of their template parameters. For the compatibility
+// purposes we detect such cases and issue a default-to-error warning that
+// can be disabled with -Wno-strict-primary-template-shadow.
+bool IssueShadowingWarning = false;
+for (Scope *Inner = S; (Inner->getFlags() & Scope::DeclScope) == 0 ||
+   Inner->isTemplateParamScope();
+ Inner = Inner->getParent()) {
+  if (IssueShadowingWarning = Inner->isDeclScope(TPD))
+break;
+}
+
 // Maybe we will complain about the shadowed template parameter.
 if (!D.isInvalidType())
-  DiagnoseTemplateParameterShadow(D.getIdentifierLoc(),
-  Previous.getFoundDecl());
+  DiagnoseTemplateParameterShadow(D.getIdentifierLoc(), TPD,
+  IssueShadowingWarning);

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread Krystian Stasiowski via cfe-commits 


@@ -6506,12 +6500,25 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
 RemoveUsingDecls(Previous);
   }
 
-  if (Previous.isSingleResult() &&
-  Previous.getFoundDecl()->isTemplateParameter()) {
+  // if (Previous.isSingleResult() &&
+  //Previous.getFoundDecl()->isTemplateParameter()) {
+  if (auto *TPD = Previous.getAsSingle(); TPD && 
TPD->isTemplateParameter()) {
+// Older versions of clang allowed the names of function/variable templates
+// to shadow the names of their template parameters. For the compatibility 
purposes
+// we detect such cases and issue a default-to-error warning that can be 
disabled with
+// -fno-strict-primary-template-shadow.

sdkrystian wrote:

Yes :)

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread via cfe-commits 


@@ -6506,12 +6500,25 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
 RemoveUsingDecls(Previous);
   }
 
-  if (Previous.isSingleResult() &&
-  Previous.getFoundDecl()->isTemplateParameter()) {
+  // if (Previous.isSingleResult() &&
+  //Previous.getFoundDecl()->isTemplateParameter()) {
+  if (auto *TPD = Previous.getAsSingle(); TPD && 
TPD->isTemplateParameter()) {
+// Older versions of clang allowed the names of function/variable templates
+// to shadow the names of their template parameters. For the compatibility 
purposes
+// we detect such cases and issue a default-to-error warning that can be 
disabled with
+// -fno-strict-primary-template-shadow.

cor3ntin wrote:

Do you mean `-Wno-strict-primary-template-shadow` here?

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian edited 
https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread Krystian Stasiowski via cfe-commits 

sdkrystian wrote:

Need to run clang-format on this and mention the flag in the release notes, but 
should be otherwise GTG

https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread via cfe-commits 

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff faef68bca852d08511ea0311d8a0d221cb202e73 
abc8f062add9e41ce00b9d035c796256a62859ef -- clang/include/clang/Sema/Sema.h 
clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaTemplate.cpp 
clang/test/CXX/temp/temp.res/temp.local/p6.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5c148db481..d0fb4f0991 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8256,7 +8256,8 @@ public:
   TemplateSpecializationKind TSK,
   bool Complain = true);
 
-  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl, 
bool IssueWarning = false);
+  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning = false);
   TemplateDecl *AdjustDeclIfTemplate(Decl *);
 
   NamedDecl *ActOnTypeParameter(Scope *S, bool Typename,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index de65b3c09c..2cb971b052 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6502,22 +6502,23 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
 
   // if (Previous.isSingleResult() &&
   //Previous.getFoundDecl()->isTemplateParameter()) {
-  if (auto *TPD = Previous.getAsSingle(); TPD && 
TPD->isTemplateParameter()) {
+  if (auto *TPD = Previous.getAsSingle();
+  TPD && TPD->isTemplateParameter()) {
 // Older versions of clang allowed the names of function/variable templates
-// to shadow the names of their template parameters. For the compatibility 
purposes
-// we detect such cases and issue a default-to-error warning that can be 
disabled with
-// -fno-strict-primary-template-shadow.
+// to shadow the names of their template parameters. For the compatibility
+// purposes we detect such cases and issue a default-to-error warning that
+// can be disabled with -fno-strict-primary-template-shadow.
 bool IssueShadowingWarning = false;
 for (Scope *Inner = S; (Inner->getFlags() & Scope::DeclScope) == 0 ||
- Inner->isTemplateParamScope(); Inner = Inner->getParent()) {
+   Inner->isTemplateParamScope();
+ Inner = Inner->getParent()) {
   if (IssueShadowingWarning = Inner->isDeclScope(TPD))
 break;
 }
 
 // Maybe we will complain about the shadowed template parameter.
 if (!D.isInvalidType())
-  DiagnoseTemplateParameterShadow(D.getIdentifierLoc(),
-  TPD,
+  DiagnoseTemplateParameterShadow(D.getIdentifierLoc(), TPD,
   IssueShadowingWarning);
 
 // Just pretend that we didn't see the previous declaration.
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 4ba8dfc19d..2993da99ff 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -885,7 +885,8 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation 
PointOfInstantiation,
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl, 
bool IssueWarning) {
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+   bool IssueWarning) {
   assert(PrevDecl->isTemplateParameter() && "Not a template parameter");
 
   // C++ [temp.local]p4:
@@ -894,10 +895,9 @@ void Sema::DiagnoseTemplateParameterShadow(SourceLocation 
Loc, Decl *PrevDecl, b
   //
   // Make this a warning when MSVC compatibility is requested.
   unsigned DiagId = getLangOpts().MSVCCompat
-  ? diag::ext_template_param_shadow
-  : (IssueWarning
-  ? diag::ext_compat_template_param_shadow
-  : diag::err_template_param_shadow);
+? diag::ext_template_param_shadow
+: (IssueWarning ? 
diag::ext_compat_template_param_shadow
+: diag::err_template_param_shadow);
   const auto *ND = cast(PrevDecl);
   Diag(Loc, DiagId) << ND->getDeclName();
   NoteTemplateParameterLocation(*ND);

``




https://github.com/llvm/llvm-project/pull/79683
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)


Changes

Reapplies #78274 with the addition of a default-error warning 
(`strict-primary-template-shadow`) that is issued for instances of shadowing 
which were previously accepted prior to this patch.


---
Full diff: https://github.com/llvm/llvm-project/pull/79683.diff


6 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+1) 
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3) 
- (modified) clang/include/clang/Sema/Sema.h (+1-1) 
- (modified) clang/lib/Sema/SemaDecl.cpp (+22-9) 
- (modified) clang/lib/Sema/SemaTemplate.cpp (+6-3) 
- (modified) clang/test/CXX/temp/temp.res/temp.local/p6.cpp (+18-4) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5330cd9caad801..24d3ada5bfd9df 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -97,6 +97,7 @@ Attribute Changes in Clang
 
 Improvements to Clang's diagnostics
 ---
+- Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
 
 Improvements to Clang's time-trace
 --
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 649de9f47c4619..4e3b20548913ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4966,6 +4966,9 @@ def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
 def ext_template_param_shadow : ExtWarn<
   err_template_param_shadow.Summary>, InGroup;
+def ext_compat_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Summary>, InGroup<
+  DiagGroup<"strict-primary-template-shadow">>, DefaultError;
 def note_template_param_here : Note<"template parameter is declared here">;
 def note_template_param_external : Note<
   "template parameter from hidden source: %0">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1f1cbd11ff7358..5c148db48140ed 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8256,7 +8256,7 @@ class Sema final {
   TemplateSpecializationKind TSK,
   bool Complain = true);
 
-  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl);
+  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl, 
bool IssueWarning = false);
   TemplateDecl *AdjustDeclIfTemplate(Decl *);
 
   NamedDecl *ActOnTypeParameter(Scope *S, bool Typename,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f9bf1d14bdc4f6..de65b3c09c28cf 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6377,12 +6377,6 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
   } else if (DiagnoseUnexpandedParameterPack(NameInfo, UPPC_DeclarationType))
 return nullptr;
 
-  // The scope passed in may not be a decl scope.  Zip up the scope tree until
-  // we find one that is.
-  while ((S->getFlags() & Scope::DeclScope) == 0 ||
- (S->getFlags() & Scope::TemplateParamScope) != 0)
-S = S->getParent();
-
   DeclContext *DC = CurContext;
   if (D.getCXXScopeSpec().isInvalid())
 D.setInvalidType();
@@ -6506,12 +6500,25 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
 RemoveUsingDecls(Previous);
   }
 
-  if (Previous.isSingleResult() &&
-  Previous.getFoundDecl()->isTemplateParameter()) {
+  // if (Previous.isSingleResult() &&
+  //Previous.getFoundDecl()->isTemplateParameter()) {
+  if (auto *TPD = Previous.getAsSingle(); TPD && 
TPD->isTemplateParameter()) {
+// Older versions of clang allowed the names of function/variable templates
+// to shadow the names of their template parameters. For the compatibility 
purposes
+// we detect such cases and issue a default-to-error warning that can be 
disabled with
+// -fno-strict-primary-template-shadow.
+bool IssueShadowingWarning = false;
+for (Scope *Inner = S; (Inner->getFlags() & Scope::DeclScope) == 0 ||
+ Inner->isTemplateParamScope(); Inner = Inner->getParent()) {
+  if (IssueShadowingWarning = Inner->isDeclScope(TPD))
+break;
+}
+
 // Maybe we will complain about the shadowed template parameter.
 if (!D.isInvalidType())
   DiagnoseTemplateParameterShadow(D.getIdentifierLoc(),
-  Previous.getFoundDecl());
+  TPD,
+  IssueShadowingWarning);
 
 // Just pretend that we didn't see the previous declaration.
 Previous.clear();
@@ -6535,6 +6542,12 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
   if (getLangOpts().CPlusPlus)
 CheckExtraCXXDefaultArguments(D);
 
+  // The scope passed in may

[clang] Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (PR #79683)

2024-01-27 Thread Krystian Stasiowski via cfe-commits 

https://github.com/sdkrystian created 
https://github.com/llvm/llvm-project/pull/79683

Reapplies #78274 with the addition of a default-error warning 
(`strict-primary-template-shadow`) that is issued for instances of shadowing 
which were previously accepted prior to this patch.


>From abc8f062add9e41ce00b9d035c796256a62859ef Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski 
Date: Tue, 16 Jan 2024 08:05:33 -0500
Subject: [PATCH] Reapply "[Clang][Sema] Diagnose function/variable templates
 that shadow their own template parameters (#78274)"

---
 clang/docs/ReleaseNotes.rst   |  1 +
 .../clang/Basic/DiagnosticSemaKinds.td|  3 ++
 clang/include/clang/Sema/Sema.h   |  2 +-
 clang/lib/Sema/SemaDecl.cpp   | 31 +--
 clang/lib/Sema/SemaTemplate.cpp   |  9 --
 .../test/CXX/temp/temp.res/temp.local/p6.cpp  | 22 ++---
 6 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5330cd9caad8011..24d3ada5bfd9dfc 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -97,6 +97,7 @@ Attribute Changes in Clang
 
 Improvements to Clang's diagnostics
 ---
+- Clang now diagnoses function/variable templates that shadow their own 
template parameters, e.g. ``template void T();``.
 
 Improvements to Clang's time-trace
 --
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 649de9f47c46195..4e3b20548913ab5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4966,6 +4966,9 @@ def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
 def ext_template_param_shadow : ExtWarn<
   err_template_param_shadow.Summary>, InGroup;
+def ext_compat_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Summary>, InGroup<
+  DiagGroup<"strict-primary-template-shadow">>, DefaultError;
 def note_template_param_here : Note<"template parameter is declared here">;
 def note_template_param_external : Note<
   "template parameter from hidden source: %0">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1f1cbd11ff73581..5c148db48140ed4 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8256,7 +8256,7 @@ class Sema final {
   TemplateSpecializationKind TSK,
   bool Complain = true);
 
-  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl);
+  void DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl, 
bool IssueWarning = false);
   TemplateDecl *AdjustDeclIfTemplate(Decl *);
 
   NamedDecl *ActOnTypeParameter(Scope *S, bool Typename,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f9bf1d14bdc4f68..de65b3c09c28cfe 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6377,12 +6377,6 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
   } else if (DiagnoseUnexpandedParameterPack(NameInfo, UPPC_DeclarationType))
 return nullptr;
 
-  // The scope passed in may not be a decl scope.  Zip up the scope tree until
-  // we find one that is.
-  while ((S->getFlags() & Scope::DeclScope) == 0 ||
- (S->getFlags() & Scope::TemplateParamScope) != 0)
-S = S->getParent();
-
   DeclContext *DC = CurContext;
   if (D.getCXXScopeSpec().isInvalid())
 D.setInvalidType();
@@ -6506,12 +6500,25 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator 
,
 RemoveUsingDecls(Previous);
   }
 
-  if (Previous.isSingleResult() &&
-  Previous.getFoundDecl()->isTemplateParameter()) {
+  // if (Previous.isSingleResult() &&
+  //Previous.getFoundDecl()->isTemplateParameter()) {
+  if (auto *TPD = Previous.getAsSingle(); TPD && 
TPD->isTemplateParameter()) {
+// Older versions of clang allowed the names of function/variable templates
+// to shadow the names of their template parameters. For the compatibility 
purposes
+// we detect such cases and issue a default-to-error warning that can be 
disabled with
+// -fno-strict-primary-template-shadow.
+bool IssueShadowingWarning = false;
+for (Scope *Inner = S; (Inner->getFlags() & Scope::DeclScope) == 0 ||
+ Inner->isTemplateParamScope(); Inner = Inner->getParent()) {
+  if (IssueShadowingWarning = Inner->isDeclScope(TPD))
+break;
+}
+
 // Maybe we will complain about the shadowed template parameter.
 if (!D.isInvalidType())
   DiagnoseTemplateParameterShadow(D.getIdentifierLoc(),
-  Previous.getFoundDecl());
+  TPD,
+  IssueShadowingWarning);
 
 // Just pretend that we didn't