[Clang][diagnostics] Fix structured binding shadows template param loc #129116
Conversation
AmrDeveloper
added
clang
llvmbot
added
the
clang:frontend
|
@llvm/pr-subscribers-clang Author: Amr Hesham (AmrDeveloper) ChangesFix structured binding shadows template parameter location Fixes: #129060 Full diff: https://github.com/llvm/llvm-project/pull/129116.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 664d48ccbc382..a3a028b9485d6 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -883,8 +883,7 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator &D,
// It's not permitted to shadow a template parameter name.
if (Previous.isSingleResult() &&
Previous.getFoundDecl()->isTemplateParameter()) {
- DiagnoseTemplateParameterShadow(D.getIdentifierLoc(),
- Previous.getFoundDecl());
+ DiagnoseTemplateParameterShadow(B.NameLoc, Previous.getFoundDecl());
Previous.clear();
}
diff --git a/clang/test/CXX/temp/temp.res/temp.local/p6.cpp b/clang/test/CXX/temp/temp.res/temp.local/p6.cpp
index 00bb35813c39a..e464bb5e7eaef 100644
--- a/clang/test/CXX/temp/temp.res/temp.local/p6.cpp
+++ b/clang/test/CXX/temp/temp.res/temp.local/p6.cpp
@@ -162,3 +162,8 @@ struct A {
};
A<0>::B a;
}
+
+template <typename T> void shadow9() { // expected-note{{template parameter is declared here}}
+ using arr = int[1]; // expected-warning@+1 {{decomposition declarations are a C++17 extension}}
+ auto [T] = arr{}; // expected-error {{declaration of 'T' shadows template parameter}}
+}
|
|
@shafik I added the sample to our test cases, not sure if we can add source loc in expected line, I will check that |
h/t @erichkeane something like this should help get us the confirmation in the test we want: https://godbolt.org/z/1GfK1K8qf |
|
|
||
| template <typename T> void shadow9() { // expected-note{{template parameter is declared here}} | ||
| using arr = int[1]; // expected-warning@+1 {{decomposition declarations are a C++17 extension}} | ||
| auto [T] = arr{}; // expected-error {{declaration of 'T' shadows template parameter}} |
There was a problem hiding this comment.
I think this test case would benefit from the new lines added in the initial issue report - the reported behavior is due to us choosing to report an error based on the wrong part of the declaration, and I don't believe that this test case is sufficient to highlight the problem (or more to the point catch a regression in the behavior)
clang/docs/ReleaseNotes.rst
Outdated
| @@ -214,6 +214,8 @@ Improvements to Clang's diagnostics | |||
| :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The | |||
| feature will be default-enabled with ``-Wthread-safety`` in a future release. | |||
|
|
|||
| - Improve the diagnostics for shadows template parameter. | |||
There was a problem hiding this comment.
This should probably mention the bug# as well as slightly better clarify that it is giving a more correct location.
| @@ -162,3 +162,10 @@ struct A { | |||
| }; | |||
| A<0>::B a; | |||
| } | |||
|
|
|||
| template <typename T> void shadow() { // expected-note{{template parameter is declared here}} | |||
There was a problem hiding this comment.
Please instantiate this template as well.
There was a problem hiding this comment.
You mean something like 'template void shadow2() { ... }` or creating a sample with function call
There was a problem hiding this comment.
Just a call to this function, so: shadow<int>() call. If you change it to return int, you can do: auto Use = shadow<int>() below this to instantiate it.
| @@ -162,3 +162,10 @@ struct A { | |||
| }; | |||
| A<0>::B a; | |||
| } | |||
|
|
|||
| template <typename T> void shadow() { // expected-note{{template parameter is declared here}} | |||
| using arr = int[1]; // expected-warning@+1 {{decomposition declarations are a C++17 extension}} | |||
There was a problem hiding this comment.
| using arr = int[1]; // expected-warning@+1 {{decomposition declarations are a C++17 extension}} | |
| using arr = int[1]; | |
| // expected-warning@+1 {{decomposition declarations are a C++17 extension}} |
llvm#129116) Fix structured binding shadows template parameter location Fixes: llvm#129060
Fix structured binding shadows template parameter location
Fixes: #129060