https://github.com/philnik777 updated https://github.com/llvm/llvm-project/pull/101469
>From 82ab798fc72c6de64ae527d96393f0dc67307e98 Mon Sep 17 00:00:00 2001 From: Nikolas Klauser <nikolasklau...@berlin.de> Date: Thu, 1 Aug 2024 12:30:22 +0200 Subject: [PATCH 1/6] [Clang] Add [[clang::diagnose_specializations]] --- clang/include/clang/Basic/Attr.td | 13 ++++++- clang/include/clang/Basic/AttrDocs.td | 14 ++++++-- clang/include/clang/Basic/DiagnosticGroups.td | 1 + .../clang/Basic/DiagnosticSemaKinds.td | 3 ++ clang/lib/Sema/SemaDeclAttr.cpp | 9 +++++ clang/lib/Sema/SemaTemplate.cpp | 6 ++++ .../SemaCXX/attr-diagnose-specializations.cpp | 34 +++++++++++++++++++ 7 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 clang/test/SemaCXX/attr-diagnose-specializations.cpp diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 8ac2079099c854..e074cc8b285a95 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -103,6 +103,9 @@ def NonParmVar : SubsetSubject<Var, def NonLocalVar : SubsetSubject<Var, [{!S->hasLocalStorage()}], "variables with non-local storage">; +def VarTmpl : SubsetSubject<Var, [{S->getDescribedVarTemplate()}], + "variable template">; + def NonBitField : SubsetSubject<Field, [{!S->isBitField()}], "non-bit-field non-static data members">; @@ -3327,6 +3330,14 @@ def DiagnoseIf : InheritableAttr { let Documentation = [DiagnoseIfDocs]; } +def DiagnoseSpecializations : InheritableAttr { + let Spellings = [Clang<"diagnose_specializations", /*AllowInC*/0>]; + let Subjects = SubjectList<[ClassTmpl, VarTmpl]>; + let Documentation = [DiagnoseSpecializationsDocs]; + let MeaningfulToClassTemplateDefinition = 1; + let TemplateDependent = 1; +} + def ArcWeakrefUnavailable : InheritableAttr { let Spellings = [Clang<"objc_arc_weak_reference_unavailable">]; let Subjects = SubjectList<[ObjCInterface], ErrorDiag>; @@ -4581,7 +4592,7 @@ def HLSLResource : InheritableAttr { let Spellings = []; let Subjects = SubjectList<[Struct]>; let LangOpts = [HLSL]; - let Args = [ + let Args = [ EnumArgument< "ResourceKind", "llvm::hlsl::ResourceKind", /*is_string=*/0, diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 94c284fc731589..4ca67a63714d4b 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -975,6 +975,15 @@ Query for this feature with ``__has_attribute(diagnose_if)``. }]; } +def DiagnoseSpecializationsDocs : Documentation { + let Category = DocCatDecl; + let Content = [{ +``clang::diagnose_specializations`` can be appied to class templates which +should not be specialized by users. This is primarily used to diagnose user +specializations of standard library type traits. + }]; +} + def PassObjectSizeDocs : Documentation { let Category = DocCatVariable; // Technically it's a parameter doc, but eh. let Heading = "pass_object_size, pass_dynamic_object_size"; @@ -7388,10 +7397,10 @@ def HLSLLoopHintDocs : Documentation { let Content = [{ The ``[loop]`` directive allows loop optimization hints to be specified for the subsequent loop. The directive allows unrolling to -be disabled and is not compatible with [unroll(x)]. +be disabled and is not compatible with [unroll(x)]. Specifying the parameter, ``[loop]``, directs the -unroller to not unroll the loop. +unroller to not unroll the loop. .. code-block:: hlsl @@ -8306,4 +8315,3 @@ Declares that a function potentially allocates heap memory, and prevents any pot of ``nonallocating`` by the compiler. }]; } - diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 19c3f1e0433496..d6f6111f708684 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -472,6 +472,7 @@ def ExpansionToDefined : DiagGroup<"expansion-to-defined">; def FlagEnum : DiagGroup<"flag-enum">; def IncrementBool : DiagGroup<"increment-bool", [DeprecatedIncrementBool]>; def InfiniteRecursion : DiagGroup<"infinite-recursion">; +def InvalidSpecialization : DiagGroup<"invalid-specialization">; def PureVirtualCallFromCtorDtor: DiagGroup<"call-to-pure-virtual-from-ctor-dtor">; def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">; def IgnoredGCH : DiagGroup<"ignored-gch">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 581434d33c5c9a..5972d630347ec4 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -5407,6 +5407,9 @@ def note_dependent_function_template_spec_discard_reason : Note< "candidate ignored: %select{not a function template|" "not a member of the enclosing %select{class template|" "namespace; did you mean to explicitly qualify the specialization?}1}0">; +def warn_diag_specialization : Warning< + "specializing a template which should not be specialized">, + DefaultError, InGroup<InvalidSpecialization>; // C++ class template specializations and out-of-line definitions def err_template_spec_needs_header : Error< diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 9011fa547638e5..eb0a705fef0449 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -1215,6 +1215,12 @@ static void handlePreferredName(Sema &S, Decl *D, const ParsedAttr &AL) { << TT->getDecl(); } +static void handleDiagnoseSpecializations(Sema &S, Decl *D, + const ParsedAttr &AL) { + D->getDescribedTemplate()->addAttr( + DiagnoseSpecializationsAttr::Create(S.Context, AL)); +} + bool Sema::isValidPointerAttrType(QualType T, bool RefOkay) { if (RefOkay) { if (T->isReferenceType()) @@ -6700,6 +6706,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_PreferredName: handlePreferredName(S, D, AL); break; + case ParsedAttr::AT_DiagnoseSpecializations: + handleDiagnoseSpecializations(S, D, AL); + break; case ParsedAttr::AT_Section: handleSectionAttr(S, D, AL); break; diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index c22e329bef2b90..ddcc3272cff1ff 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -3976,6 +3976,9 @@ DeclResult Sema::ActOnVarTemplateSpecialization( << IsPartialSpecialization; } + if (VarTemplate->hasAttr<DiagnoseSpecializationsAttr>()) + Diag(TemplateNameLoc, diag::warn_diag_specialization); + // Check for unexpanded parameter packs in any of the template arguments. for (unsigned I = 0, N = TemplateArgs.size(); I != N; ++I) if (DiagnoseUnexpandedParameterPack(TemplateArgs[I], @@ -8085,6 +8088,9 @@ DeclResult Sema::ActOnClassTemplateSpecialization( return true; } + if (ClassTemplate->hasAttr<DiagnoseSpecializationsAttr>()) + Diag(TemplateNameLoc, diag::warn_diag_specialization); + bool isMemberSpecialization = false; bool isPartialSpecialization = false; diff --git a/clang/test/SemaCXX/attr-diagnose-specializations.cpp b/clang/test/SemaCXX/attr-diagnose-specializations.cpp new file mode 100644 index 00000000000000..bf9ea2c56c18a6 --- /dev/null +++ b/clang/test/SemaCXX/attr-diagnose-specializations.cpp @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 %s -verify + +#if !__has_cpp_attribute(clang::diagnose_specializations) +# error +#endif + +struct [[clang::diagnose_specializations]] S {}; // expected-warning {{'diagnose_specializations' attribute only applies to class templates}} + +template <class T, class U> +struct [[clang::diagnose_specializations]] is_same { + static constexpr bool value = __is_same(T, U); +}; + +template <> +struct is_same<int, char> {}; // expected-error {{specializing a template which should not be specialized}} + +template <class> +struct Template {}; + +template <class T> +struct is_same<Template<T>, Template <T>> {}; // expected-error {{specializing a template which should not be specialized}} + +bool test_instantiation1 = is_same<int, int>::value; + +template <class T, class U> +[[clang::diagnose_specializations]] inline constexpr bool is_same_v = __is_same(T, U); + +template <> +inline constexpr bool is_same_v<int, char> = false; // expected-error {{specializing a template which should not be specialized}} + +template <class T> +inline constexpr bool is_same_v<Template <T>, Template <T>> = true; // expected-error {{specializing a template which should not be specialized}} + +bool test_instantiation2 = is_same_v<int, int>; >From 9d1b83b509acf7525281cb400136071177f2c397 Mon Sep 17 00:00:00 2001 From: Nikolas Klauser <nikolasklau...@berlin.de> Date: Thu, 1 Aug 2024 14:59:03 +0200 Subject: [PATCH 2/6] Address comments --- clang/include/clang/Basic/AttrDocs.td | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 4ca67a63714d4b..876902f1a276fb 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -978,9 +978,9 @@ Query for this feature with ``__has_attribute(diagnose_if)``. def DiagnoseSpecializationsDocs : Documentation { let Category = DocCatDecl; let Content = [{ -``clang::diagnose_specializations`` can be appied to class templates which -should not be specialized by users. This is primarily used to diagnose user -specializations of standard library type traits. +``[[clang::diagnose_specializations]]`` can be appied to class or variable +templates which should not be specialized by users. This is primarily used to +diagnose user specializations of standard library type traits. }]; } >From 328874bce6725cd09f7a99e08ec63046727439ad Mon Sep 17 00:00:00 2001 From: Nikolas Klauser <nikolasklau...@berlin.de> Date: Thu, 1 Aug 2024 15:15:24 +0200 Subject: [PATCH 3/6] Fix typo --- clang/include/clang/Basic/AttrDocs.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 876902f1a276fb..393a4b68ad4b2c 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -978,7 +978,7 @@ Query for this feature with ``__has_attribute(diagnose_if)``. def DiagnoseSpecializationsDocs : Documentation { let Category = DocCatDecl; let Content = [{ -``[[clang::diagnose_specializations]]`` can be appied to class or variable +``[[clang::diagnose_specializations]]`` can be applied to class or variable templates which should not be specialized by users. This is primarily used to diagnose user specializations of standard library type traits. }]; >From dce818ff483085a019f141318335ffdf0ab0a4ee Mon Sep 17 00:00:00 2001 From: Nikolas Klauser <nikolasklau...@berlin.de> Date: Fri, 2 Aug 2024 09:37:01 +0200 Subject: [PATCH 4/6] Address comments --- clang/docs/ReleaseNotes.rst | 7 ++++++- clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++-- clang/lib/Sema/SemaTemplate.cpp | 4 ++-- clang/test/SemaCXX/attr-diagnose-specializations.cpp | 8 ++++---- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3c2e0282d1c72d..1190607b98c608 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -124,13 +124,18 @@ Attribute Changes in Clang - The ``hybrid_patchable`` attribute is now supported on ARM64EC targets. It can be used to specify that a function requires an additional x86-64 thunk, which may be patched at runtime. +- The attribute ``[[clang::diagnose_specializations]]`` has been added to warn + users that a specific template shouldn't be specialized. This is useful for + e.g. standard library type traits, where adding a specialization results in + undefined behaviour. + Improvements to Clang's diagnostics ----------------------------------- - Some template related diagnostics have been improved. .. code-block:: c++ - + void foo() { template <typename> int i; } // error: templates can only be declared in namespace or class scope struct S { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 5972d630347ec4..4d91b6671186dd 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -5407,8 +5407,8 @@ def note_dependent_function_template_spec_discard_reason : Note< "candidate ignored: %select{not a function template|" "not a member of the enclosing %select{class template|" "namespace; did you mean to explicitly qualify the specialization?}1}0">; -def warn_diag_specialization : Warning< - "specializing a template which should not be specialized">, +def warn_invalid_specialization : Warning< + "%0 should not be specialized">, DefaultError, InGroup<InvalidSpecialization>; // C++ class template specializations and out-of-line definitions diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index ddcc3272cff1ff..027ed9140d726c 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -3977,7 +3977,7 @@ DeclResult Sema::ActOnVarTemplateSpecialization( } if (VarTemplate->hasAttr<DiagnoseSpecializationsAttr>()) - Diag(TemplateNameLoc, diag::warn_diag_specialization); + Diag(TemplateNameLoc, diag::warn_invalid_specialization) << VarTemplate; // Check for unexpanded parameter packs in any of the template arguments. for (unsigned I = 0, N = TemplateArgs.size(); I != N; ++I) @@ -8089,7 +8089,7 @@ DeclResult Sema::ActOnClassTemplateSpecialization( } if (ClassTemplate->hasAttr<DiagnoseSpecializationsAttr>()) - Diag(TemplateNameLoc, diag::warn_diag_specialization); + Diag(TemplateNameLoc, diag::warn_invalid_specialization) << ClassTemplate; bool isMemberSpecialization = false; bool isPartialSpecialization = false; diff --git a/clang/test/SemaCXX/attr-diagnose-specializations.cpp b/clang/test/SemaCXX/attr-diagnose-specializations.cpp index bf9ea2c56c18a6..a2d4021f947482 100644 --- a/clang/test/SemaCXX/attr-diagnose-specializations.cpp +++ b/clang/test/SemaCXX/attr-diagnose-specializations.cpp @@ -12,13 +12,13 @@ struct [[clang::diagnose_specializations]] is_same { }; template <> -struct is_same<int, char> {}; // expected-error {{specializing a template which should not be specialized}} +struct is_same<int, char> {}; // expected-error {{'is_same' should not be specialized}} template <class> struct Template {}; template <class T> -struct is_same<Template<T>, Template <T>> {}; // expected-error {{specializing a template which should not be specialized}} +struct is_same<Template<T>, Template <T>> {}; // expected-error {{'is_same' should not be specialized}} bool test_instantiation1 = is_same<int, int>::value; @@ -26,9 +26,9 @@ template <class T, class U> [[clang::diagnose_specializations]] inline constexpr bool is_same_v = __is_same(T, U); template <> -inline constexpr bool is_same_v<int, char> = false; // expected-error {{specializing a template which should not be specialized}} +inline constexpr bool is_same_v<int, char> = false; // expected-error {{'is_same_v' should not be specialized}} template <class T> -inline constexpr bool is_same_v<Template <T>, Template <T>> = true; // expected-error {{specializing a template which should not be specialized}} +inline constexpr bool is_same_v<Template <T>, Template <T>> = true; // expected-error {{'is_same_v' should not be specialized}} bool test_instantiation2 = is_same_v<int, int>; >From 919835cc70311451a27547bc29182847993b8631 Mon Sep 17 00:00:00 2001 From: Nikolas Klauser <nikolasklau...@berlin.de> Date: Sat, 10 Aug 2024 12:27:57 +0200 Subject: [PATCH 5/6] Add an optional message argument --- clang/include/clang/Basic/Attr.td | 1 + .../clang/Basic/DiagnosticSemaKinds.td | 5 ++++- clang/lib/Sema/SemaDeclAttr.cpp | 5 ++++- clang/lib/Sema/SemaTemplate.cpp | 20 +++++++++++++---- .../SemaCXX/attr-diagnose-specializations.cpp | 22 +++++++++++++++---- 5 files changed, 43 insertions(+), 10 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index e074cc8b285a95..412883c7df4730 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3332,6 +3332,7 @@ def DiagnoseIf : InheritableAttr { def DiagnoseSpecializations : InheritableAttr { let Spellings = [Clang<"diagnose_specializations", /*AllowInC*/0>]; + let Args = [StringArgument<"Message", 1>]; let Subjects = SubjectList<[ClassTmpl, VarTmpl]>; let Documentation = [DiagnoseSpecializationsDocs]; let MeaningfulToClassTemplateDefinition = 1; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 4d91b6671186dd..75046a8eedcccb 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -5408,7 +5408,10 @@ def note_dependent_function_template_spec_discard_reason : Note< "not a member of the enclosing %select{class template|" "namespace; did you mean to explicitly qualify the specialization?}1}0">; def warn_invalid_specialization : Warning< - "%0 should not be specialized">, + "%0 cannot be specialized">, + DefaultError, InGroup<InvalidSpecialization>; +def warn_invalid_specialization_message : Warning< + "%0 cannot be specialized: %1">, DefaultError, InGroup<InvalidSpecialization>; // C++ class template specializations and out-of-line definitions diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index eb0a705fef0449..442d9562df3fa5 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -1217,8 +1217,11 @@ static void handlePreferredName(Sema &S, Decl *D, const ParsedAttr &AL) { static void handleDiagnoseSpecializations(Sema &S, Decl *D, const ParsedAttr &AL) { + StringRef Message; + if (AL.getNumArgs() != 0) + S.checkStringLiteralArgumentAttr(AL, 0, Message); D->getDescribedTemplate()->addAttr( - DiagnoseSpecializationsAttr::Create(S.Context, AL)); + DiagnoseSpecializationsAttr::Create(S.Context, Message, AL)); } bool Sema::isValidPointerAttrType(QualType T, bool RefOkay) { diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 027ed9140d726c..a8ad65fb1b6fed 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -3976,8 +3976,14 @@ DeclResult Sema::ActOnVarTemplateSpecialization( << IsPartialSpecialization; } - if (VarTemplate->hasAttr<DiagnoseSpecializationsAttr>()) - Diag(TemplateNameLoc, diag::warn_invalid_specialization) << VarTemplate; + if (const auto *DSA = VarTemplate->getAttr<DiagnoseSpecializationsAttr>()) { + if (auto Message = DSA->getMessage(); !Message.empty()) { + Diag(TemplateNameLoc, diag::warn_invalid_specialization_message) + << VarTemplate << Message; + } else { + Diag(TemplateNameLoc, diag::warn_invalid_specialization) << VarTemplate; + } + } // Check for unexpanded parameter packs in any of the template arguments. for (unsigned I = 0, N = TemplateArgs.size(); I != N; ++I) @@ -8088,8 +8094,14 @@ DeclResult Sema::ActOnClassTemplateSpecialization( return true; } - if (ClassTemplate->hasAttr<DiagnoseSpecializationsAttr>()) - Diag(TemplateNameLoc, diag::warn_invalid_specialization) << ClassTemplate; + if (const auto *DSA = ClassTemplate->getAttr<DiagnoseSpecializationsAttr>()) { + if (auto Message = DSA->getMessage(); !Message.empty()) { + Diag(TemplateNameLoc, diag::warn_invalid_specialization_message) + << ClassTemplate << Message; + } else { + Diag(TemplateNameLoc, diag::warn_invalid_specialization) << ClassTemplate; + } + } bool isMemberSpecialization = false; bool isPartialSpecialization = false; diff --git a/clang/test/SemaCXX/attr-diagnose-specializations.cpp b/clang/test/SemaCXX/attr-diagnose-specializations.cpp index a2d4021f947482..7dfdf6fdd1a5b4 100644 --- a/clang/test/SemaCXX/attr-diagnose-specializations.cpp +++ b/clang/test/SemaCXX/attr-diagnose-specializations.cpp @@ -12,13 +12,13 @@ struct [[clang::diagnose_specializations]] is_same { }; template <> -struct is_same<int, char> {}; // expected-error {{'is_same' should not be specialized}} +struct is_same<int, char> {}; // expected-error {{'is_same' cannot be specialized}} template <class> struct Template {}; template <class T> -struct is_same<Template<T>, Template <T>> {}; // expected-error {{'is_same' should not be specialized}} +struct is_same<Template<T>, Template <T>> {}; // expected-error {{'is_same' cannot be specialized}} bool test_instantiation1 = is_same<int, int>::value; @@ -26,9 +26,23 @@ template <class T, class U> [[clang::diagnose_specializations]] inline constexpr bool is_same_v = __is_same(T, U); template <> -inline constexpr bool is_same_v<int, char> = false; // expected-error {{'is_same_v' should not be specialized}} +inline constexpr bool is_same_v<int, char> = false; // expected-error {{'is_same_v' cannot be specialized}} template <class T> -inline constexpr bool is_same_v<Template <T>, Template <T>> = true; // expected-error {{'is_same_v' should not be specialized}} +inline constexpr bool is_same_v<Template <T>, Template <T>> = true; // expected-error {{'is_same_v' cannot be specialized}} bool test_instantiation2 = is_same_v<int, int>; + +template <class T> +struct [[clang::diagnose_specializations("specializing type traits results in undefined behaviour")]] is_trivial { + static constexpr bool value = __is_trivial(T); +}; + +template <> +struct is_trivial<int> {}; // expected-error {{'is_trivial' cannot be specialized: specializing type traits results in undefined behaviour}} + +template <class T> +[[clang::diagnose_specializations("specializing type traits results in undefined behaviour")]] inline constexpr bool is_trivial_v = __is_trivial(T); + +template <> +inline constexpr bool is_trivial_v<int> = false; // expected-error {{'is_trivial_v' cannot be specialized: specializing type traits results in undefined behaviour}} >From 003983f992e05357ddf338e757f53d0e4c9e5959 Mon Sep 17 00:00:00 2001 From: Nikolas Klauser <nikolasklau...@berlin.de> Date: Sun, 25 Aug 2024 23:26:23 +0200 Subject: [PATCH 6/6] Address comments --- clang/docs/ReleaseNotes.rst | 2 +- clang/include/clang/Basic/Attr.td | 6 +++--- clang/include/clang/Basic/AttrDocs.td | 20 +++++++++--------- .../clang/Basic/DiagnosticSemaKinds.td | 6 ++---- clang/lib/Sema/SemaTemplate.cpp | 20 +++++++----------- .../SemaCXX/attr-diagnose-specializations.cpp | 21 +++++++++++++------ 6 files changed, 39 insertions(+), 36 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 38a2e34a9a3f4c..c18aadb5bfa361 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -210,7 +210,7 @@ Attribute Changes in Clang - The ``hybrid_patchable`` attribute is now supported on ARM64EC targets. It can be used to specify that a function requires an additional x86-64 thunk, which may be patched at runtime. -- The attribute ``[[clang::diagnose_specializations]]`` has been added to warn +- The attribute ``[[clang::no_specializations]]`` has been added to warn users that a specific template shouldn't be specialized. This is useful for e.g. standard library type traits, where adding a specialization results in undefined behaviour. diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 7d19fb1577388b..06fecbec8d55f5 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -104,7 +104,7 @@ def NonLocalVar : SubsetSubject<Var, [{!S->hasLocalStorage()}], "variables with non-local storage">; def VarTmpl : SubsetSubject<Var, [{S->getDescribedVarTemplate()}], - "variable template">; + "variable templates">; def NonBitField : SubsetSubject<Field, [{!S->isBitField()}], @@ -3340,7 +3340,7 @@ def DiagnoseIf : InheritableAttr { } def DiagnoseSpecializations : InheritableAttr { - let Spellings = [Clang<"diagnose_specializations", /*AllowInC*/0>]; + let Spellings = [Clang<"no_specializations", /*AllowInC*/0>]; let Args = [StringArgument<"Message", 1>]; let Subjects = SubjectList<[ClassTmpl, VarTmpl]>; let Documentation = [DiagnoseSpecializationsDocs]; @@ -4628,7 +4628,7 @@ def HLSLResource : InheritableAttr { def HLSLROV : InheritableAttr { let Spellings = [CXX11<"hlsl", "is_rov">]; let Subjects = SubjectList<[Struct]>; - let LangOpts = [HLSL]; + let LangOpts = [HLSL]; let Documentation = [InternalOnly]; } diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 27bc60731fc321..22f157cdf9530c 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -978,9 +978,9 @@ Query for this feature with ``__has_attribute(diagnose_if)``. def DiagnoseSpecializationsDocs : Documentation { let Category = DocCatDecl; let Content = [{ -``[[clang::diagnose_specializations]]`` can be applied to class or variable -templates which should not be specialized by users. This is primarily used to -diagnose user specializations of standard library type traits. +``[[clang::no_specializations]]`` can be applied to class or variable +templates which should not be explicitly specialized by users. This is primarily +used to diagnose user specializations of standard library type traits. }]; } @@ -6839,8 +6839,8 @@ the field it is attached to, and it may also lead to emission of automatic fix-i hints which would help the user replace the use of unsafe functions(/fields) with safe alternatives, though the attribute can be used even when the fix can't be automated. -* Attribute attached to functions: The attribute does not suppress - ``-Wunsafe-buffer-usage`` inside the function to which it is attached. +* Attribute attached to functions: The attribute does not suppress + ``-Wunsafe-buffer-usage`` inside the function to which it is attached. These warnings still need to be addressed. The attribute is warranted even if the only way a function can overflow @@ -6903,10 +6903,10 @@ alternatives, though the attribute can be used even when the fix can't be automa and then use the attribute on the original ``baz()`` to help the users update their code to use the new function. -* Attribute attached to fields: The attribute should only be attached to - struct fields, if the fields can not be updated to a safe type with bounds - check, such as std::span. In other words, the buffers prone to unsafe accesses - should always be updated to use safe containers/views and attaching the attribute +* Attribute attached to fields: The attribute should only be attached to + struct fields, if the fields can not be updated to a safe type with bounds + check, such as std::span. In other words, the buffers prone to unsafe accesses + should always be updated to use safe containers/views and attaching the attribute must be last resort when such an update is infeasible. The attribute can be placed on individual fields or a set of them as shown below. @@ -6924,7 +6924,7 @@ alternatives, though the attribute can be used even when the fix can't be automa size_t sz; }; - Here, every read/write to the fields ptr1, ptr2, buf and sz will trigger a warning + Here, every read/write to the fields ptr1, ptr2, buf and sz will trigger a warning that the field has been explcitly marked as unsafe due to unsafe-buffer operations. }]; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 44f580ade10626..b112b4a60d57f5 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -5417,11 +5417,9 @@ def note_dependent_function_template_spec_discard_reason : Note< "not a member of the enclosing %select{class template|" "namespace; did you mean to explicitly qualify the specialization?}1}0">; def warn_invalid_specialization : Warning< - "%0 cannot be specialized">, - DefaultError, InGroup<InvalidSpecialization>; -def warn_invalid_specialization_message : Warning< - "%0 cannot be specialized: %1">, + "%0 cannot be specialized%select{|: %2}1">, DefaultError, InGroup<InvalidSpecialization>; +def note_marked_here : Note<"marked %0 here">; // C++ class template specializations and out-of-line definitions def err_template_spec_needs_header : Error< diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 70999ec86e7699..d6852be05ee66e 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -3980,12 +3980,10 @@ DeclResult Sema::ActOnVarTemplateSpecialization( } if (const auto *DSA = VarTemplate->getAttr<DiagnoseSpecializationsAttr>()) { - if (auto Message = DSA->getMessage(); !Message.empty()) { - Diag(TemplateNameLoc, diag::warn_invalid_specialization_message) - << VarTemplate << Message; - } else { - Diag(TemplateNameLoc, diag::warn_invalid_specialization) << VarTemplate; - } + auto Message = DSA->getMessage(); + Diag(TemplateNameLoc, diag::warn_invalid_specialization) + << VarTemplate << !Message.empty() << Message; + Diag(DSA->getLoc(), diag::note_marked_here) << DSA; } // Check for unexpanded parameter packs in any of the template arguments. @@ -8099,12 +8097,10 @@ DeclResult Sema::ActOnClassTemplateSpecialization( } if (const auto *DSA = ClassTemplate->getAttr<DiagnoseSpecializationsAttr>()) { - if (auto Message = DSA->getMessage(); !Message.empty()) { - Diag(TemplateNameLoc, diag::warn_invalid_specialization_message) - << ClassTemplate << Message; - } else { - Diag(TemplateNameLoc, diag::warn_invalid_specialization) << ClassTemplate; - } + auto Message = DSA->getMessage(); + Diag(TemplateNameLoc, diag::warn_invalid_specialization) + << ClassTemplate << !Message.empty() << Message; + Diag(DSA->getLoc(), diag::note_marked_here) << DSA; } DeclContext *DC = ClassTemplate->getDeclContext(); diff --git a/clang/test/SemaCXX/attr-diagnose-specializations.cpp b/clang/test/SemaCXX/attr-diagnose-specializations.cpp index 7dfdf6fdd1a5b4..ba8ec6bea58e69 100644 --- a/clang/test/SemaCXX/attr-diagnose-specializations.cpp +++ b/clang/test/SemaCXX/attr-diagnose-specializations.cpp @@ -1,16 +1,19 @@ // RUN: %clang_cc1 %s -verify -#if !__has_cpp_attribute(clang::diagnose_specializations) +#if !__has_cpp_attribute(clang::no_specializations) # error #endif -struct [[clang::diagnose_specializations]] S {}; // expected-warning {{'diagnose_specializations' attribute only applies to class templates}} +struct [[clang::no_specializations]] S {}; // expected-warning {{'no_specializations' attribute only applies to class templates and variable templates}} template <class T, class U> -struct [[clang::diagnose_specializations]] is_same { +struct [[clang::no_specializations]] is_same { // expected-note 2 {{marked 'no_specializations' here}} static constexpr bool value = __is_same(T, U); }; +template <class T> +using alias [[clang::no_specializations]] = T; // expected-warning {{'no_specializations' attribute only applies to class templates and variable templates}} + template <> struct is_same<int, char> {}; // expected-error {{'is_same' cannot be specialized}} @@ -23,7 +26,7 @@ struct is_same<Template<T>, Template <T>> {}; // expected-error {{'is_same' cann bool test_instantiation1 = is_same<int, int>::value; template <class T, class U> -[[clang::diagnose_specializations]] inline constexpr bool is_same_v = __is_same(T, U); +[[clang::no_specializations]] inline constexpr bool is_same_v = __is_same(T, U); // expected-note 2 {{marked 'no_specializations' here}} template <> inline constexpr bool is_same_v<int, char> = false; // expected-error {{'is_same_v' cannot be specialized}} @@ -34,7 +37,7 @@ inline constexpr bool is_same_v<Template <T>, Template <T>> = true; // expected- bool test_instantiation2 = is_same_v<int, int>; template <class T> -struct [[clang::diagnose_specializations("specializing type traits results in undefined behaviour")]] is_trivial { +struct [[clang::no_specializations("specializing type traits results in undefined behaviour")]] is_trivial { // expected-note {{marked 'no_specializations' here}} static constexpr bool value = __is_trivial(T); }; @@ -42,7 +45,13 @@ template <> struct is_trivial<int> {}; // expected-error {{'is_trivial' cannot be specialized: specializing type traits results in undefined behaviour}} template <class T> -[[clang::diagnose_specializations("specializing type traits results in undefined behaviour")]] inline constexpr bool is_trivial_v = __is_trivial(T); +[[clang::no_specializations("specializing type traits results in undefined behaviour")]] inline constexpr bool is_trivial_v = __is_trivial(T); // expected-note {{marked 'no_specializations' here}} template <> inline constexpr bool is_trivial_v<int> = false; // expected-error {{'is_trivial_v' cannot be specialized: specializing type traits results in undefined behaviour}} + +template <class T> +struct Partial {}; + +template <class T> +struct [[clang::no_specializations]] Partial<Template <T>> {}; // expected-warning {{'no_specializations' attribute only applies to class templates and variable templates}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits