[clang] Warning for incorrect useof 'pure' attribute (PR #78200)

via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 16 02:52:34 PST 2024


https://github.com/kelbon updated https://github.com/llvm/llvm-project/pull/78200

>From b080d04eb30254502ccd5d59d76b5197db1fa88d Mon Sep 17 00:00:00 2001
From: Kelbon Nik <kelbonage at 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 414779a7970ab8..0ad3ea64503d81 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 4e7049571eeb7a..e340028703b3b3 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11889,6 +11889,13 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
     NewFD->setInvalidDecl();
   }
 
+  if (NewFD->hasAttr<PureAttr>() || NewFD->hasAttr<ConstAttr>()) {
+    if (isa_and_nonnull<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 d43afccb027ea0e02c97ab9fbe55a1ad6c9d71dd Mon Sep 17 00:00:00 2001
From: Kelbon Nik <kelbonage at gmail.com>
Date: Mon, 15 Jan 2024 22:52:23 +0400
Subject: [PATCH 2/5] use precondition: NewFD is not null

---
 clang/lib/Sema/SemaDecl.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index e340028703b3b3..dcbc5c3c842cca 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11890,7 +11890,7 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
   }
 
   if (NewFD->hasAttr<PureAttr>() || NewFD->hasAttr<ConstAttr>()) {
-    if (isa_and_nonnull<CXXConstructorDecl>(NewFD))
+    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);

>From 950ca9de1c05d561a1123c088455a3e21bd9795b Mon Sep 17 00:00:00 2001
From: Kelbon Nik <kelbonage at gmail.com>
Date: Tue, 16 Jan 2024 00:03:47 +0400
Subject: [PATCH 3/5] fix old incorrect test

---
 clang/test/SemaCXX/cxx0x-cursory-default-delete.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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}}

>From 74bf7b1840cf2b20baae3348c0e46b44a04a5408 Mon Sep 17 00:00:00 2001
From: Kelbon Nik <kelbonage at gmail.com>
Date: Tue, 16 Jan 2024 11:29:49 +0400
Subject: [PATCH 4/5] fix another old test

---
 clang/test/SemaCXX/warn-unused-value-cxx11.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 bde11e037554481f90852e2ecd72fea9a3faffe6 Mon Sep 17 00:00:00 2001
From: Kelbon Nik <kelbonage at gmail.com>
Date: Tue, 16 Jan 2024 14:52:19 +0400
Subject: [PATCH 5/5] fix all old tests

---
 clang/test/Analysis/call-invalidation.cpp         | 8 ++++----
 clang/test/CodeGen/pragma-weak.c                  | 6 +++---
 clang/test/Interpreter/disambiguate-decl-stmt.cpp | 4 ++--
 3 files changed, 9 insertions(+), 9 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 {};



More information about the cfe-commits mailing list