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 <sdkryst...@gmail.com> 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<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 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<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 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 *&Decl); NamedDecl *ActOnTypeParameter(Scope *S, bool Typename, diff --git a/clang/lib/Sema/Scope.cpp b/clang/lib/Sema/Scope.cpp index 4570d8c615fe5e..a8c318dcc5327b 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 c89c3c487272d2..200fec05c93ff8 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -6359,12 +6359,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(); @@ -6488,12 +6482,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(); @@ -6517,6 +6521,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 5adc262cd6bc97..b3db063758c7ca 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -12192,10 +12192,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; @@ -13503,11 +13501,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 3f33ecb89502ea..882890f121fc48 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); @@ -8501,9 +8504,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 @@ -10572,11 +10573,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@-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@-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@-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@-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@-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@-1{{template parameter is declared here}} + }; } namespace PR28023 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits