https://github.com/kelbon updated https://github.com/llvm/llvm-project/pull/78200
>From e8c799b54c3e0b8088dbc19aaac0ef066baf136e Mon Sep 17 00:00:00 2001 From: Kelbon Nik <kelbon...@gmail.com> Date: Mon, 15 Jan 2024 22:24:34 +0400 Subject: [PATCH 1/5] add warning and test --- clang/include/clang/Basic/DiagnosticGroups.td | 1 + clang/include/clang/Basic/DiagnosticSemaKinds.td | 7 +++++++ clang/lib/Sema/SemaDecl.cpp | 7 +++++++ clang/test/Sema/incorrect_pure.cpp | 7 +++++++ 4 files changed, 22 insertions(+) create mode 100644 clang/test/Sema/incorrect_pure.cpp diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 6765721ae7002c..9fcf2be2e45458 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -414,6 +414,7 @@ def : DiagGroup<"c++2a-compat", [CXX20Compat]>; def : DiagGroup<"c++2a-compat-pedantic", [CXX20CompatPedantic]>; def ExitTimeDestructors : DiagGroup<"exit-time-destructors">; +def IncorrectAttributeUsage : DiagGroup<"incorrect-attribute-usage">; def FlexibleArrayExtensions : DiagGroup<"flexible-array-extensions">; def FourByteMultiChar : DiagGroup<"four-char-constants">; def GlobalConstructors : DiagGroup<"global-constructors"> { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 03b0122d1c08f7..1df075119a482f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -692,6 +692,13 @@ def warn_maybe_falloff_nonvoid_function : Warning< def warn_falloff_nonvoid_function : Warning< "non-void function does not return a value">, InGroup<ReturnType>; +def warn_pure_attr_on_cxx_constructor : Warning< + "constructor cannot be 'pure' (undefined behavior)">, + InGroup<IncorrectAttributeUsage>; +def warn_pure_function_returns_void : Warning< + "'pure' attribute on function returning 'void'">, + InGroup<IncorrectAttributeUsage>; + def err_maybe_falloff_nonvoid_block : Error< "non-void block does not return a value in all control paths">; def err_falloff_nonvoid_block : Error< diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index dae98f3a7406e8..86dc022fbe055d 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -11899,6 +11899,13 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD, NewFD->setInvalidDecl(); } + if (NewFD->hasAttr<PureAttr>() || NewFD->hasAttr<ConstAttr>()) { + if (isa<CXXConstructorDecl>(NewFD)) + Diag(NewFD->getLocation(), diag::warn_pure_attr_on_cxx_constructor); + else if (NewFD->getReturnType()->isVoidType()) + Diag(NewFD->getLocation(), diag::warn_pure_function_returns_void); + } + // C++11 [dcl.constexpr]p8: // A constexpr specifier for a non-static member function that is not // a constructor declares that member function to be const. diff --git a/clang/test/Sema/incorrect_pure.cpp b/clang/test/Sema/incorrect_pure.cpp new file mode 100644 index 00000000000000..ce02309f086386 --- /dev/null +++ b/clang/test/Sema/incorrect_pure.cpp @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +[[gnu::pure]] void foo(); // expected-warning{{'pure' attribute on function returning 'void'}} + +struct A { + [[gnu::pure]] A(); // expected-warning{{constructor cannot be 'pure' (undefined behavior)}} +}; >From d152fa4881d6052bfad33c922f276d4909c305ec Mon Sep 17 00:00:00 2001 From: Kelbon Nik <kelbon...@gmail.com> Date: Tue, 16 Jan 2024 00:03:47 +0400 Subject: [PATCH 2/5] fix old incorrect test --- clang/test/Analysis/call-invalidation.cpp | 8 ++++---- clang/test/CodeGen/pragma-weak.c | 6 +++--- clang/test/Interpreter/disambiguate-decl-stmt.cpp | 4 ++-- clang/test/SemaCXX/cxx0x-cursory-default-delete.cpp | 2 +- clang/test/SemaCXX/warn-unused-value-cxx11.cpp | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/clang/test/Analysis/call-invalidation.cpp b/clang/test/Analysis/call-invalidation.cpp index ef6505e19cf803..727217f228b054 100644 --- a/clang/test/Analysis/call-invalidation.cpp +++ b/clang/test/Analysis/call-invalidation.cpp @@ -90,8 +90,8 @@ void testConstReferenceStruct() { } -void usePointerPure(int * const *) __attribute__((pure)); -void usePointerConst(int * const *) __attribute__((const)); +int usePointerPure(int * const *) __attribute__((pure)); +int usePointerConst(int * const *) __attribute__((const)); void testPureConst() { extern int global; @@ -104,11 +104,11 @@ void testPureConst() { clang_analyzer_eval(x == 42); // expected-warning{{TRUE}} clang_analyzer_eval(global == -5); // expected-warning{{TRUE}} - usePointerPure(&p); + (void)usePointerPure(&p); clang_analyzer_eval(x == 42); // expected-warning{{TRUE}} clang_analyzer_eval(global == -5); // expected-warning{{TRUE}} - usePointerConst(&p); + (void)usePointerConst(&p); clang_analyzer_eval(x == 42); // expected-warning{{TRUE}} clang_analyzer_eval(global == -5); // expected-warning{{TRUE}} diff --git a/clang/test/CodeGen/pragma-weak.c b/clang/test/CodeGen/pragma-weak.c index 52328bf9ff1be7..d4011628bda7f1 100644 --- a/clang/test/CodeGen/pragma-weak.c +++ b/clang/test/CodeGen/pragma-weak.c @@ -16,7 +16,7 @@ // CHECK-DAG: @declfirstattr = weak{{.*}} alias void (), ptr @__declfirstattr // CHECK-DAG: @mix2 = weak{{.*}} alias void (), ptr @__mix2 // CHECK-DAG: @a1 = weak{{.*}} alias void (), ptr @__a1 -// CHECK-DAG: @xxx = weak{{.*}} alias void (), ptr @__xxx +// CHECK-DAG: @xxx = weak{{.*}} alias int (), ptr @__xxx // CHECK-DAG: @undecfunc_alias1 = weak{{.*}} alias void (), ptr @undecfunc // CHECK-DAG: @undecfunc_alias2 = weak{{.*}} alias void (), ptr @undecfunc // CHECK-DAG: @undecfunc_alias3 = weak{{.*}} alias void (), ptr @undecfunc @@ -137,8 +137,8 @@ void __a1(void) {} // CHECK: define{{.*}} void @__a1() [[NI:#[0-9]+]] #pragma weak xxx = __xxx -__attribute((pure,noinline,const)) void __xxx(void) { } -// CHECK: void @__xxx() [[RN:#[0-9]+]] +__attribute((pure,noinline,const)) int __xxx(void) { return 0; } +// CHECK: int @__xxx() [[RN:#[0-9]+]] ///////////// PR28611: Try multiple aliases of same undeclared symbol or alias #pragma weak undecfunc_alias1 = undecfunc diff --git a/clang/test/Interpreter/disambiguate-decl-stmt.cpp b/clang/test/Interpreter/disambiguate-decl-stmt.cpp index a49d7013c540ac..1f4d5e267288bc 100644 --- a/clang/test/Interpreter/disambiguate-decl-stmt.cpp +++ b/clang/test/Interpreter/disambiguate-decl-stmt.cpp @@ -95,10 +95,10 @@ Ns::Ns::Fs(); Ns::Ns::Ns(); struct Attrs1 { Attrs1(); }; -Attrs1::Attrs1() __attribute((pure)) = default; +Attrs1::Attrs1() __attribute((noreturn)) = default; struct Attrs2 { Attrs2(); }; -__attribute((pure)) Attrs2::Attrs2() = default; +__attribute((noreturn)) Attrs2::Attrs2() = default; // Extra semicolon namespace N {}; diff --git a/clang/test/SemaCXX/cxx0x-cursory-default-delete.cpp b/clang/test/SemaCXX/cxx0x-cursory-default-delete.cpp index 9d68a0e5d358f6..6ae146f0d08c7d 100644 --- a/clang/test/SemaCXX/cxx0x-cursory-default-delete.cpp +++ b/clang/test/SemaCXX/cxx0x-cursory-default-delete.cpp @@ -194,7 +194,7 @@ struct except_spec_d_match : except_spec_a, except_spec_b { // gcc-compatibility: allow attributes on default definitions // (but not normal definitions) struct S { S(); }; -S::S() __attribute((pure)) = default; +S::S() __attribute((noreturn)) = default; using size_t = decltype(sizeof(0)); void *operator new(size_t) = delete; // expected-error {{deleted definition must be first declaration}} expected-note {{implicit}} diff --git a/clang/test/SemaCXX/warn-unused-value-cxx11.cpp b/clang/test/SemaCXX/warn-unused-value-cxx11.cpp index a6f41c3fd6b3b2..687278a98f4e1a 100644 --- a/clang/test/SemaCXX/warn-unused-value-cxx11.cpp +++ b/clang/test/SemaCXX/warn-unused-value-cxx11.cpp @@ -1,6 +1,6 @@ // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -Wunused-value %s -void f() __attribute__((const)); +int f() __attribute__((const)); namespace PR18571 { // Unevaluated contexts should not trigger unused result warnings. >From 58e264511cd8b2f2eb20bb23ec797d8dadd5ac19 Mon Sep 17 00:00:00 2001 From: Kelbon Nik <kelbon...@gmail.com> Date: Wed, 17 Jan 2024 15:00:39 +0400 Subject: [PATCH 3/5] release notes, update warning message, handle pure && const declarations --- clang/docs/ReleaseNotes.rst | 2 +- clang/include/clang/Basic/DiagnosticGroups.td | 1 - .../clang/Basic/DiagnosticSemaKinds.td | 10 +++---- clang/lib/Sema/SemaDecl.cpp | 27 ++++++++++++++----- clang/test/CodeGen/pragma-weak.c | 6 ++--- clang/test/Sema/attr-print.c | 8 +++--- clang/test/Sema/incorrect_pure.cpp | 11 ++++++-- 7 files changed, 43 insertions(+), 22 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d63781cf8b11bf..e929852b0994a1 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -569,7 +569,7 @@ Improvements to Clang's diagnostics and template friend declarations with a constraint that depends on a template parameter from an enclosing template must be a definition. - Clang now diagnoses function/variable templates that shadow their own template parameters, e.g. ``template<class T> void T();``. - +- Clang now diagnoses incorrect usage of ``const`` and ``pure`` attributes, so ``-Wignored-attributes`` diagnoses more cases. Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 9fcf2be2e45458..6765721ae7002c 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -414,7 +414,6 @@ def : DiagGroup<"c++2a-compat", [CXX20Compat]>; def : DiagGroup<"c++2a-compat-pedantic", [CXX20CompatPedantic]>; def ExitTimeDestructors : DiagGroup<"exit-time-destructors">; -def IncorrectAttributeUsage : DiagGroup<"incorrect-attribute-usage">; def FlexibleArrayExtensions : DiagGroup<"flexible-array-extensions">; def FourByteMultiChar : DiagGroup<"four-char-constants">; def GlobalConstructors : DiagGroup<"global-constructors"> { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 1df075119a482f..ee74a8211c34af 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -692,12 +692,12 @@ def warn_maybe_falloff_nonvoid_function : Warning< def warn_falloff_nonvoid_function : Warning< "non-void function does not return a value">, InGroup<ReturnType>; -def warn_pure_attr_on_cxx_constructor : Warning< - "constructor cannot be 'pure' (undefined behavior)">, - InGroup<IncorrectAttributeUsage>; +def warn_const_attr_with_pure_attr : Warning< + "'const' attribute imposes more restrictions, 'pure' attribute ignored">, + InGroup<IgnoredAttributes>; def warn_pure_function_returns_void : Warning< - "'pure' attribute on function returning 'void'">, - InGroup<IncorrectAttributeUsage>; + "'%select{pure|const}0' attribute on function returning 'void', attribute ignored">, + InGroup<IgnoredAttributes>; def err_maybe_falloff_nonvoid_block : Error< "non-void block does not return a value in all control paths">; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 86dc022fbe055d..5b35783a268948 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -11802,6 +11802,26 @@ static bool CheckMultiVersionFunction(Sema &S, FunctionDecl *NewFD, OldDecl, Previous); } +static void CheckFunctionDeclarationAttributesUsage(Sema &S, + FunctionDecl *NewFD) { + bool IsPure = NewFD->hasAttr<PureAttr>(); + bool IsConst = NewFD->hasAttr<ConstAttr>(); + + if (IsPure && IsConst) { + S.Diag(NewFD->getLocation(), diag::warn_const_attr_with_pure_attr); + NewFD->dropAttr<PureAttr>(); + } + if (IsPure || IsConst) { + // Constructors and destructors are functions which return void, so are handled here as well. + if (NewFD->getReturnType()->isVoidType()) { + S.Diag(NewFD->getLocation(), diag::warn_pure_function_returns_void) + << IsConst; + NewFD->dropAttr<PureAttr>(); + NewFD->dropAttr<ConstAttr>(); + } + } +} + /// Perform semantic checking of a new function declaration. /// /// Performs semantic analysis of the new function declaration @@ -11899,12 +11919,7 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD, NewFD->setInvalidDecl(); } - if (NewFD->hasAttr<PureAttr>() || NewFD->hasAttr<ConstAttr>()) { - if (isa<CXXConstructorDecl>(NewFD)) - Diag(NewFD->getLocation(), diag::warn_pure_attr_on_cxx_constructor); - else if (NewFD->getReturnType()->isVoidType()) - Diag(NewFD->getLocation(), diag::warn_pure_function_returns_void); - } + CheckFunctionDeclarationAttributesUsage(*this, NewFD); // C++11 [dcl.constexpr]p8: // A constexpr specifier for a non-static member function that is not diff --git a/clang/test/CodeGen/pragma-weak.c b/clang/test/CodeGen/pragma-weak.c index d4011628bda7f1..ea508a485ef612 100644 --- a/clang/test/CodeGen/pragma-weak.c +++ b/clang/test/CodeGen/pragma-weak.c @@ -16,7 +16,7 @@ // CHECK-DAG: @declfirstattr = weak{{.*}} alias void (), ptr @__declfirstattr // CHECK-DAG: @mix2 = weak{{.*}} alias void (), ptr @__mix2 // CHECK-DAG: @a1 = weak{{.*}} alias void (), ptr @__a1 -// CHECK-DAG: @xxx = weak{{.*}} alias int (), ptr @__xxx +// CHECK-DAG: @xxx = weak{{.*}} alias i32 (), ptr @__xxx // CHECK-DAG: @undecfunc_alias1 = weak{{.*}} alias void (), ptr @undecfunc // CHECK-DAG: @undecfunc_alias2 = weak{{.*}} alias void (), ptr @undecfunc // CHECK-DAG: @undecfunc_alias3 = weak{{.*}} alias void (), ptr @undecfunc @@ -137,8 +137,8 @@ void __a1(void) {} // CHECK: define{{.*}} void @__a1() [[NI:#[0-9]+]] #pragma weak xxx = __xxx -__attribute((pure,noinline,const)) int __xxx(void) { return 0; } -// CHECK: int @__xxx() [[RN:#[0-9]+]] +__attribute((noinline,const)) int __xxx(void) { return 0; } +// CHECK: i32 @__xxx() [[RN:#[0-9]+]] ///////////// PR28611: Try multiple aliases of same undeclared symbol or alias #pragma weak undecfunc_alias1 = undecfunc diff --git a/clang/test/Sema/attr-print.c b/clang/test/Sema/attr-print.c index ffa27de4a5d306..8492356e5d2e57 100644 --- a/clang/test/Sema/attr-print.c +++ b/clang/test/Sema/attr-print.c @@ -9,11 +9,11 @@ __declspec(align(4)) int y; // CHECK: short arr[3] __attribute__((aligned)); short arr[3] __attribute__((aligned)); -// CHECK: void foo(void) __attribute__((const)); -void foo(void) __attribute__((const)); +// CHECK: int foo(void) __attribute__((const)); +int foo(void) __attribute__((const)); -// CHECK: void bar(void) __attribute__((__const)); -void bar(void) __attribute__((__const)); +// CHECK: int bar(void) __attribute__((__const)); +int bar(void) __attribute__((__const)); // CHECK: int * __ptr32 p32; int * __ptr32 p32; diff --git a/clang/test/Sema/incorrect_pure.cpp b/clang/test/Sema/incorrect_pure.cpp index ce02309f086386..1fa17785ec3880 100644 --- a/clang/test/Sema/incorrect_pure.cpp +++ b/clang/test/Sema/incorrect_pure.cpp @@ -1,7 +1,14 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s -[[gnu::pure]] void foo(); // expected-warning{{'pure' attribute on function returning 'void'}} +[[gnu::pure]] void foo(); // expected-warning{{'pure' attribute on function returning 'void', attribute ignored}} + +[[gnu::const]] void bar(); // expected-warning{{'const' attribute on function returning 'void', attribute ignored}} struct A { - [[gnu::pure]] A(); // expected-warning{{constructor cannot be 'pure' (undefined behavior)}} + [[gnu::pure]] A(); // expected-warning{{'pure' attribute on function returning 'void', attribute ignored}} + + [[gnu::const]] A(int); // expected-warning{{'const' attribute on function returning 'void', attribute ignored}} + [[gnu::pure]] ~A(); // expected-warning{{'pure' attribute on function returning 'void', attribute ignored}} + + [[gnu::const]] [[gnu::pure]] int m(); // expected-warning{{'const' attribute imposes more restrictions, 'pure' attribute ignored}} }; >From f52fa936035a10cfe852b8a52b47a68138c2e4d9 Mon Sep 17 00:00:00 2001 From: Kelbon Nik <kelbon...@gmail.com> Date: Wed, 17 Jan 2024 22:12:22 +0400 Subject: [PATCH 4/5] format --- clang/lib/Sema/SemaDecl.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 5b35783a268948..53a57e13a05732 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -11812,7 +11812,8 @@ static void CheckFunctionDeclarationAttributesUsage(Sema &S, NewFD->dropAttr<PureAttr>(); } if (IsPure || IsConst) { - // Constructors and destructors are functions which return void, so are handled here as well. + // Constructors and destructors are functions which return void, so are + // handled here as well. if (NewFD->getReturnType()->isVoidType()) { S.Diag(NewFD->getLocation(), diag::warn_pure_function_returns_void) << IsConst; >From 712950f0fa8bb669ebb71b5c30b159f509619144 Mon Sep 17 00:00:00 2001 From: Kelbon Nik <kelbon...@gmail.com> Date: Wed, 17 Jan 2024 22:56:51 +0400 Subject: [PATCH 5/5] fix old tests --- clang/test/SemaCXX/attr-print.cpp | 12 ++++++------ clang/test/SemaCXX/cxx11-attr-print.cpp | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/clang/test/SemaCXX/attr-print.cpp b/clang/test/SemaCXX/attr-print.cpp index dff290be696be2..c7c58a25504eed 100644 --- a/clang/test/SemaCXX/attr-print.cpp +++ b/clang/test/SemaCXX/attr-print.cpp @@ -6,15 +6,15 @@ int x __attribute__((aligned(4))); // CHECK: __declspec(align(4)) int y; __declspec(align(4)) int y; -// CHECK: void foo() __attribute__((const)); -void foo() __attribute__((const)); +// CHECK: int foo() __attribute__((const)); +int foo() __attribute__((const)); -// CHECK: void bar() __attribute__((__const)); -void bar() __attribute__((__const)); +// CHECK: int bar() __attribute__((__const)); +int bar() __attribute__((__const)); // FIXME: Print this with correct format. -// CHECK: void foo1() __attribute__((noinline)) __attribute__((pure)); -void foo1() __attribute__((noinline, pure)); +// CHECK: int foo1() __attribute__((noinline)) __attribute__((pure)); +int foo1() __attribute__((noinline, pure)); // CHECK: typedef int Small1 __attribute__((mode(byte))); typedef int Small1 __attribute__((mode(byte))); diff --git a/clang/test/SemaCXX/cxx11-attr-print.cpp b/clang/test/SemaCXX/cxx11-attr-print.cpp index 7095c462031d93..c988972aeb1a5a 100644 --- a/clang/test/SemaCXX/cxx11-attr-print.cpp +++ b/clang/test/SemaCXX/cxx11-attr-print.cpp @@ -30,11 +30,11 @@ alignas(4) int cxx11_alignas; // CHECK: int c11_alignas _Alignas(int); _Alignas(int) int c11_alignas; -// CHECK: void foo() __attribute__((const)); -void foo() __attribute__((const)); +// CHECK: int foo() __attribute__((const)); +int foo() __attribute__((const)); -// CHECK: void bar() __attribute__((__const)); -void bar() __attribute__((__const)); +// CHECK: int bar() __attribute__((__const)); +int bar() __attribute__((__const)); // FIXME: It's unfortunate that the string literal prints with the below three // cases given that the string is only exposed via the [[nodiscard]] spelling. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits