llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Krystian Stasiowski (sdkrystian) <details> <summary>Changes</summary> According to [[temp.expl.spec] p2](http://eel.is/c++draft/temp.expl.spec#<!-- -->2): > The declaration in an _explicit-specialization_ shall not be an _export-declaration_. An explicit specialization shall not use a _storage-class-specifier_ other than `thread_local`. Clang partially implements this, but a number of issues exist: 1. We don't diagnose class scope explicit specializations of variable templates with _storage-class-specifiers_, e.g. ```cpp struct A { template<typename T> static constexpr int x = 0; template<> static constexpr int x<void> = 1; // ill-formed, but clang accepts }; ```` 2. We incorrectly reject class scope explicit specializations of variable templates when `static` is not used, e.g. ```cpp struct A { template<typename T> static constexpr int x = 0; template<> constexpr int x<void> = 1; // error: non-static data member cannot be constexpr; did you intend to make it static? }; ```` 3. We don't diagnose dependent class scope explicit specializations of function templates with storage class specifiers, e.g. ```cpp template<typename T> struct A { template<typename U> static void f(); template<> static void f<int>(); // ill-formed, but clang accepts }; ```` This patch addresses these issues as follows: - [#<!-- -->1]() is fixed by issuing a diagnostic when an explicit specialization of a variable template has storage class specifier - [#<!-- -->2]() is fixed by considering any non-function declaration with any template parameter lists at class scope to be a static data member. This also allows for better error recovery (it's more likely the user intended to declare a variable template than a "field template"). - [#<!-- -->3]() is fixed by checking whether a function template explicit specialization has a storage class specifier even when the primary template is not yet known. One thing to note is that it would be far simpler to diagnose this when parsing the _decl-specifier-seq_, but such an implementation would necessitate a refactor of `ParsedTemplateInfo` which I believe to be outside the scope of this patch. --- Patch is 49.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/93873.diff 18 Files Affected: - (modified) clang/lib/Parse/ParseDeclCXX.cpp (+3-2) - (modified) clang/lib/Sema/DeclSpec.cpp (+1) - (modified) clang/lib/Sema/SemaDecl.cpp (+134-114) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+5-3) - (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.stc/p1.cpp (+3-2) - (modified) clang/test/CXX/drs/cwg7xx.cpp (+12-12) - (modified) clang/test/CXX/temp/temp.decls/temp.mem/p2.cpp (+1-1) - (modified) clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp (+1-1) - (modified) clang/test/CXX/temp/temp.spec/temp.expl.spec/p17.cpp (+8-7) - (modified) clang/test/CXX/temp/temp.spec/temp.expl.spec/p2-0x.cpp (+31-28) - (added) clang/test/CXX/temp/temp.spec/temp.expl.spec/p2-20.cpp (+274) - (modified) clang/test/Modules/Inputs/redecl-templates/a.h (+1-1) - (modified) clang/test/Modules/redecl-templates.cpp (+4-2) - (modified) clang/test/PCH/cxx-templates.h (+6-6) - (modified) clang/test/PCH/cxx1y-variable-templates.cpp (+16-8) - (modified) clang/test/SemaCXX/cxx1y-variable-templates_in_class.cpp (+18-12) - (modified) clang/test/SemaTemplate/explicit-specialization-member.cpp (+4-4) - (modified) clang/test/SemaTemplate/nested-template.cpp (+7-7) ``````````diff diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 9a4a777f575b2..d02548f6441f9 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -3162,7 +3162,8 @@ Parser::DeclGroupPtrTy Parser::ParseCXXClassMemberDeclaration( DeclSpec::SCS_static && DeclaratorInfo.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_typedef && - !DS.isFriendSpecified()) { + !DS.isFriendSpecified() && + TemplateInfo.Kind == ParsedTemplateInfo::NonTemplate) { // It's a default member initializer. if (BitfieldSize.get()) Diag(Tok, getLangOpts().CPlusPlus20 @@ -3261,7 +3262,7 @@ Parser::DeclGroupPtrTy Parser::ParseCXXClassMemberDeclaration( } else if (ThisDecl) Actions.AddInitializerToDecl(ThisDecl, Init.get(), EqualLoc.isInvalid()); - } else if (ThisDecl && DS.getStorageClassSpec() == DeclSpec::SCS_static) + } else if (ThisDecl && DeclaratorInfo.isStaticMember()) // No initializer. Actions.ActOnUninitializedDecl(ThisDecl); diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp index 60e8189025700..96c90a60b9682 100644 --- a/clang/lib/Sema/DeclSpec.cpp +++ b/clang/lib/Sema/DeclSpec.cpp @@ -416,6 +416,7 @@ bool Declarator::isDeclarationOfFunction() const { bool Declarator::isStaticMember() { assert(getContext() == DeclaratorContext::Member); return getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static || + (!isDeclarationOfFunction() && !getTemplateParameterLists().empty()) || (getName().getKind() == UnqualifiedIdKind::IK_OperatorFunctionId && CXXMethodDecl::isStaticOverloadedOperator( getName().OperatorFunctionId.Operator)); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 2a87b26f17a2b..cc36387c0e332 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -7602,80 +7602,8 @@ NamedDecl *Sema::ActOnVariableDeclarator( NTCUC_AutoVar, NTCUK_Destruct); } else { bool Invalid = false; - - if (DC->isRecord() && !CurContext->isRecord()) { - // This is an out-of-line definition of a static data member. - switch (SC) { - case SC_None: - break; - case SC_Static: - Diag(D.getDeclSpec().getStorageClassSpecLoc(), - diag::err_static_out_of_line) - << FixItHint::CreateRemoval(D.getDeclSpec().getStorageClassSpecLoc()); - break; - case SC_Auto: - case SC_Register: - case SC_Extern: - // [dcl.stc] p2: The auto or register specifiers shall be applied only - // to names of variables declared in a block or to function parameters. - // [dcl.stc] p6: The extern specifier cannot be used in the declaration - // of class members - - Diag(D.getDeclSpec().getStorageClassSpecLoc(), - diag::err_storage_class_for_static_member) - << FixItHint::CreateRemoval(D.getDeclSpec().getStorageClassSpecLoc()); - break; - case SC_PrivateExtern: - llvm_unreachable("C storage class in c++!"); - } - } - - if (SC == SC_Static && CurContext->isRecord()) { - if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC)) { - // Walk up the enclosing DeclContexts to check for any that are - // incompatible with static data members. - const DeclContext *FunctionOrMethod = nullptr; - const CXXRecordDecl *AnonStruct = nullptr; - for (DeclContext *Ctxt = DC; Ctxt; Ctxt = Ctxt->getParent()) { - if (Ctxt->isFunctionOrMethod()) { - FunctionOrMethod = Ctxt; - break; - } - const CXXRecordDecl *ParentDecl = dyn_cast<CXXRecordDecl>(Ctxt); - if (ParentDecl && !ParentDecl->getDeclName()) { - AnonStruct = ParentDecl; - break; - } - } - if (FunctionOrMethod) { - // C++ [class.static.data]p5: A local class shall not have static data - // members. - Diag(D.getIdentifierLoc(), - diag::err_static_data_member_not_allowed_in_local_class) - << Name << RD->getDeclName() - << llvm::to_underlying(RD->getTagKind()); - } else if (AnonStruct) { - // C++ [class.static.data]p4: Unnamed classes and classes contained - // directly or indirectly within unnamed classes shall not contain - // static data members. - Diag(D.getIdentifierLoc(), - diag::err_static_data_member_not_allowed_in_anon_struct) - << Name << llvm::to_underlying(AnonStruct->getTagKind()); - Invalid = true; - } else if (RD->isUnion()) { - // C++98 [class.union]p1: If a union contains a static data member, - // the program is ill-formed. C++11 drops this restriction. - Diag(D.getIdentifierLoc(), - getLangOpts().CPlusPlus11 - ? diag::warn_cxx98_compat_static_data_member_in_union - : diag::ext_static_data_member_in_union) << Name; - } - } - } - // Match up the template parameter lists with the scope specifier, then // determine whether we have a template or a template specialization. - bool InvalidScope = false; TemplateParams = MatchTemplateParametersToScopeSpecifier( D.getDeclSpec().getBeginLoc(), D.getIdentifierLoc(), D.getCXXScopeSpec(), @@ -7683,8 +7611,7 @@ NamedDecl *Sema::ActOnVariableDeclarator( ? D.getName().TemplateId : nullptr, TemplateParamLists, - /*never a friend*/ false, IsMemberSpecialization, InvalidScope); - Invalid |= InvalidScope; + /*never a friend*/ false, IsMemberSpecialization, Invalid); if (TemplateParams) { if (!TemplateParams->size() && @@ -7727,6 +7654,102 @@ NamedDecl *Sema::ActOnVariableDeclarator( "should have a 'template<>' for this decl"); } + bool IsExplicitSpecialization = + IsVariableTemplateSpecialization && !IsPartialSpecialization; + + // C++ [temp.expl.spec]p2: + // The declaration in an explicit-specialization shall not be an + // export-declaration. An explicit specialization shall not use a + // storage-class-specifier other than thread_local. + // + // We use the storage-class-specifier from DeclSpec because we may have + // added implicit 'extern' for declarations with __declspec(dllimport)! + if (SCSpec != DeclSpec::SCS_unspecified && + (IsExplicitSpecialization || IsMemberSpecialization)) { + Diag(D.getDeclSpec().getStorageClassSpecLoc(), + diag::ext_explicit_specialization_storage_class) + << FixItHint::CreateRemoval(D.getDeclSpec().getStorageClassSpecLoc()); + } + + if (CurContext->isRecord()) { + if (SC == SC_Static) { + if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC)) { + // Walk up the enclosing DeclContexts to check for any that are + // incompatible with static data members. + const DeclContext *FunctionOrMethod = nullptr; + const CXXRecordDecl *AnonStruct = nullptr; + for (DeclContext *Ctxt = DC; Ctxt; Ctxt = Ctxt->getParent()) { + if (Ctxt->isFunctionOrMethod()) { + FunctionOrMethod = Ctxt; + break; + } + const CXXRecordDecl *ParentDecl = dyn_cast<CXXRecordDecl>(Ctxt); + if (ParentDecl && !ParentDecl->getDeclName()) { + AnonStruct = ParentDecl; + break; + } + } + if (FunctionOrMethod) { + // C++ [class.static.data]p5: A local class shall not have static + // data members. + Diag(D.getIdentifierLoc(), + diag::err_static_data_member_not_allowed_in_local_class) + << Name << RD->getDeclName() + << llvm::to_underlying(RD->getTagKind()); + } else if (AnonStruct) { + // C++ [class.static.data]p4: Unnamed classes and classes contained + // directly or indirectly within unnamed classes shall not contain + // static data members. + Diag(D.getIdentifierLoc(), + diag::err_static_data_member_not_allowed_in_anon_struct) + << Name << llvm::to_underlying(AnonStruct->getTagKind()); + Invalid = true; + } else if (RD->isUnion()) { + // C++98 [class.union]p1: If a union contains a static data member, + // the program is ill-formed. C++11 drops this restriction. + Diag(D.getIdentifierLoc(), + getLangOpts().CPlusPlus11 + ? diag::warn_cxx98_compat_static_data_member_in_union + : diag::ext_static_data_member_in_union) + << Name; + } + } + } else if (IsVariableTemplate || IsPartialSpecialization) { + // There is no such thing as a member field template. + Diag(D.getIdentifierLoc(), diag::err_template_member) + << II << TemplateParams->getSourceRange(); + // Recover by pretending this is a static data member template. + SC = SC_Static; + } + } else if (DC->isRecord()) { + // This is an out-of-line definition of a static data member. + switch (SC) { + case SC_None: + break; + case SC_Static: + Diag(D.getDeclSpec().getStorageClassSpecLoc(), + diag::err_static_out_of_line) + << FixItHint::CreateRemoval( + D.getDeclSpec().getStorageClassSpecLoc()); + break; + case SC_Auto: + case SC_Register: + case SC_Extern: + // [dcl.stc] p2: The auto or register specifiers shall be applied only + // to names of variables declared in a block or to function parameters. + // [dcl.stc] p6: The extern specifier cannot be used in the declaration + // of class members + + Diag(D.getDeclSpec().getStorageClassSpecLoc(), + diag::err_storage_class_for_static_member) + << FixItHint::CreateRemoval( + D.getDeclSpec().getStorageClassSpecLoc()); + break; + case SC_PrivateExtern: + llvm_unreachable("C storage class in c++!"); + } + } + if (IsVariableTemplateSpecialization) { SourceLocation TemplateKWLoc = TemplateParamLists.size() > 0 @@ -7772,8 +7795,6 @@ NamedDecl *Sema::ActOnVariableDeclarator( // the variable (matching the scope specifier), store them. // An explicit variable template specialization does not own any template // parameter lists. - bool IsExplicitSpecialization = - IsVariableTemplateSpecialization && !IsPartialSpecialization; unsigned VDTemplateParamLists = (TemplateParams && !IsExplicitSpecialization) ? 1 : 0; if (TemplateParamLists.size() > VDTemplateParamLists) @@ -10203,25 +10224,45 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, NewFD->setImplicitlyInline(ImplicitInlineCXX20); } - if (SC == SC_Static && isa<CXXMethodDecl>(NewFD) && - !CurContext->isRecord()) { - // C++ [class.static]p1: - // A data or function member of a class may be declared static - // in a class definition, in which case it is a static member of - // the class. + if (!isFriend && SC != SC_None) { + // C++ [temp.expl.spec]p2: + // The declaration in an explicit-specialization shall not be an + // export-declaration. An explicit specialization shall not use a + // storage-class-specifier other than thread_local. + // + // We diagnose friend declarations with storage-class-specifiers + // elsewhere. + if (isFunctionTemplateSpecialization || isMemberSpecialization) { + Diag(D.getDeclSpec().getStorageClassSpecLoc(), + diag::ext_explicit_specialization_storage_class) + << FixItHint::CreateRemoval( + D.getDeclSpec().getStorageClassSpecLoc()); + } - // Complain about the 'static' specifier if it's on an out-of-line - // member function definition. + if (SC == SC_Static && !CurContext->isRecord() && DC->isRecord()) { + assert(isa<CXXMethodDecl>(NewFD) && + "Out-of-line member function should be a CXXMethodDecl"); + // C++ [class.static]p1: + // A data or function member of a class may be declared static + // in a class definition, in which case it is a static member of + // the class. - // MSVC permits the use of a 'static' storage specifier on an out-of-line - // member function template declaration and class member template - // declaration (MSVC versions before 2015), warn about this. - Diag(D.getDeclSpec().getStorageClassSpecLoc(), - ((!getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2015) && - cast<CXXRecordDecl>(DC)->getDescribedClassTemplate()) || - (getLangOpts().MSVCCompat && NewFD->getDescribedFunctionTemplate())) - ? diag::ext_static_out_of_line : diag::err_static_out_of_line) - << FixItHint::CreateRemoval(D.getDeclSpec().getStorageClassSpecLoc()); + // Complain about the 'static' specifier if it's on an out-of-line + // member function definition. + + // MSVC permits the use of a 'static' storage specifier on an + // out-of-line member function template declaration and class member + // template declaration (MSVC versions before 2015), warn about this. + Diag(D.getDeclSpec().getStorageClassSpecLoc(), + ((!getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2015) && + cast<CXXRecordDecl>(DC)->getDescribedClassTemplate()) || + (getLangOpts().MSVCCompat && + NewFD->getDescribedFunctionTemplate())) + ? diag::ext_static_out_of_line + : diag::err_static_out_of_line) + << FixItHint::CreateRemoval( + D.getDeclSpec().getStorageClassSpecLoc()); + } } // C++11 [except.spec]p15: @@ -10589,27 +10630,6 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, Previous)) NewFD->setInvalidDecl(); } - - // C++ [dcl.stc]p1: - // A storage-class-specifier shall not be specified in an explicit - // specialization (14.7.3) - // FIXME: We should be checking this for dependent specializations. - FunctionTemplateSpecializationInfo *Info = - NewFD->getTemplateSpecializationInfo(); - if (Info && SC != SC_None) { - if (SC != Info->getTemplate()->getTemplatedDecl()->getStorageClass()) - Diag(NewFD->getLocation(), - diag::err_explicit_specialization_inconsistent_storage_class) - << SC - << FixItHint::CreateRemoval( - D.getDeclSpec().getStorageClassSpecLoc()); - - else - Diag(NewFD->getLocation(), - diag::ext_explicit_specialization_storage_class) - << FixItHint::CreateRemoval( - D.getDeclSpec().getStorageClassSpecLoc()); - } } else if (isMemberSpecialization && isa<CXXMethodDecl>(NewFD)) { if (CheckMemberSpecialization(NewFD, Previous)) NewFD->setInvalidDecl(); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 8ab429e2a136e..55284d638efb1 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -3489,9 +3489,9 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D, break; } - bool isInstField = ((DS.getStorageClassSpec() == DeclSpec::SCS_unspecified || - DS.getStorageClassSpec() == DeclSpec::SCS_mutable) && - !isFunc); + bool isInstField = (DS.getStorageClassSpec() == DeclSpec::SCS_unspecified || + DS.getStorageClassSpec() == DeclSpec::SCS_mutable) && + !isFunc && TemplateParameterLists.empty(); if (DS.hasConstexprSpecifier() && isInstField) { SemaDiagnosticBuilder B = @@ -3541,6 +3541,7 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D, IdentifierInfo *II = Name.getAsIdentifierInfo(); +#if 0 // Member field could not be with "template" keyword. // So TemplateParameterLists should be empty in this case. if (TemplateParameterLists.size()) { @@ -3561,6 +3562,7 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D, } return nullptr; } +#endif if (D.getName().getKind() == UnqualifiedIdKind::IK_TemplateId) { Diag(D.getIdentifierLoc(), diag::err_member_with_template_arguments) diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.stc/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.stc/p1.cpp index cbb439ef5fecd..f6b5d2487e73d 100644 --- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.stc/p1.cpp +++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.stc/p1.cpp @@ -7,7 +7,7 @@ template<typename T> void f(T) {} template<typename T> static void g(T) {} -template<> static void f<int>(int); // expected-error{{explicit specialization has extraneous, inconsistent storage class 'static'}} +template<> static void f<int>(int); // expected-warning{{explicit specialization cannot have a storage class}} template static void f<float>(float); // expected-error{{explicit instantiation cannot have a storage class}} template<> void f<double>(double); @@ -29,4 +29,5 @@ int X<T>::value = 17; template static int X<int>::value; // expected-error{{explicit instantiation cannot have a storage class}} -template<> static int X<float>::value; // expected-error{{'static' can only be specified inside the class definition}} +template<> static int X<float>::value; // expected-warning{{explicit specialization cannot have a storage class}} + // expected-error@-1{{'static' can only be specified inside the class definition}} diff --git a/clang/test/CXX/drs/cwg7xx.cpp b/clang/test/CXX/drs/cwg7xx.cpp index 0300dae08d6d3..6d93e2948dadb 100644 --- a/clang/test/CXX/drs/cwg7xx.cpp +++ b/clang/test/CXX/drs/cwg7xx.cpp @@ -80,7 +80,7 @@ namespace cwg727 { // cwg727: partial template<> struct C<int>; template<> void f<int>(); - template<> static int N<int>; + template<> int N<int>; template<typename T> struct C<T*>; template<typename T> static int N<T*>; @@ -91,7 +91,7 @@ namespace cwg727 { // cwg727: partial // expected-note@#cwg727-C {{explicitly specialized declaration is here}} template<> void f<float>(); // expected-error@-1 {{no function template matches function template specialization 'f'}} - template<> static int N<float>; + template<> int N<float>; // expected-error@-1 {{variable template specialization of 'N' not in class 'A' or an enclosing namespace}} // expected-note@#cwg727-N {{explicitly specialized declaration is here}} @@ -109,7 +109,7 @@ namespace cwg727 { // cwg727: partial template<> void A::f<double>(); // expected-error@-1 {{o function template matches function template specialization 'f'}} // expected-error@-2 {{non-friend class member 'f' cannot have a qualified name}} - template<> static int A::N<double>; + template<> int A::N<double>; // expected-error@-1 {{non-friend class member 'N' cannot have a qualified name}} // expected-error@-2 {{variable template specialization of 'N' not in class 'A' or an enclosing namespace}} // expected-note@#cwg727-N {{explicitly specialized declaration is here}} @@ -166,7 +166,7 @@ namespace cwg727 { // cwg727: partial template<> struct C<int> {}; template<> void f<int>() {} - template<> static const int N<int>; + template<> const int N<int>; template<typename T> struct C<T*> {}; template<typename T> static const int N<T*>; @@ -208,18 +208,18 @@ namespace cwg727 { // cwg727: partial #if __cplusplus >= 201402L template<int> struct B { template<int> static const int u = 1; - template<> static const i... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/93873 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
