[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
@@ -13464,6 +13464,14 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS, } TemplateParameterList *TemplateParams = TemplateParamLists[0]; +// Check shadowing of a template parameter name +for (NamedDecl *TP : TemplateParams->asArray()) { + if (NameInfo.getName() == TP->getDeclName()) { +DiagnoseTemplateParameterShadow(Name.StartLocation, TP); +return nullptr; + } +} + BaLiKfromUA wrote: I replaced my initial loop with the removal of `S = S->getDeclParent();`, but, as I understand, we are waiting for other folks to comment. https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
@@ -13464,6 +13464,14 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS, } TemplateParameterList *TemplateParams = TemplateParamLists[0]; +// Check shadowing of a template parameter name +for (NamedDecl *TP : TemplateParams->asArray()) { + if (NameInfo.getName() == TP->getDeclName()) { +DiagnoseTemplateParameterShadow(Name.StartLocation, TP); +return nullptr; + } +} + zyn0217 wrote: So GCC seems to start rejecting this somewhere between 10.2 and 10.3. Is it a DR? https://compiler-explorer.com/z/GEoWGYbbY https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
@@ -13464,6 +13464,14 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS, } TemplateParameterList *TemplateParams = TemplateParamLists[0]; +// Check shadowing of a template parameter name +for (NamedDecl *TP : TemplateParams->asArray()) { + if (NameInfo.getName() == TP->getDeclName()) { +DiagnoseTemplateParameterShadow(Name.StartLocation, TP); +return nullptr; + } +} + zyn0217 wrote: @cor3ntin @erichkeane I don't understand why we were skipping past template parameter scopes even prior to @sdkrystian's patch. It doesn't seem very right for this case. https://github.com/llvm/llvm-project/commit/c1d8d0aa156f651ee48414fa002e9608d6998763#diff-9ced4fa47ee2b9c03b6996ce89a1d131c0f5b71013993bc582209f50d5e934daL13519 https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA edited https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA edited https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA edited https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA edited https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
@@ -13464,6 +13464,14 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS, } TemplateParameterList *TemplateParams = TemplateParamLists[0]; +// Check shadowing of a template parameter name +for (NamedDecl *TP : TemplateParams->asArray()) { + if (NameInfo.getName() == TP->getDeclName()) { +DiagnoseTemplateParameterShadow(Name.StartLocation, TP); +return nullptr; + } +} + BaLiKfromUA wrote: Thank you for the review! I also was with this expectation and made some investigation (but be aware that it might not be 100% correct ๐). According to my debugging, the reason for `LookupName(Previous, S);` at line `13425` not seeing template params of alias template is logic on the beginning of a function (line `13401`): ``` S = S->getDeclParent(); ``` If you remove this line, the behavior mentioned in the issue is successfully diagnosed. Initially, I thought that this logic was important for some other template aliasing functionality that I was missing, but I run locally all clang tests without this line, and seems like it doesn't. So I am probably overthinking and we could just remove line `13401` instead of implementing this loop. @zyn0217 Let me know if I am missing something! Otherwise, I will send a fix without this loop later today. https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
@@ -191,4 +192,4 @@ int g = sfinae_me(); // expected-error{{no matching function for call to 's namespace NullExceptionDecl { template auto get = []() { try { } catch(...) {}; return I; }; // expected-error{{initializer contains unexpanded parameter pack 'I'}} -} +} zyn0217 wrote: Please keep a blank line at the EOF https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
@@ -13464,6 +13464,14 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS, } TemplateParameterList *TemplateParams = TemplateParamLists[0]; +// Check shadowing of a template parameter name +for (NamedDecl *TP : TemplateParams->asArray()) { + if (NameInfo.getName() == TP->getDeclName()) { +DiagnoseTemplateParameterShadow(Name.StartLocation, TP); +return nullptr; + } +} + zyn0217 wrote: Did you investigate why the if block around line 13428 doesn't work? I would have expected it to catch the issue earlier, before the creation of TypeAliasDecl. ```cpp // Warn about shadowing the name of a template parameter. if (Previous.isSingleResult() && Previous.getFoundDecl()->isTemplateParameter()) { DiagnoseTemplateParameterShadow(Name.StartLocation,Previous.getFoundDecl()); Previous.clear(); } ``` https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA ready_for_review https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA edited https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA updated https://github.com/llvm/llvm-project/pull/123533 >From e451a8869420d9240f9006eb2adb599a3e6fd9f8 Mon Sep 17 00:00:00 2001 From: Valentyn Yukhymenko Date: Sun, 19 Jan 2025 23:13:46 + Subject: [PATCH] [Clang] Reject declaring an alias template with the same name as its template parameter. Fixes llvm#123423 --- clang/docs/ReleaseNotes.rst | 1 + clang/lib/Sema/SemaDeclCXX.cpp| 8 .../CXX/temp/temp.decls/temp.variadic/fixed-expansion.cpp | 4 ++-- clang/test/SemaCXX/alias-template.cpp | 5 +++-- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index aa1c02d04f7caa..29e40b4ecab412 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -956,6 +956,7 @@ Bug Fixes to C++ Support - Fixed a crash caused by the incorrect construction of template arguments for CTAD alias guides when type constraints are applied. (#GH122134) - Fixed canonicalization of pack indexing types - Clang did not always recognized identical pack indexing. (#GH123033) +- Clang now rejects declaring an alias template with the same name as its template parameter. (#GH123423) Bug Fixes to AST Handling diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index a867ed73bd4033..4e43a8397cec4e 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -13464,6 +13464,14 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS, } TemplateParameterList *TemplateParams = TemplateParamLists[0]; +// Check shadowing of a template parameter name +for (NamedDecl *TP : TemplateParams->asArray()) { + if (NameInfo.getName() == TP->getDeclName()) { +DiagnoseTemplateParameterShadow(Name.StartLocation, TP); +return nullptr; + } +} + // Check that we can declare a template here. if (CheckTemplateDeclScope(S, TemplateParams)) return nullptr; diff --git a/clang/test/CXX/temp/temp.decls/temp.variadic/fixed-expansion.cpp b/clang/test/CXX/temp/temp.decls/temp.variadic/fixed-expansion.cpp index a990c82564aa40..ab4c663d24c7d5 100644 --- a/clang/test/CXX/temp/temp.decls/temp.variadic/fixed-expansion.cpp +++ b/clang/test/CXX/temp/temp.decls/temp.variadic/fixed-expansion.cpp @@ -121,8 +121,8 @@ namespace PartialSpecialization { namespace FixedAliasTemplate { template struct S {}; - template using U = S; // expected-note 2{{template parameter is declared here}} - template U &f(U, Ts...); // expected-error 2{{pack expansion used as argument for non-pack parameter of alias template}} + template using Z = S; // expected-note 2{{template parameter is declared here}} + template Z &f(Z, Ts...); // expected-error 2{{pack expansion used as argument for non-pack parameter of alias template}} S &s1 = f({}, 0, 0.0); // expected-error {{no matching function}} } diff --git a/clang/test/SemaCXX/alias-template.cpp b/clang/test/SemaCXX/alias-template.cpp index 5189405e23db56..97134d2f3a96ad 100644 --- a/clang/test/SemaCXX/alias-template.cpp +++ b/clang/test/SemaCXX/alias-template.cpp @@ -65,7 +65,8 @@ namespace InFunctions { template struct S3 { // expected-note {{template parameter is declared here}} template using T = int; // expected-error {{declaration of 'T' shadows template parameter}} }; - template using Z = Z; + template // expected-note {{template parameter is declared here}} + using Z = Z; // expected-error {{declaration of 'Z' shadows template parameter}} } namespace ClassNameRedecl { @@ -191,4 +192,4 @@ int g = sfinae_me(); // expected-error{{no matching function for call to 's namespace NullExceptionDecl { template auto get = []() { try { } catch(...) {}; return I; }; // expected-error{{initializer contains unexpanded parameter pack 'I'}} -} +} \ No newline at end of file ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Valentyn Yukhymenko (BaLiKfromUA) Changes Fixes llvm#123423 **How did I test it?** Modified existing test and checked an example from the issue manually. --- Full diff: https://github.com/llvm/llvm-project/pull/123533.diff 3 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+1) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+8) - (modified) clang/test/SemaCXX/alias-template.cpp (+3-2) ``diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index aa1c02d04f7caa..29e40b4ecab412 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -956,6 +956,7 @@ Bug Fixes to C++ Support - Fixed a crash caused by the incorrect construction of template arguments for CTAD alias guides when type constraints are applied. (#GH122134) - Fixed canonicalization of pack indexing types - Clang did not always recognized identical pack indexing. (#GH123033) +- Clang now rejects declaring an alias template with the same name as its template parameter. (#GH123423) Bug Fixes to AST Handling diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index a867ed73bd4033..4e43a8397cec4e 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -13464,6 +13464,14 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS, } TemplateParameterList *TemplateParams = TemplateParamLists[0]; +// Check shadowing of a template parameter name +for (NamedDecl *TP : TemplateParams->asArray()) { + if (NameInfo.getName() == TP->getDeclName()) { +DiagnoseTemplateParameterShadow(Name.StartLocation, TP); +return nullptr; + } +} + // Check that we can declare a template here. if (CheckTemplateDeclScope(S, TemplateParams)) return nullptr; diff --git a/clang/test/SemaCXX/alias-template.cpp b/clang/test/SemaCXX/alias-template.cpp index 5189405e23db56..97134d2f3a96ad 100644 --- a/clang/test/SemaCXX/alias-template.cpp +++ b/clang/test/SemaCXX/alias-template.cpp @@ -65,7 +65,8 @@ namespace InFunctions { template struct S3 { // expected-note {{template parameter is declared here}} template using T = int; // expected-error {{declaration of 'T' shadows template parameter}} }; - template using Z = Z; + template // expected-note {{template parameter is declared here}} + using Z = Z; // expected-error {{declaration of 'Z' shadows template parameter}} } namespace ClassNameRedecl { @@ -191,4 +192,4 @@ int g = sfinae_me(); // expected-error{{no matching function for call to 's namespace NullExceptionDecl { template auto get = []() { try { } catch(...) {}; return I; }; // expected-error{{initializer contains unexpanded parameter pack 'I'}} -} +} \ No newline at end of file `` https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA converted_to_draft https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA edited https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
@@ -65,7 +65,8 @@ namespace InFunctions { template struct S3 { // expected-note {{template parameter is declared here}} template using T = int; // expected-error {{declaration of 'T' shadows template parameter}} }; - template using Z = Z; + template // expected-note {{template parameter is declared here}} + using Z = Z; // expected-error {{declaration of 'Z' shadows template parameter}} BaLiKfromUA wrote: I am not sure that `InFunctions` is the correct namespace for this test but it was here before and I adapted this use case to the new behavior. https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment โPingโ. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/123533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reject declaring an alias template with the same name as its template parameter. (PR #123533)
https://github.com/BaLiKfromUA created https://github.com/llvm/llvm-project/pull/123533 Fixes llvm#123423 **How did I test it?** Modified existing test and checked an example from the issue manually. >From 0852c8ca587e772d5f851ac0983f43bdeec2ebd5 Mon Sep 17 00:00:00 2001 From: Valentyn Yukhymenko Date: Sun, 19 Jan 2025 23:13:46 + Subject: [PATCH] [Clang] Reject declaring an alias template with the same name as its template parameter. Fixes llvm#123423 --- clang/docs/ReleaseNotes.rst | 1 + clang/lib/Sema/SemaDeclCXX.cpp| 8 clang/test/SemaCXX/alias-template.cpp | 5 +++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index aa1c02d04f7caa..29e40b4ecab412 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -956,6 +956,7 @@ Bug Fixes to C++ Support - Fixed a crash caused by the incorrect construction of template arguments for CTAD alias guides when type constraints are applied. (#GH122134) - Fixed canonicalization of pack indexing types - Clang did not always recognized identical pack indexing. (#GH123033) +- Clang now rejects declaring an alias template with the same name as its template parameter. (#GH123423) Bug Fixes to AST Handling diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index a867ed73bd4033..4e43a8397cec4e 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -13464,6 +13464,14 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS, } TemplateParameterList *TemplateParams = TemplateParamLists[0]; +// Check shadowing of a template parameter name +for (NamedDecl *TP : TemplateParams->asArray()) { + if (NameInfo.getName() == TP->getDeclName()) { +DiagnoseTemplateParameterShadow(Name.StartLocation, TP); +return nullptr; + } +} + // Check that we can declare a template here. if (CheckTemplateDeclScope(S, TemplateParams)) return nullptr; diff --git a/clang/test/SemaCXX/alias-template.cpp b/clang/test/SemaCXX/alias-template.cpp index 5189405e23db56..97134d2f3a96ad 100644 --- a/clang/test/SemaCXX/alias-template.cpp +++ b/clang/test/SemaCXX/alias-template.cpp @@ -65,7 +65,8 @@ namespace InFunctions { template struct S3 { // expected-note {{template parameter is declared here}} template using T = int; // expected-error {{declaration of 'T' shadows template parameter}} }; - template using Z = Z; + template // expected-note {{template parameter is declared here}} + using Z = Z; // expected-error {{declaration of 'Z' shadows template parameter}} } namespace ClassNameRedecl { @@ -191,4 +192,4 @@ int g = sfinae_me(); // expected-error{{no matching function for call to 's namespace NullExceptionDecl { template auto get = []() { try { } catch(...) {}; return I; }; // expected-error{{initializer contains unexpanded parameter pack 'I'}} -} +} \ No newline at end of file ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits