[PATCH] Add clang's invalid-noreturn warning flag

[PATCH] Add clang's invalid-noreturn warning flag

[Julian Waters]

[-- Attachment #1.1: Type: text/plain, Size: 1389 bytes --]

Please review a patch to add clang's invalid-noreturn flag to toggle
noreturn  warnings. This patch keeps the old behaviour of always warning on
every noreturn violation, but unlike clang also adds an extra layer of fine
tuning by turning invalid-noreturn into a warning with levels, where level
1 warns about noreturn functions that do return, level 2 warns about
noreturn functions that explicitly have return statements, and level 3,
which is the default to match old behaviour, warns for both instances.

gcc/doc/ChangeLog:

* invoke.texi (-Wno-invalid-noreturn, -Winvalid-noreturn=): Document new
options.

gcc/ChangeLog:

* tree-cfg.cc (pass_warn_function_return::execute): Use new warning option.

gcc/c-family/ChangeLog:

* c.opt (Winvalid-noreturn, Winvalid-noreturn=): New options.

gcc/c/ChangeLog:

* c-typeck.cc (c_finish_return): Use new warning option.
* gimple-parser.cc (c_finish_gimple_return): Likewise.

gcc/cp/ChangeLog:

* coroutines.cc (finish_co_return_stmt): Use new warning option.
* typeck.cc (check_return_expr): Likewise.

 gcc/c-family/c.opt     |  8 ++++++++
 gcc/c/c-typeck.cc      |  9 ++++++---
 gcc/c/gimple-parser.cc |  9 ++++++---
 gcc/cp/coroutines.cc   | 11 +++++++----
 gcc/cp/typeck.cc       |  7 +++++--
 gcc/doc/invoke.texi    | 26 ++++++++++++++++++++++++++
 gcc/tree-cfg.cc        |  5 ++++-
 7 files changed, 62 insertions(+), 13 deletions(-)

[-- Attachment #2: 0001-Add-the-invalid-noreturn-warning-to-match-clang.patch --]
[-- Type: application/octet-stream, Size: 7000 bytes --]

From b1265ab3450f5f9e12ecbf5f5c2125140fa4d863 Mon Sep 17 00:00:00 2001
From: TheShermanTanker <tanksherman27@gmail.com>
Date: Sun, 13 Aug 2023 16:43:03 +0800
Subject: [PATCH] Add the invalid-noreturn warning to match clang

---
 gcc/c-family/c.opt     |  8 ++++++++
 gcc/c/c-typeck.cc      |  9 ++++++---
 gcc/c/gimple-parser.cc |  9 ++++++---
 gcc/cp/coroutines.cc   | 11 +++++++----
 gcc/cp/typeck.cc       |  7 +++++--
 gcc/doc/invoke.texi    | 26 ++++++++++++++++++++++++++
 gcc/tree-cfg.cc        |  5 ++++-
 7 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 0ed87fcc7be..ccb1e2a0cb3 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -478,6 +478,14 @@ Wc++23-extensions
 C++ ObjC++ Var(warn_cxx23_extensions) Warning Init(1)
 Warn about C++23 constructs in code compiled with an older standard.
 
+Winvalid-noreturn
+C ObjC C++ ObjC++ Warning Alias(Winvalid-noreturn=, 3, 0)
+Warn about code marked noreturn that actually returns.
+
+Winvalid-noreturn=
+C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_invalid_noreturn) Warning Init(3) IntegerRange(0, 3)
+Warn about code marked noreturn that actually returns.
+
 Wcast-function-type
 C ObjC C++ ObjC++ Var(warn_cast_function_type) Warning EnabledBy(Wextra)
 Warn about casts between incompatible function types.
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 7cf411155c6..9d5a8b21d78 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -11247,9 +11247,12 @@ c_finish_return (location_t loc, tree retval, tree origtype)
      in a function returning void.  */
   location_t xloc = expansion_point_location_if_in_system_header (loc);
 
-  if (TREE_THIS_VOLATILE (current_function_decl))
-    warning_at (xloc, 0,
-		"function declared %<noreturn%> has a %<return%> statement");
+  if (TREE_THIS_VOLATILE (current_function_decl)) {
+    if (warn_invalid_noreturn == 2 || warn_invalid_noreturn == 3) {
+      warning_at (xloc, OPT_Winvalid_noreturn_,
+		  "function declared %<noreturn%> has a %<return%> statement");
+    }
+  }
 
   if (retval)
     {
diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
index cc3a8899d97..cfb7eed8277 100644
--- a/gcc/c/gimple-parser.cc
+++ b/gcc/c/gimple-parser.cc
@@ -2466,9 +2466,12 @@ c_finish_gimple_return (location_t loc, tree retval)
      in a function returning void.  */
   location_t xloc = expansion_point_location_if_in_system_header (loc);
 
-  if (TREE_THIS_VOLATILE (current_function_decl))
-    warning_at (xloc, 0,
-		"function declared %<noreturn%> has a %<return%> statement");
+  if (TREE_THIS_VOLATILE (current_function_decl)) {
+    if (warn_invalid_noreturn == 2 || warn_invalid_noreturn == 3) {
+      warning_at (xloc, OPT_Winvalid_noreturn_,
+		  "function declared %<noreturn%> has a %<return%> statement");
+    }
+  }
 
   if (! retval)
     current_function_returns_null = 1;
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 3493d3c6ed3..24fa7c5341b 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1386,10 +1386,13 @@ finish_co_return_stmt (location_t kw, tree expr)
     }
 
   /* Makes no sense for a co-routine really. */
-  if (TREE_THIS_VOLATILE (current_function_decl))
-    warning_at (kw, 0,
-		"function declared %<noreturn%> has a"
-		" %<co_return%> statement");
+  if (TREE_THIS_VOLATILE (current_function_decl)) {
+    if (warn_invalid_noreturn == 2 || warn_invalid_noreturn == 3) {
+      warning_at (kw, OPT_Winvalid_noreturn_,
+		  "function declared %<noreturn%> has a"
+		  " %<co_return%> statement");
+    }
+  }
 
   expr = build2_loc (kw, CO_RETURN_EXPR, void_type_node, expr, co_ret_call);
   expr = maybe_cleanup_point_expr_void (expr);
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index d5c0c85ed51..2ad07c40c70 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10941,8 +10941,11 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
   /* A `volatile' function is one that isn't supposed to return, ever.
      (This is a G++ extension, used to get better code for functions
      that call the `volatile' function.)  */
-  if (TREE_THIS_VOLATILE (current_function_decl))
-    warning (0, "function declared %<noreturn%> has a %<return%> statement");
+  if (TREE_THIS_VOLATILE (current_function_decl)) {
+    if (warn_invalid_noreturn == 2 || warn_invalid_noreturn == 3) {
+      warning (OPT_Winvalid_noreturn_, "function declared %<noreturn%> has a %<return%> statement");
+    }
+  }
 
   /* Check for various simple errors.  */
   if (DECL_DESTRUCTOR_P (current_function_decl))
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 104766f446d..ad4ad663786 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -342,6 +342,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wc++20-compat
 -Wno-c++11-extensions  -Wno-c++14-extensions -Wno-c++17-extensions
 -Wno-c++20-extensions  -Wno-c++23-extensions
+-Wno-invalid-noreturn  -Winvalid-noreturn=@var{n}
 -Wcast-align  -Wcast-align=strict  -Wcast-function-type  -Wcast-qual
 -Wchar-subscripts
 -Wclobbered  -Wcomment
@@ -9008,6 +9009,31 @@ Do not warn about C++23 constructs in code being compiled using
 an older C++ standard.  Even without this option, some C++23 constructs
 will only be diagnosed if @option{-Wpedantic} is used.
 
+@opindex Winvalid-noreturn
+@opindex Wno-invalid-noreturn
+@item -Wno-invalid-noreturn
+@itemx -Winvalid-noreturn=@var{n}
+When off, do not warn about code marked noreturn that returns in its
+body, whether it does so explicitly or implicitly.  Different levels
+warn for different cases.  By default, this warning is enabled and
+set to level 3.
+
+@table @gcctabopt
+@item -Winvalid-noreturn=1
+Warn only for cases where code that is marked noreturn implicitly
+returns to its caller without an explicit return statement, that is
+to say it has a branch which does not terminate the program, enter
+an infinite loop, or throw an exception.
+
+@item -Winvalid-noreturn=2
+Warn only for cases where code that is marked noreturn has an explicit
+return statement in its body.
+
+@item -Winvalid-noreturn=3
+Warn in both cases when code marked noreturn implicitly returns to its
+caller, or when code marked noreturn contains an explicit return
+statement in its body.  This is the default.
+
 @opindex Wcast-qual
 @opindex Wno-cast-qual
 @item -Wcast-qual
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index c65af8cc800..1ce4acbf031 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -9665,7 +9665,10 @@ pass_warn_function_return::execute (function *fun)
 	}
       if (location == UNKNOWN_LOCATION)
 	location = cfun->function_end_locus;
-      warning_at (location, 0, "%<noreturn%> function does return");
+
+      if (warn_invalid_noreturn == 1 || warn_invalid_noreturn == 3) {
+        warning_at (location, OPT_Winvalid_noreturn_, "%<noreturn%> function does return");
+      }
     }
 
   /* If we see "return;" in some basic block, then we do reach the end
-- 
2.35.1.windows.2

Re: [PATCH] Add clang's invalid-noreturn warning flag

[Julian Waters]

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

Anyone?

On Sun, Aug 13, 2023 at 4:48 PM Julian Waters <tanksherman27@gmail.com>
wrote:

> Please review a patch to add clang's invalid-noreturn flag to toggle
> noreturn  warnings. This patch keeps the old behaviour of always warning on
> every noreturn violation, but unlike clang also adds an extra layer of fine
> tuning by turning invalid-noreturn into a warning with levels, where level
> 1 warns about noreturn functions that do return, level 2 warns about
> noreturn functions that explicitly have return statements, and level 3,
> which is the default to match old behaviour, warns for both instances.
>
> gcc/doc/ChangeLog:
>
> * invoke.texi (-Wno-invalid-noreturn, -Winvalid-noreturn=): Document new
> options.
>
> gcc/ChangeLog:
>
> * tree-cfg.cc (pass_warn_function_return::execute)
> : Use new warning option.
>
> gcc/c-family/ChangeLog:
>
> * c.opt (Winvalid-noreturn, Winvalid-noreturn=): New options.
>
> gcc/c/ChangeLog:
>
> * c-typeck.cc (c_finish_return): Use new warning option.
> * gimple-parser.cc (c_finish_gimple_return): Likewise.
>
> gcc/cp/ChangeLog:
>
> * coroutines.cc (finish_co_return_stmt): Use new warning option.
> * typeck.cc (check_return_expr): Likewise.
>
>  gcc/c-family/c.opt     |  8 ++++++++
>  gcc/c/c-typeck.cc      |  9 ++++++---
>  gcc/c/gimple-parser.cc |  9 ++++++---
>  gcc/cp/coroutines.cc   | 11 +++++++----
>  gcc/cp/typeck.cc       |  7 +++++--
>  gcc/doc/invoke.texi    | 26 ++++++++++++++++++++++++++
>  gcc/tree-cfg.cc        |  5 ++++-
>  7 files changed, 62 insertions(+), 13 deletions(-)
>
>
>

Re: [PATCH] Add clang's invalid-noreturn warning flag

[Sam James]

Julian Waters via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Anyone?

Please see https://gcc.gnu.org/contribute.html#patches, specifically
the "Pinging patches, Getting patches applied" section.