[clang] c1d8d0a - Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (#79683)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 4 06:18:26 PST 2024
Author: Krystian Stasiowski
Date: 2024-03-04T09:18:21-05:00
New Revision: c1d8d0aa156f651ee48414fa002e9608d6998763
URL: https://github.com/llvm/llvm-project/commit/c1d8d0aa156f651ee48414fa002e9608d6998763
DIFF: https://github.com/llvm/llvm-project/commit/c1d8d0aa156f651ee48414fa002e9608d6998763.diff
LOG: Reapply "[Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (#78274)" (#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.
I couldn't find an established convention for naming diagnostics related
to compatibility with previous versions of clang, so I just used the
prefix `ext_compat_`.
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Scope.h
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Scope.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/CXX/temp/temp.res/temp.local/p6.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4797546f2891ce..c381da7e354c70 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<class T> 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<MicrosoftTemplateShadow>;
+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 *&Decl);
NamedDecl *ActOnTypeParameter(Scope *S, bool Typename,
diff --git a/clang/lib/Sema/Scope.cpp b/clang/lib/Sema/Scope.cpp
index cea6a62e347477..11a41753a1bda7 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;
TemplateParamParent = nullptr;
+ DeclParent = nullptr;
MSLastManglingNumber = 1;
MSCurManglingNumber = 1;
}
@@ -76,6 +78,7 @@ void Scope::setFlags(Scope *parent, unsigned flags) {
PrototypeDepth++;
if (flags & DeclScope) {
+ DeclParent = this;
if (flags & FunctionPrototypeScope)
; // Prototype scopes are uninteresting.
else if ((flags & ClassScope) && getParent()->isClassScope())
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 6289cf75e17413..3ae78748a4e499 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6353,12 +6353,6 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
} 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();
@@ -6486,12 +6480,22 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
RemoveUsingDecls(Previous);
}
- if (Previous.isSingleResult() &&
- Previous.getFoundDecl()->isTemplateParameter()) {
- // Maybe we will complain about the shadowed template parameter.
- if (!D.isInvalidType())
- DiagnoseTemplateParameterShadow(D.getIdentifierLoc(),
- Previous.getFoundDecl());
+ if (auto *TPD = Previous.getAsSingle<NamedDecl>();
+ 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.
+ if (!D.isInvalidType()) {
+ bool AllowForCompatibility = false;
+ if (Scope *DeclParent = S->getDeclParent();
+ Scope *TemplateParamParent = S->getTemplateParamParent()) {
+ AllowForCompatibility = DeclParent->Contains(*TemplateParamParent) &&
+ TemplateParamParent->isDeclScope(TPD);
+ }
+ DiagnoseTemplateParameterShadow(D.getIdentifierLoc(), TPD,
+ AllowForCompatibility);
+ }
// Just pretend that we didn't see the previous declaration.
Previous.clear();
@@ -6515,6 +6519,9 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
if (getLangOpts().CPlusPlus)
CheckExtraCXXDefaultArguments(D);
+ /// Get the innermost enclosing declaration scope.
+ S = S->getDeclParent();
+
NamedDecl *New;
bool AddToScope = true;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 5bbe381f5c4cfd..199f2523cfb5d2 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -12203,10 +12203,8 @@ Decl *Sema::ActOnUsingDirective(Scope *S, SourceLocation UsingLoc,
assert(NamespcName && "Invalid NamespcName.");
assert(IdentLoc.isValid() && "Invalid NamespceName location.");
- // This can only happen along a recovery path.
- while (S->isTemplateParamScope())
- S = S->getParent();
- assert(S->getFlags() & Scope::DeclScope && "Invalid Scope.");
+ // Get the innermost enclosing declaration scope.
+ S = S->getDeclParent();
UsingDirectiveDecl *UDir = nullptr;
NestedNameSpecifier *Qualifier = nullptr;
@@ -13516,11 +13514,8 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS,
SourceLocation UsingLoc, UnqualifiedId &Name,
const ParsedAttributesView &AttrList,
TypeResult Type, Decl *DeclFromDeclSpec) {
- // Skip up to the relevant declaration scope.
- while (S->isTemplateParamScope())
- S = S->getParent();
- assert((S->getFlags() & Scope::DeclScope) &&
- "got alias-declaration outside of declaration scope");
+ // Get the innermost enclosing declaration scope.
+ S = S->getDeclParent();
if (Type.isInvalid())
return nullptr;
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index a7910bda874c8d..873ea10ebe0660 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -881,20 +881,23 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
return true;
}
-/// 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.
-void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) {
+void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl,
+ bool SupportedForCompatibility) {
assert(PrevDecl->isTemplateParameter() && "Not a template parameter");
- // C++ [temp.local]p4:
- // A template-parameter shall not be redeclared within its
- // scope (including nested scopes).
+ // C++23 [temp.local]p6:
+ // The name of a template-parameter shall not be bound to any following.
+ // declaration whose locus is contained by the scope to which the
+ // template-parameter belongs.
//
- // Make this a warning when MSVC compatibility is requested.
- unsigned DiagId = getLangOpts().MSVCCompat ? diag::ext_template_param_shadow
- : diag::err_template_param_shadow;
+ // When MSVC compatibility is enabled, the diagnostic is always a warning
+ // by default. Otherwise, it an error unless SupportedForCompatibility is
+ // true, in which case it is a default-to-error warning.
+ unsigned DiagId =
+ getLangOpts().MSVCCompat
+ ? diag::ext_template_param_shadow
+ : (SupportedForCompatibility ? diag::ext_compat_template_param_shadow
+ : diag::err_template_param_shadow);
const auto *ND = cast<NamedDecl>(PrevDecl);
Diag(Loc, DiagId) << ND->getDeclName();
NoteTemplateParameterLocation(*ND);
@@ -8502,9 +8505,7 @@ Sema::CheckTemplateDeclScope(Scope *S, TemplateParameterList *TemplateParams) {
return false;
// Find the nearest enclosing declaration scope.
- while ((S->getFlags() & Scope::DeclScope) == 0 ||
- (S->getFlags() & Scope::TemplateParamScope) != 0)
- S = S->getParent();
+ S = S->getDeclParent();
// C++ [temp.pre]p6: [P2096]
// A template, explicit specialization, or partial specialization shall not
@@ -10619,11 +10620,8 @@ DeclResult Sema::ActOnExplicitInstantiation(Scope *S,
return true;
}
- // 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();
+ // Get the innermost enclosing declaration scope.
+ S = S->getDeclParent();
// Determine the type of the declaration.
TypeSourceInfo *T = GetTypeForDeclarator(D);
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 e2aa0ff344291d..00bb35813c39af 100644
--- a/clang/test/CXX/temp/temp.res/temp.local/p6.cpp
+++ b/clang/test/CXX/temp/temp.res/temp.local/p6.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s -fcxx-exceptions -std=c++1y
+// RUN: %clang_cc1 -verify %s -fcxx-exceptions -std=c++1y -Wno-error=strict-primary-template-shadow
namespace N {}
@@ -127,16 +127,30 @@ template<int T> struct Z { // expected-note 16{{declared here}}
template<typename T> // expected-note {{declared here}}
void f(int T) {} // expected-error {{declaration of 'T' shadows template parameter}}
-// FIXME: These are ill-formed: a template-parameter shall not have the same name as the template name.
namespace A {
template<typename T> struct T {}; // expected-error{{declaration of 'T' shadows template parameter}}
// expected-note at -1{{template parameter is declared here}}
+ template<typename T> struct U {
+ template<typename V> struct V {}; // expected-error{{declaration of 'V' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+ };
}
namespace B {
- template<typename T> void T() {}
+ template<typename T> void T() {} // expected-warning{{declaration of 'T' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+
+ template<typename T> struct U {
+ template<typename V> void V(); // expected-warning{{declaration of 'V' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+ };
}
namespace C {
- template<typename T> int T;
+ template<typename T> int T; // expected-warning{{declaration of 'T' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+ template<typename T> struct U {
+ template<typename V> static int V; // expected-warning{{declaration of 'V' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+ };
}
namespace PR28023 {
More information about the cfe-commits
mailing list