[PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability

[PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability

[Julian Waters]

Currently, gcc warns about noreturn marked functions that return both explicitly and implicitly, with no way to turn this warning off. clang does have an option for these classes of warnings, -Winvalid-noreturn. However, we can do better. Instead of just having 1 option that switches the warnings for both on and off, we can define an extra layer of granularity, and have a separate options for implicit returns and explicit returns, as in -Winvalid-return=explicit and -Winvalid-noreturn=implicit. This patch adds both to gcc, for compatibility with clang. Do note that I am relatively new to gcc's codebase, and as such couldn't figure out how to cleanly define a general -Winvalid-noreturn warning that switch both on and off, for better compatibility with clang. If someone should point out how to do so, I'll happily rewrite my patch. I also do not have write access to gcc, and will need help pushing this patch once the green light is given

best regards,
Julian

gcc/c-family/ChangeLog:

	* c.opt: Introduce -Winvalid-noreturn=explicit and -Winvalid-noreturn=implicit

gcc/ChangeLog:

	* tree-cfg.cc (pass_warn_function_return::execute): Use it

gcc/c/ChangeLog:

	* c-typeck.cc (c_finish_return): Use it
	* gimple-parser.cc (c_finish_gimple_return): Use it

gcc/config/mingw/ChangeLog:

	* mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons

gcc/cp/ChangeLog:

	* coroutines.cc (finish_co_return_stmt): Use it
	* typeck.cc (check_return_expr): Use it

gcc/doc/ChangeLog:

	* invoke.texi: Document new options

From 4daf884f8bbc1e318ba93121a6fdf4139da80b64 Mon Sep 17 00:00:00 2001
From: TheShermanTanker <tanksherman27@gmail.com>
Date: Wed, 29 May 2024 21:32:08 +0800
Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with extra
 tuneability

Signed-off-by: TheShermanTanker <tanksherman27@gmail.com>
---
 gcc/c-family/c.opt         |  8 ++++++++
 gcc/c/c-typeck.cc          |  2 +-
 gcc/c/gimple-parser.cc     |  2 +-
 gcc/config/mingw/mingw32.h |  6 +++---
 gcc/cp/coroutines.cc       |  2 +-
 gcc/cp/typeck.cc           |  2 +-
 gcc/doc/invoke.texi        | 13 +++++++++++++
 gcc/tree-cfg.cc            |  2 +-
 8 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index fb34c3b7031..32a2859fdcc 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -886,6 +886,14 @@ Winvalid-constexpr
 C++ ObjC++ Var(warn_invalid_constexpr) Init(-1) Warning
 Warn when a function never produces a constant expression.
 
+Winvalid-noreturn=explicit
+C ObjC C++ ObjC++ Warning
+Warn when a function marked noreturn returns explicitly.
+
+Winvalid-noreturn=implicit
+C ObjC C++ ObjC++ Warning
+Warn when a function marked noreturn returns implicitly.
+
 Winvalid-offsetof
 C++ ObjC++ Var(warn_invalid_offsetof) Init(1) Warning
 Warn about invalid uses of the \"offsetof\" macro.
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index ad4c7add562..1941fbc44cb 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -11468,7 +11468,7 @@ c_finish_return (location_t loc, tree retval, tree origtype)
   location_t xloc = expansion_point_location_if_in_system_header (loc);
 
   if (TREE_THIS_VOLATILE (current_function_decl))
-    warning_at (xloc, 0,
+    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
 		"function declared %<noreturn%> has a %<return%> statement");
 
   if (retval)
diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
index d156d83cd37..1acaf75f844 100644
--- a/gcc/c/gimple-parser.cc
+++ b/gcc/c/gimple-parser.cc
@@ -2593,7 +2593,7 @@ c_finish_gimple_return (location_t loc, tree retval)
   location_t xloc = expansion_point_location_if_in_system_header (loc);
 
   if (TREE_THIS_VOLATILE (current_function_decl))
-    warning_at (xloc, 0,
+    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
 		"function declared %<noreturn%> has a %<return%> statement");
 
   if (! retval)
diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h
index fa6e307476c..a69926133b1 100644
--- a/gcc/config/mingw/mingw32.h
+++ b/gcc/config/mingw/mingw32.h
@@ -35,9 +35,9 @@ along with GCC; see the file COPYING3.  If not see
 	 | MASK_MS_BITFIELD_LAYOUT)
 
 #ifdef TARGET_USING_MCFGTHREAD
-#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__");
+#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__")
 #elif defined(TARGET_USE_PTHREAD_BY_DEFAULT)
-#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__");
+#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__")
 #else
 #define DEFINE_THREAD_MODEL
 #endif
@@ -60,7 +60,7 @@ along with GCC; see the file COPYING3.  If not see
 	  builtin_define_std ("WIN64");				\
 	  builtin_define ("_WIN64");				\
 	}							\
-      DEFINE_THREAD_MODEL					\
+      DEFINE_THREAD_MODEL;					\
     }								\
   while (0)
 
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 97bc211ff67..53e56bd4959 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1375,7 +1375,7 @@ 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,
+    warning_at (kw, OPT_Winvalid_noreturn_explicit,
 		"function declared %<noreturn%> has a"
 		" %<co_return%> statement");
 
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 1b7a31d32f3..74cc59bfb87 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -11062,7 +11062,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
      (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");
+    warning (OPT_Winvalid_noreturn_explicit, "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 2cba380718b..27d880fd4f0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -376,6 +376,7 @@ Objective-C and Objective-C++ Dialects}.
 -Winfinite-recursion
 -Winit-self  -Winline  -Wno-int-conversion  -Wint-in-bool-context
 -Wno-int-to-pointer-cast  -Wno-invalid-memory-model
+-Winvalid-noreturn=explicit  -Winvalid-noreturn=implicit
 -Winvalid-pch  -Winvalid-utf8  -Wno-unicode  -Wjump-misses-init
 -Wlarger-than=@var{byte-size}  -Wlogical-not-parentheses  -Wlogical-op
 -Wlong-long  -Wno-lto-type-mismatch -Wmain  -Wmaybe-uninitialized
@@ -10482,6 +10483,18 @@ an error. @option{Wint-to-pointer-cast} is enabled by default.
 Suppress warnings from casts from a pointer to an integer type of a
 different size.
 
+@opindex Winvalid-noreturn=explicit
+@opindex Wno-invalid-noreturn=explicit
+@item -Winvalid-noreturn=explicit
+Warn if a function marked noreturn returns explicitly, that is, it
+contains a return statement.
+
+@opindex Winvalid-noreturn=implicit
+@opindex Wno-invalid-noreturn=implicit
+@item -Winvalid-noreturn=implicit
+Warn if a function marked noreturn returns implicitly, that is, it
+has a path in its control flow that can reach the end of its body.
+
 @opindex Winvalid-pch
 @opindex Wno-invalid-pch
 @item -Winvalid-pch
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 7fb7b92966b..cfe91038dd1 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -9817,7 +9817,7 @@ pass_warn_function_return::execute (function *fun)
 	}
       if (location == UNKNOWN_LOCATION)
 	location = cfun->function_end_locus;
-      warning_at (location, 0, "%<noreturn%> function does return");
+      warning_at (location, OPT_Winvalid_noreturn_implicit, "%<noreturn%> function does return");
     }
 
   /* If we see "return;" in some basic block, then we do reach the end
-- 
2.41.0

Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability

[Jason Merrill]

On 5/29/24 09:58, Julian Waters wrote:
> Currently, gcc warns about noreturn marked functions that return both explicitly and implicitly, with no way to turn this warning off. clang does have an option for these classes of warnings, -Winvalid-noreturn. However, we can do better. Instead of just having 1 option that switches the warnings for both on and off, we can define an extra layer of granularity, and have a separate options for implicit returns and explicit returns, as in -Winvalid-return=explicit and -Winvalid-noreturn=implicit. This patch adds both to gcc, for compatibility with clang.

Thanks!

> Do note that I am relatively new to gcc's codebase, and as such couldn't figure out how to cleanly define a general -Winvalid-noreturn warning that switch both on and off, for better compatibility with clang. If someone should point out how to do so, I'll happily rewrite my patch.

See -fstrong-eval-order for an example of an option that can be used 
with or without =arg.

> I also do not have write access to gcc, and will need help pushing this patch once the green light is given

Good to know, I can take care of that.

> best regards,
> Julian
> 
> gcc/c-family/ChangeLog:
> 
> 	* c.opt: Introduce -Winvalid-noreturn=explicit and -Winvalid-noreturn=implicit
> 
> gcc/ChangeLog:
> 
> 	* tree-cfg.cc (pass_warn_function_return::execute): Use it
> 
> gcc/c/ChangeLog:
> 
> 	* c-typeck.cc (c_finish_return): Use it
> 	* gimple-parser.cc (c_finish_gimple_return): Use it
> 
> gcc/config/mingw/ChangeLog:
> 
> 	* mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons
> 
> gcc/cp/ChangeLog:
> 
> 	* coroutines.cc (finish_co_return_stmt): Use it
> 	* typeck.cc (check_return_expr): Use it
> 
> gcc/doc/ChangeLog:
> 
> 	* invoke.texi: Document new options
> 
>  From 4daf884f8bbc1e318ba93121a6fdf4139da80b64 Mon Sep 17 00:00:00 2001
> From: TheShermanTanker <tanksherman27@gmail.com>
> Date: Wed, 29 May 2024 21:32:08 +0800
> Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with extra
>   tuneability

The rationale and ChangeLog entries should be part of the commit message 
(and so the git format-patch output).

> 
> Signed-off-by: TheShermanTanker <tanksherman27@gmail.com>

A DCO sign-off can't use a pseudonym, sorry; please either sign off 
using your real name or file a copyright assignment for the pseudonym 
with the FSF.

See https://gcc.gnu.org/contribute.html#legal for more detail.

> ---
>   gcc/c-family/c.opt         |  8 ++++++++
>   gcc/c/c-typeck.cc          |  2 +-
>   gcc/c/gimple-parser.cc     |  2 +-
>   gcc/config/mingw/mingw32.h |  6 +++---
>   gcc/cp/coroutines.cc       |  2 +-
>   gcc/cp/typeck.cc           |  2 +-
>   gcc/doc/invoke.texi        | 13 +++++++++++++
>   gcc/tree-cfg.cc            |  2 +-
>   8 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index fb34c3b7031..32a2859fdcc 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -886,6 +886,14 @@ Winvalid-constexpr
>   C++ ObjC++ Var(warn_invalid_constexpr) Init(-1) Warning
>   Warn when a function never produces a constant expression.
>   
> +Winvalid-noreturn=explicit
> +C ObjC C++ ObjC++ Warning
> +Warn when a function marked noreturn returns explicitly.
> +
> +Winvalid-noreturn=implicit
> +C ObjC C++ ObjC++ Warning
> +Warn when a function marked noreturn returns implicitly.
> +
>   Winvalid-offsetof
>   C++ ObjC++ Var(warn_invalid_offsetof) Init(1) Warning
>   Warn about invalid uses of the \"offsetof\" macro.
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index ad4c7add562..1941fbc44cb 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -11468,7 +11468,7 @@ c_finish_return (location_t loc, tree retval, tree origtype)
>     location_t xloc = expansion_point_location_if_in_system_header (loc);
>   
>     if (TREE_THIS_VOLATILE (current_function_decl))
> -    warning_at (xloc, 0,
> +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
>   		"function declared %<noreturn%> has a %<return%> statement");
>   
>     if (retval)
> diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
> index d156d83cd37..1acaf75f844 100644
> --- a/gcc/c/gimple-parser.cc
> +++ b/gcc/c/gimple-parser.cc
> @@ -2593,7 +2593,7 @@ c_finish_gimple_return (location_t loc, tree retval)
>     location_t xloc = expansion_point_location_if_in_system_header (loc);
>   
>     if (TREE_THIS_VOLATILE (current_function_decl))
> -    warning_at (xloc, 0,
> +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
>   		"function declared %<noreturn%> has a %<return%> statement");
>   
>     if (! retval)
> diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h
> index fa6e307476c..a69926133b1 100644
> --- a/gcc/config/mingw/mingw32.h
> +++ b/gcc/config/mingw/mingw32.h
> @@ -35,9 +35,9 @@ along with GCC; see the file COPYING3.  If not see
>   	 | MASK_MS_BITFIELD_LAYOUT)
>   
>   #ifdef TARGET_USING_MCFGTHREAD
> -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__");
> +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__")
>   #elif defined(TARGET_USE_PTHREAD_BY_DEFAULT)
> -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__");
> +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__")
>   #else
>   #define DEFINE_THREAD_MODEL
>   #endif
> @@ -60,7 +60,7 @@ along with GCC; see the file COPYING3.  If not see
>   	  builtin_define_std ("WIN64");				\
>   	  builtin_define ("_WIN64");				\
>   	}							\
> -      DEFINE_THREAD_MODEL					\
> +      DEFINE_THREAD_MODEL;					\
>       }								\
>     while (0)
>   
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 97bc211ff67..53e56bd4959 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -1375,7 +1375,7 @@ 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,
> +    warning_at (kw, OPT_Winvalid_noreturn_explicit,
>   		"function declared %<noreturn%> has a"
>   		" %<co_return%> statement");
>   
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 1b7a31d32f3..74cc59bfb87 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -11062,7 +11062,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>        (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");
> +    warning (OPT_Winvalid_noreturn_explicit, "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 2cba380718b..27d880fd4f0 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -376,6 +376,7 @@ Objective-C and Objective-C++ Dialects}.
>   -Winfinite-recursion
>   -Winit-self  -Winline  -Wno-int-conversion  -Wint-in-bool-context
>   -Wno-int-to-pointer-cast  -Wno-invalid-memory-model
> +-Winvalid-noreturn=explicit  -Winvalid-noreturn=implicit
>   -Winvalid-pch  -Winvalid-utf8  -Wno-unicode  -Wjump-misses-init
>   -Wlarger-than=@var{byte-size}  -Wlogical-not-parentheses  -Wlogical-op
>   -Wlong-long  -Wno-lto-type-mismatch -Wmain  -Wmaybe-uninitialized
> @@ -10482,6 +10483,18 @@ an error. @option{Wint-to-pointer-cast} is enabled by default.
>   Suppress warnings from casts from a pointer to an integer type of a
>   different size.
>   
> +@opindex Winvalid-noreturn=explicit
> +@opindex Wno-invalid-noreturn=explicit
> +@item -Winvalid-noreturn=explicit
> +Warn if a function marked noreturn returns explicitly, that is, it
> +contains a return statement.
> +
> +@opindex Winvalid-noreturn=implicit
> +@opindex Wno-invalid-noreturn=implicit
> +@item -Winvalid-noreturn=implicit
> +Warn if a function marked noreturn returns implicitly, that is, it
> +has a path in its control flow that can reach the end of its body.
> +
>   @opindex Winvalid-pch
>   @opindex Wno-invalid-pch
>   @item -Winvalid-pch
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> index 7fb7b92966b..cfe91038dd1 100644
> --- a/gcc/tree-cfg.cc
> +++ b/gcc/tree-cfg.cc
> @@ -9817,7 +9817,7 @@ pass_warn_function_return::execute (function *fun)
>   	}
>         if (location == UNKNOWN_LOCATION)
>   	location = cfun->function_end_locus;
> -      warning_at (location, 0, "%<noreturn%> function does return");
> +      warning_at (location, OPT_Winvalid_noreturn_implicit, "%<noreturn%> function does return");
>       }
>   
>     /* If we see "return;" in some basic block, then we do reach the end

Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability

[Julian Waters]

Hi Jason,

Thanks for the reply! I'll address your comments soon. I have a
question, if there is an option defined in c.opt as an Enum, like
fstrong-eval-order, and the -no variant of the option is passed, would
the Var somehow reflect the negated option? Eg

Winvalid-noreturn=
C ObjC C++ ObjC++ Var(warn_invalid_noreturn) Joined
Enum(invalid_noreturn) Warning

Enum
Name(invalid_noreturn) Type(int)

EnumValue
Enum(invalid_noreturn) String(explicit) Value(0)

Would warn_invalid_noreturn then != 0 if
-Wno-invalid-noreturn=explicit is passed? Or is there a way to make a
warning call depend on 2 different OPT_ entries?

best regards,
Julian

On Sat, Jun 1, 2024 at 4:57 AM Jason Merrill <jason@redhat.com> wrote:
>
> On 5/29/24 09:58, Julian Waters wrote:
> > Currently, gcc warns about noreturn marked functions that return both explicitly and implicitly, with no way to turn this warning off. clang does have an option for these classes of warnings, -Winvalid-noreturn. However, we can do better. Instead of just having 1 option that switches the warnings for both on and off, we can define an extra layer of granularity, and have a separate options for implicit returns and explicit returns, as in -Winvalid-return=explicit and -Winvalid-noreturn=implicit. This patch adds both to gcc, for compatibility with clang.
>
> Thanks!
>
> > Do note that I am relatively new to gcc's codebase, and as such couldn't figure out how to cleanly define a general -Winvalid-noreturn warning that switch both on and off, for better compatibility with clang. If someone should point out how to do so, I'll happily rewrite my patch.
>
> See -fstrong-eval-order for an example of an option that can be used
> with or without =arg.
>
> > I also do not have write access to gcc, and will need help pushing this patch once the green light is given
>
> Good to know, I can take care of that.
>
> > best regards,
> > Julian
> >
> > gcc/c-family/ChangeLog:
> >
> >       * c.opt: Introduce -Winvalid-noreturn=explicit and -Winvalid-noreturn=implicit
> >
> > gcc/ChangeLog:
> >
> >       * tree-cfg.cc (pass_warn_function_return::execute): Use it
> >
> > gcc/c/ChangeLog:
> >
> >       * c-typeck.cc (c_finish_return): Use it
> >       * gimple-parser.cc (c_finish_gimple_return): Use it
> >
> > gcc/config/mingw/ChangeLog:
> >
> >       * mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons
> >
> > gcc/cp/ChangeLog:
> >
> >       * coroutines.cc (finish_co_return_stmt): Use it
> >       * typeck.cc (check_return_expr): Use it
> >
> > gcc/doc/ChangeLog:
> >
> >       * invoke.texi: Document new options
> >
> >  From 4daf884f8bbc1e318ba93121a6fdf4139da80b64 Mon Sep 17 00:00:00 2001
> > From: TheShermanTanker <tanksherman27@gmail.com>
> > Date: Wed, 29 May 2024 21:32:08 +0800
> > Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with extra
> >   tuneability
>
> The rationale and ChangeLog entries should be part of the commit message
> (and so the git format-patch output).
>
> >
> > Signed-off-by: TheShermanTanker <tanksherman27@gmail.com>
>
> A DCO sign-off can't use a pseudonym, sorry; please either sign off
> using your real name or file a copyright assignment for the pseudonym
> with the FSF.
>
> See https://gcc.gnu.org/contribute.html#legal for more detail.
>
> > ---
> >   gcc/c-family/c.opt         |  8 ++++++++
> >   gcc/c/c-typeck.cc          |  2 +-
> >   gcc/c/gimple-parser.cc     |  2 +-
> >   gcc/config/mingw/mingw32.h |  6 +++---
> >   gcc/cp/coroutines.cc       |  2 +-
> >   gcc/cp/typeck.cc           |  2 +-
> >   gcc/doc/invoke.texi        | 13 +++++++++++++
> >   gcc/tree-cfg.cc            |  2 +-
> >   8 files changed, 29 insertions(+), 8 deletions(-)
> >
> > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> > index fb34c3b7031..32a2859fdcc 100644
> > --- a/gcc/c-family/c.opt
> > +++ b/gcc/c-family/c.opt
> > @@ -886,6 +886,14 @@ Winvalid-constexpr
> >   C++ ObjC++ Var(warn_invalid_constexpr) Init(-1) Warning
> >   Warn when a function never produces a constant expression.
> >
> > +Winvalid-noreturn=explicit
> > +C ObjC C++ ObjC++ Warning
> > +Warn when a function marked noreturn returns explicitly.
> > +
> > +Winvalid-noreturn=implicit
> > +C ObjC C++ ObjC++ Warning
> > +Warn when a function marked noreturn returns implicitly.
> > +
> >   Winvalid-offsetof
> >   C++ ObjC++ Var(warn_invalid_offsetof) Init(1) Warning
> >   Warn about invalid uses of the \"offsetof\" macro.
> > diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> > index ad4c7add562..1941fbc44cb 100644
> > --- a/gcc/c/c-typeck.cc
> > +++ b/gcc/c/c-typeck.cc
> > @@ -11468,7 +11468,7 @@ c_finish_return (location_t loc, tree retval, tree origtype)
> >     location_t xloc = expansion_point_location_if_in_system_header (loc);
> >
> >     if (TREE_THIS_VOLATILE (current_function_decl))
> > -    warning_at (xloc, 0,
> > +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
> >               "function declared %<noreturn%> has a %<return%> statement");
> >
> >     if (retval)
> > diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
> > index d156d83cd37..1acaf75f844 100644
> > --- a/gcc/c/gimple-parser.cc
> > +++ b/gcc/c/gimple-parser.cc
> > @@ -2593,7 +2593,7 @@ c_finish_gimple_return (location_t loc, tree retval)
> >     location_t xloc = expansion_point_location_if_in_system_header (loc);
> >
> >     if (TREE_THIS_VOLATILE (current_function_decl))
> > -    warning_at (xloc, 0,
> > +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
> >               "function declared %<noreturn%> has a %<return%> statement");
> >
> >     if (! retval)
> > diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h
> > index fa6e307476c..a69926133b1 100644
> > --- a/gcc/config/mingw/mingw32.h
> > +++ b/gcc/config/mingw/mingw32.h
> > @@ -35,9 +35,9 @@ along with GCC; see the file COPYING3.  If not see
> >        | MASK_MS_BITFIELD_LAYOUT)
> >
> >   #ifdef TARGET_USING_MCFGTHREAD
> > -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__");
> > +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__")
> >   #elif defined(TARGET_USE_PTHREAD_BY_DEFAULT)
> > -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__");
> > +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__")
> >   #else
> >   #define DEFINE_THREAD_MODEL
> >   #endif
> > @@ -60,7 +60,7 @@ along with GCC; see the file COPYING3.  If not see
> >         builtin_define_std ("WIN64");                         \
> >         builtin_define ("_WIN64");                            \
> >       }                                                       \
> > -      DEFINE_THREAD_MODEL                                    \
> > +      DEFINE_THREAD_MODEL;                                   \
> >       }                                                               \
> >     while (0)
> >
> > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> > index 97bc211ff67..53e56bd4959 100644
> > --- a/gcc/cp/coroutines.cc
> > +++ b/gcc/cp/coroutines.cc
> > @@ -1375,7 +1375,7 @@ 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,
> > +    warning_at (kw, OPT_Winvalid_noreturn_explicit,
> >               "function declared %<noreturn%> has a"
> >               " %<co_return%> statement");
> >
> > diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> > index 1b7a31d32f3..74cc59bfb87 100644
> > --- a/gcc/cp/typeck.cc
> > +++ b/gcc/cp/typeck.cc
> > @@ -11062,7 +11062,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
> >        (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");
> > +    warning (OPT_Winvalid_noreturn_explicit, "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 2cba380718b..27d880fd4f0 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -376,6 +376,7 @@ Objective-C and Objective-C++ Dialects}.
> >   -Winfinite-recursion
> >   -Winit-self  -Winline  -Wno-int-conversion  -Wint-in-bool-context
> >   -Wno-int-to-pointer-cast  -Wno-invalid-memory-model
> > +-Winvalid-noreturn=explicit  -Winvalid-noreturn=implicit
> >   -Winvalid-pch  -Winvalid-utf8  -Wno-unicode  -Wjump-misses-init
> >   -Wlarger-than=@var{byte-size}  -Wlogical-not-parentheses  -Wlogical-op
> >   -Wlong-long  -Wno-lto-type-mismatch -Wmain  -Wmaybe-uninitialized
> > @@ -10482,6 +10483,18 @@ an error. @option{Wint-to-pointer-cast} is enabled by default.
> >   Suppress warnings from casts from a pointer to an integer type of a
> >   different size.
> >
> > +@opindex Winvalid-noreturn=explicit
> > +@opindex Wno-invalid-noreturn=explicit
> > +@item -Winvalid-noreturn=explicit
> > +Warn if a function marked noreturn returns explicitly, that is, it
> > +contains a return statement.
> > +
> > +@opindex Winvalid-noreturn=implicit
> > +@opindex Wno-invalid-noreturn=implicit
> > +@item -Winvalid-noreturn=implicit
> > +Warn if a function marked noreturn returns implicitly, that is, it
> > +has a path in its control flow that can reach the end of its body.
> > +
> >   @opindex Winvalid-pch
> >   @opindex Wno-invalid-pch
> >   @item -Winvalid-pch
> > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > index 7fb7b92966b..cfe91038dd1 100644
> > --- a/gcc/tree-cfg.cc
> > +++ b/gcc/tree-cfg.cc
> > @@ -9817,7 +9817,7 @@ pass_warn_function_return::execute (function *fun)
> >       }
> >         if (location == UNKNOWN_LOCATION)
> >       location = cfun->function_end_locus;
> > -      warning_at (location, 0, "%<noreturn%> function does return");
> > +      warning_at (location, OPT_Winvalid_noreturn_implicit, "%<noreturn%> function does return");
> >       }
> >
> >     /* If we see "return;" in some basic block, then we do reach the end
>

Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability

[Eric Gallager]

On Sat, Jun 1, 2024 at 11:32 AM Julian Waters <tanksherman27@gmail.com> wrote:
>
> Hi Jason,
>
> Thanks for the reply! I'll address your comments soon. I have a
> question, if there is an option defined in c.opt as an Enum, like
> fstrong-eval-order, and the -no variant of the option is passed, would
> the Var somehow reflect the negated option? Eg
>
> Winvalid-noreturn=
> C ObjC C++ ObjC++ Var(warn_invalid_noreturn) Joined
> Enum(invalid_noreturn) Warning
>
> Enum
> Name(invalid_noreturn) Type(int)
>
> EnumValue
> Enum(invalid_noreturn) String(explicit) Value(0)
>
> Would warn_invalid_noreturn then != 0 if
> -Wno-invalid-noreturn=explicit is passed? Or is there a way to make a
> warning call depend on 2 different OPT_ entries?
>
> best regards,
> Julian

This kind of issue is one of the reasons why I think it's better to
just avoid the "=" character in warning option names, and to just make
completely separate options using the "-" character instead... (i.e.,
-Winvalid-noreturn-implicit and -Winvalid-noreturn-explicit)

>
> On Sat, Jun 1, 2024 at 4:57 AM Jason Merrill <jason@redhat.com> wrote:
> >
> > On 5/29/24 09:58, Julian Waters wrote:
> > > Currently, gcc warns about noreturn marked functions that return both explicitly and implicitly, with no way to turn this warning off. clang does have an option for these classes of warnings, -Winvalid-noreturn. However, we can do better. Instead of just having 1 option that switches the warnings for both on and off, we can define an extra layer of granularity, and have a separate options for implicit returns and explicit returns, as in -Winvalid-return=explicit and -Winvalid-noreturn=implicit. This patch adds both to gcc, for compatibility with clang.
> >
> > Thanks!
> >
> > > Do note that I am relatively new to gcc's codebase, and as such couldn't figure out how to cleanly define a general -Winvalid-noreturn warning that switch both on and off, for better compatibility with clang. If someone should point out how to do so, I'll happily rewrite my patch.
> >
> > See -fstrong-eval-order for an example of an option that can be used
> > with or without =arg.
> >
> > > I also do not have write access to gcc, and will need help pushing this patch once the green light is given
> >
> > Good to know, I can take care of that.
> >
> > > best regards,
> > > Julian
> > >
> > > gcc/c-family/ChangeLog:
> > >
> > >       * c.opt: Introduce -Winvalid-noreturn=explicit and -Winvalid-noreturn=implicit
> > >
> > > gcc/ChangeLog:
> > >
> > >       * tree-cfg.cc (pass_warn_function_return::execute): Use it
> > >
> > > gcc/c/ChangeLog:
> > >
> > >       * c-typeck.cc (c_finish_return): Use it
> > >       * gimple-parser.cc (c_finish_gimple_return): Use it
> > >
> > > gcc/config/mingw/ChangeLog:
> > >
> > >       * mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons
> > >
> > > gcc/cp/ChangeLog:
> > >
> > >       * coroutines.cc (finish_co_return_stmt): Use it
> > >       * typeck.cc (check_return_expr): Use it
> > >
> > > gcc/doc/ChangeLog:
> > >
> > >       * invoke.texi: Document new options
> > >
> > >  From 4daf884f8bbc1e318ba93121a6fdf4139da80b64 Mon Sep 17 00:00:00 2001
> > > From: TheShermanTanker <tanksherman27@gmail.com>
> > > Date: Wed, 29 May 2024 21:32:08 +0800
> > > Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with extra
> > >   tuneability
> >
> > The rationale and ChangeLog entries should be part of the commit message
> > (and so the git format-patch output).
> >
> > >
> > > Signed-off-by: TheShermanTanker <tanksherman27@gmail.com>
> >
> > A DCO sign-off can't use a pseudonym, sorry; please either sign off
> > using your real name or file a copyright assignment for the pseudonym
> > with the FSF.
> >
> > See https://gcc.gnu.org/contribute.html#legal for more detail.
> >
> > > ---
> > >   gcc/c-family/c.opt         |  8 ++++++++
> > >   gcc/c/c-typeck.cc          |  2 +-
> > >   gcc/c/gimple-parser.cc     |  2 +-
> > >   gcc/config/mingw/mingw32.h |  6 +++---
> > >   gcc/cp/coroutines.cc       |  2 +-
> > >   gcc/cp/typeck.cc           |  2 +-
> > >   gcc/doc/invoke.texi        | 13 +++++++++++++
> > >   gcc/tree-cfg.cc            |  2 +-
> > >   8 files changed, 29 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> > > index fb34c3b7031..32a2859fdcc 100644
> > > --- a/gcc/c-family/c.opt
> > > +++ b/gcc/c-family/c.opt
> > > @@ -886,6 +886,14 @@ Winvalid-constexpr
> > >   C++ ObjC++ Var(warn_invalid_constexpr) Init(-1) Warning
> > >   Warn when a function never produces a constant expression.
> > >
> > > +Winvalid-noreturn=explicit
> > > +C ObjC C++ ObjC++ Warning
> > > +Warn when a function marked noreturn returns explicitly.
> > > +
> > > +Winvalid-noreturn=implicit
> > > +C ObjC C++ ObjC++ Warning
> > > +Warn when a function marked noreturn returns implicitly.
> > > +
> > >   Winvalid-offsetof
> > >   C++ ObjC++ Var(warn_invalid_offsetof) Init(1) Warning
> > >   Warn about invalid uses of the \"offsetof\" macro.
> > > diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> > > index ad4c7add562..1941fbc44cb 100644
> > > --- a/gcc/c/c-typeck.cc
> > > +++ b/gcc/c/c-typeck.cc
> > > @@ -11468,7 +11468,7 @@ c_finish_return (location_t loc, tree retval, tree origtype)
> > >     location_t xloc = expansion_point_location_if_in_system_header (loc);
> > >
> > >     if (TREE_THIS_VOLATILE (current_function_decl))
> > > -    warning_at (xloc, 0,
> > > +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
> > >               "function declared %<noreturn%> has a %<return%> statement");
> > >
> > >     if (retval)
> > > diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
> > > index d156d83cd37..1acaf75f844 100644
> > > --- a/gcc/c/gimple-parser.cc
> > > +++ b/gcc/c/gimple-parser.cc
> > > @@ -2593,7 +2593,7 @@ c_finish_gimple_return (location_t loc, tree retval)
> > >     location_t xloc = expansion_point_location_if_in_system_header (loc);
> > >
> > >     if (TREE_THIS_VOLATILE (current_function_decl))
> > > -    warning_at (xloc, 0,
> > > +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
> > >               "function declared %<noreturn%> has a %<return%> statement");
> > >
> > >     if (! retval)
> > > diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h
> > > index fa6e307476c..a69926133b1 100644
> > > --- a/gcc/config/mingw/mingw32.h
> > > +++ b/gcc/config/mingw/mingw32.h
> > > @@ -35,9 +35,9 @@ along with GCC; see the file COPYING3.  If not see
> > >        | MASK_MS_BITFIELD_LAYOUT)
> > >
> > >   #ifdef TARGET_USING_MCFGTHREAD
> > > -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__");
> > > +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__")
> > >   #elif defined(TARGET_USE_PTHREAD_BY_DEFAULT)
> > > -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__");
> > > +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__")
> > >   #else
> > >   #define DEFINE_THREAD_MODEL
> > >   #endif
> > > @@ -60,7 +60,7 @@ along with GCC; see the file COPYING3.  If not see
> > >         builtin_define_std ("WIN64");                         \
> > >         builtin_define ("_WIN64");                            \
> > >       }                                                       \
> > > -      DEFINE_THREAD_MODEL                                    \
> > > +      DEFINE_THREAD_MODEL;                                   \
> > >       }                                                               \
> > >     while (0)
> > >
> > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> > > index 97bc211ff67..53e56bd4959 100644
> > > --- a/gcc/cp/coroutines.cc
> > > +++ b/gcc/cp/coroutines.cc
> > > @@ -1375,7 +1375,7 @@ 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,
> > > +    warning_at (kw, OPT_Winvalid_noreturn_explicit,
> > >               "function declared %<noreturn%> has a"
> > >               " %<co_return%> statement");
> > >
> > > diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> > > index 1b7a31d32f3..74cc59bfb87 100644
> > > --- a/gcc/cp/typeck.cc
> > > +++ b/gcc/cp/typeck.cc
> > > @@ -11062,7 +11062,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
> > >        (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");
> > > +    warning (OPT_Winvalid_noreturn_explicit, "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 2cba380718b..27d880fd4f0 100644
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -376,6 +376,7 @@ Objective-C and Objective-C++ Dialects}.
> > >   -Winfinite-recursion
> > >   -Winit-self  -Winline  -Wno-int-conversion  -Wint-in-bool-context
> > >   -Wno-int-to-pointer-cast  -Wno-invalid-memory-model
> > > +-Winvalid-noreturn=explicit  -Winvalid-noreturn=implicit
> > >   -Winvalid-pch  -Winvalid-utf8  -Wno-unicode  -Wjump-misses-init
> > >   -Wlarger-than=@var{byte-size}  -Wlogical-not-parentheses  -Wlogical-op
> > >   -Wlong-long  -Wno-lto-type-mismatch -Wmain  -Wmaybe-uninitialized
> > > @@ -10482,6 +10483,18 @@ an error. @option{Wint-to-pointer-cast} is enabled by default.
> > >   Suppress warnings from casts from a pointer to an integer type of a
> > >   different size.
> > >
> > > +@opindex Winvalid-noreturn=explicit
> > > +@opindex Wno-invalid-noreturn=explicit
> > > +@item -Winvalid-noreturn=explicit
> > > +Warn if a function marked noreturn returns explicitly, that is, it
> > > +contains a return statement.
> > > +
> > > +@opindex Winvalid-noreturn=implicit
> > > +@opindex Wno-invalid-noreturn=implicit
> > > +@item -Winvalid-noreturn=implicit
> > > +Warn if a function marked noreturn returns implicitly, that is, it
> > > +has a path in its control flow that can reach the end of its body.
> > > +
> > >   @opindex Winvalid-pch
> > >   @opindex Wno-invalid-pch
> > >   @item -Winvalid-pch
> > > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> > > index 7fb7b92966b..cfe91038dd1 100644
> > > --- a/gcc/tree-cfg.cc
> > > +++ b/gcc/tree-cfg.cc
> > > @@ -9817,7 +9817,7 @@ pass_warn_function_return::execute (function *fun)
> > >       }
> > >         if (location == UNKNOWN_LOCATION)
> > >       location = cfun->function_end_locus;
> > > -      warning_at (location, 0, "%<noreturn%> function does return");
> > > +      warning_at (location, OPT_Winvalid_noreturn_implicit, "%<noreturn%> function does return");
> > >       }
> > >
> > >     /* If we see "return;" in some basic block, then we do reach the end
> >

Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability

[Jason Merrill]

On 6/1/24 11:31, Julian Waters wrote:
> Hi Jason,
> 
> Thanks for the reply! I'll address your comments soon. I have a
> question, if there is an option defined in c.opt as an Enum, like
> fstrong-eval-order, and the -no variant of the option is passed, would
> the Var somehow reflect the negated option? Eg
> 
> Winvalid-noreturn=
> C ObjC C++ ObjC++ Var(warn_invalid_noreturn) Joined
> Enum(invalid_noreturn) Warning
> 
> Enum
> Name(invalid_noreturn) Type(int)
> 
> EnumValue
> Enum(invalid_noreturn) String(explicit) Value(0)

-fstrong-eval-order has

fstrong-eval-order
C++ ObjC++ Common Alias(fstrong-eval-order=, all, none)

to represent that plain -fstrong-eval-order is equivalent to 
-fstrong-eval-order=all, and -fno-strong-eval-order is equivalent to =none.

> Would warn_invalid_noreturn then != 0 if
> -Wno-invalid-noreturn=explicit is passed? Or is there a way to make a
> warning call depend on 2 different OPT_ entries?

Typically = options will specify RejectNegative so the driver will 
reject e.g. -Wno-invalid-noreturn=explicit.

Jason

> best regards,
> Julian
> 
> On Sat, Jun 1, 2024 at 4:57 AM Jason Merrill <jason@redhat.com> wrote:
>>
>> On 5/29/24 09:58, Julian Waters wrote:
>>> Currently, gcc warns about noreturn marked functions that return both explicitly and implicitly, with no way to turn this warning off. clang does have an option for these classes of warnings, -Winvalid-noreturn. However, we can do better. Instead of just having 1 option that switches the warnings for both on and off, we can define an extra layer of granularity, and have a separate options for implicit returns and explicit returns, as in -Winvalid-return=explicit and -Winvalid-noreturn=implicit. This patch adds both to gcc, for compatibility with clang.
>>
>> Thanks!
>>
>>> Do note that I am relatively new to gcc's codebase, and as such couldn't figure out how to cleanly define a general -Winvalid-noreturn warning that switch both on and off, for better compatibility with clang. If someone should point out how to do so, I'll happily rewrite my patch.
>>
>> See -fstrong-eval-order for an example of an option that can be used
>> with or without =arg.
>>
>>> I also do not have write access to gcc, and will need help pushing this patch once the green light is given
>>
>> Good to know, I can take care of that.
>>
>>> best regards,
>>> Julian
>>>
>>> gcc/c-family/ChangeLog:
>>>
>>>        * c.opt: Introduce -Winvalid-noreturn=explicit and -Winvalid-noreturn=implicit
>>>
>>> gcc/ChangeLog:
>>>
>>>        * tree-cfg.cc (pass_warn_function_return::execute): Use it
>>>
>>> gcc/c/ChangeLog:
>>>
>>>        * c-typeck.cc (c_finish_return): Use it
>>>        * gimple-parser.cc (c_finish_gimple_return): Use it
>>>
>>> gcc/config/mingw/ChangeLog:
>>>
>>>        * mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons
>>>
>>> gcc/cp/ChangeLog:
>>>
>>>        * coroutines.cc (finish_co_return_stmt): Use it
>>>        * typeck.cc (check_return_expr): Use it
>>>
>>> gcc/doc/ChangeLog:
>>>
>>>        * invoke.texi: Document new options
>>>
>>>   From 4daf884f8bbc1e318ba93121a6fdf4139da80b64 Mon Sep 17 00:00:00 2001
>>> From: TheShermanTanker <tanksherman27@gmail.com>
>>> Date: Wed, 29 May 2024 21:32:08 +0800
>>> Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with extra
>>>    tuneability
>>
>> The rationale and ChangeLog entries should be part of the commit message
>> (and so the git format-patch output).
>>
>>>
>>> Signed-off-by: TheShermanTanker <tanksherman27@gmail.com>
>>
>> A DCO sign-off can't use a pseudonym, sorry; please either sign off
>> using your real name or file a copyright assignment for the pseudonym
>> with the FSF.
>>
>> See https://gcc.gnu.org/contribute.html#legal for more detail.
>>
>>> ---
>>>    gcc/c-family/c.opt         |  8 ++++++++
>>>    gcc/c/c-typeck.cc          |  2 +-
>>>    gcc/c/gimple-parser.cc     |  2 +-
>>>    gcc/config/mingw/mingw32.h |  6 +++---
>>>    gcc/cp/coroutines.cc       |  2 +-
>>>    gcc/cp/typeck.cc           |  2 +-
>>>    gcc/doc/invoke.texi        | 13 +++++++++++++
>>>    gcc/tree-cfg.cc            |  2 +-
>>>    8 files changed, 29 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>> index fb34c3b7031..32a2859fdcc 100644
>>> --- a/gcc/c-family/c.opt
>>> +++ b/gcc/c-family/c.opt
>>> @@ -886,6 +886,14 @@ Winvalid-constexpr
>>>    C++ ObjC++ Var(warn_invalid_constexpr) Init(-1) Warning
>>>    Warn when a function never produces a constant expression.
>>>
>>> +Winvalid-noreturn=explicit
>>> +C ObjC C++ ObjC++ Warning
>>> +Warn when a function marked noreturn returns explicitly.
>>> +
>>> +Winvalid-noreturn=implicit
>>> +C ObjC C++ ObjC++ Warning
>>> +Warn when a function marked noreturn returns implicitly.
>>> +
>>>    Winvalid-offsetof
>>>    C++ ObjC++ Var(warn_invalid_offsetof) Init(1) Warning
>>>    Warn about invalid uses of the \"offsetof\" macro.
>>> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
>>> index ad4c7add562..1941fbc44cb 100644
>>> --- a/gcc/c/c-typeck.cc
>>> +++ b/gcc/c/c-typeck.cc
>>> @@ -11468,7 +11468,7 @@ c_finish_return (location_t loc, tree retval, tree origtype)
>>>      location_t xloc = expansion_point_location_if_in_system_header (loc);
>>>
>>>      if (TREE_THIS_VOLATILE (current_function_decl))
>>> -    warning_at (xloc, 0,
>>> +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
>>>                "function declared %<noreturn%> has a %<return%> statement");
>>>
>>>      if (retval)
>>> diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
>>> index d156d83cd37..1acaf75f844 100644
>>> --- a/gcc/c/gimple-parser.cc
>>> +++ b/gcc/c/gimple-parser.cc
>>> @@ -2593,7 +2593,7 @@ c_finish_gimple_return (location_t loc, tree retval)
>>>      location_t xloc = expansion_point_location_if_in_system_header (loc);
>>>
>>>      if (TREE_THIS_VOLATILE (current_function_decl))
>>> -    warning_at (xloc, 0,
>>> +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
>>>                "function declared %<noreturn%> has a %<return%> statement");
>>>
>>>      if (! retval)
>>> diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h
>>> index fa6e307476c..a69926133b1 100644
>>> --- a/gcc/config/mingw/mingw32.h
>>> +++ b/gcc/config/mingw/mingw32.h
>>> @@ -35,9 +35,9 @@ along with GCC; see the file COPYING3.  If not see
>>>         | MASK_MS_BITFIELD_LAYOUT)
>>>
>>>    #ifdef TARGET_USING_MCFGTHREAD
>>> -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__");
>>> +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__")
>>>    #elif defined(TARGET_USE_PTHREAD_BY_DEFAULT)
>>> -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__");
>>> +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__")
>>>    #else
>>>    #define DEFINE_THREAD_MODEL
>>>    #endif
>>> @@ -60,7 +60,7 @@ along with GCC; see the file COPYING3.  If not see
>>>          builtin_define_std ("WIN64");                         \
>>>          builtin_define ("_WIN64");                            \
>>>        }                                                       \
>>> -      DEFINE_THREAD_MODEL                                    \
>>> +      DEFINE_THREAD_MODEL;                                   \
>>>        }                                                               \
>>>      while (0)
>>>
>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
>>> index 97bc211ff67..53e56bd4959 100644
>>> --- a/gcc/cp/coroutines.cc
>>> +++ b/gcc/cp/coroutines.cc
>>> @@ -1375,7 +1375,7 @@ 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,
>>> +    warning_at (kw, OPT_Winvalid_noreturn_explicit,
>>>                "function declared %<noreturn%> has a"
>>>                " %<co_return%> statement");
>>>
>>> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
>>> index 1b7a31d32f3..74cc59bfb87 100644
>>> --- a/gcc/cp/typeck.cc
>>> +++ b/gcc/cp/typeck.cc
>>> @@ -11062,7 +11062,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>>>         (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");
>>> +    warning (OPT_Winvalid_noreturn_explicit, "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 2cba380718b..27d880fd4f0 100644
>>> --- a/gcc/doc/invoke.texi
>>> +++ b/gcc/doc/invoke.texi
>>> @@ -376,6 +376,7 @@ Objective-C and Objective-C++ Dialects}.
>>>    -Winfinite-recursion
>>>    -Winit-self  -Winline  -Wno-int-conversion  -Wint-in-bool-context
>>>    -Wno-int-to-pointer-cast  -Wno-invalid-memory-model
>>> +-Winvalid-noreturn=explicit  -Winvalid-noreturn=implicit
>>>    -Winvalid-pch  -Winvalid-utf8  -Wno-unicode  -Wjump-misses-init
>>>    -Wlarger-than=@var{byte-size}  -Wlogical-not-parentheses  -Wlogical-op
>>>    -Wlong-long  -Wno-lto-type-mismatch -Wmain  -Wmaybe-uninitialized
>>> @@ -10482,6 +10483,18 @@ an error. @option{Wint-to-pointer-cast} is enabled by default.
>>>    Suppress warnings from casts from a pointer to an integer type of a
>>>    different size.
>>>
>>> +@opindex Winvalid-noreturn=explicit
>>> +@opindex Wno-invalid-noreturn=explicit
>>> +@item -Winvalid-noreturn=explicit
>>> +Warn if a function marked noreturn returns explicitly, that is, it
>>> +contains a return statement.
>>> +
>>> +@opindex Winvalid-noreturn=implicit
>>> +@opindex Wno-invalid-noreturn=implicit
>>> +@item -Winvalid-noreturn=implicit
>>> +Warn if a function marked noreturn returns implicitly, that is, it
>>> +has a path in its control flow that can reach the end of its body.
>>> +
>>>    @opindex Winvalid-pch
>>>    @opindex Wno-invalid-pch
>>>    @item -Winvalid-pch
>>> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
>>> index 7fb7b92966b..cfe91038dd1 100644
>>> --- a/gcc/tree-cfg.cc
>>> +++ b/gcc/tree-cfg.cc
>>> @@ -9817,7 +9817,7 @@ pass_warn_function_return::execute (function *fun)
>>>        }
>>>          if (location == UNKNOWN_LOCATION)
>>>        location = cfun->function_end_locus;
>>> -      warning_at (location, 0, "%<noreturn%> function does return");
>>> +      warning_at (location, OPT_Winvalid_noreturn_implicit, "%<noreturn%> function does return");
>>>        }
>>>
>>>      /* If we see "return;" in some basic block, then we do reach the end
>>
> 

Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability

[Julian Waters]

Hi Jason,

Thanks for the reply. I'm a little bit overwhelmed with university at
the moment, would it be ok if I delay implementing this a little bit?

best regards,
Julian

On Tue, Jun 4, 2024 at 1:04 AM Jason Merrill <jason@redhat.com> wrote:
>
> On 6/1/24 11:31, Julian Waters wrote:
> > Hi Jason,
> >
> > Thanks for the reply! I'll address your comments soon. I have a
> > question, if there is an option defined in c.opt as an Enum, like
> > fstrong-eval-order, and the -no variant of the option is passed, would
> > the Var somehow reflect the negated option? Eg
> >
> > Winvalid-noreturn=
> > C ObjC C++ ObjC++ Var(warn_invalid_noreturn) Joined
> > Enum(invalid_noreturn) Warning
> >
> > Enum
> > Name(invalid_noreturn) Type(int)
> >
> > EnumValue
> > Enum(invalid_noreturn) String(explicit) Value(0)
>
> -fstrong-eval-order has
>
> fstrong-eval-order
> C++ ObjC++ Common Alias(fstrong-eval-order=, all, none)
>
> to represent that plain -fstrong-eval-order is equivalent to
> -fstrong-eval-order=all, and -fno-strong-eval-order is equivalent to =none.
>
> > Would warn_invalid_noreturn then != 0 if
> > -Wno-invalid-noreturn=explicit is passed? Or is there a way to make a
> > warning call depend on 2 different OPT_ entries?
>
> Typically = options will specify RejectNegative so the driver will
> reject e.g. -Wno-invalid-noreturn=explicit.
>
> Jason
>
> > best regards,
> > Julian
> >
> > On Sat, Jun 1, 2024 at 4:57 AM Jason Merrill <jason@redhat.com> wrote:
> >>
> >> On 5/29/24 09:58, Julian Waters wrote:
> >>> Currently, gcc warns about noreturn marked functions that return both explicitly and implicitly, with no way to turn this warning off. clang does have an option for these classes of warnings, -Winvalid-noreturn. However, we can do better. Instead of just having 1 option that switches the warnings for both on and off, we can define an extra layer of granularity, and have a separate options for implicit returns and explicit returns, as in -Winvalid-return=explicit and -Winvalid-noreturn=implicit. This patch adds both to gcc, for compatibility with clang.
> >>
> >> Thanks!
> >>
> >>> Do note that I am relatively new to gcc's codebase, and as such couldn't figure out how to cleanly define a general -Winvalid-noreturn warning that switch both on and off, for better compatibility with clang. If someone should point out how to do so, I'll happily rewrite my patch.
> >>
> >> See -fstrong-eval-order for an example of an option that can be used
> >> with or without =arg.
> >>
> >>> I also do not have write access to gcc, and will need help pushing this patch once the green light is given
> >>
> >> Good to know, I can take care of that.
> >>
> >>> best regards,
> >>> Julian
> >>>
> >>> gcc/c-family/ChangeLog:
> >>>
> >>>        * c.opt: Introduce -Winvalid-noreturn=explicit and -Winvalid-noreturn=implicit
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>>        * tree-cfg.cc (pass_warn_function_return::execute): Use it
> >>>
> >>> gcc/c/ChangeLog:
> >>>
> >>>        * c-typeck.cc (c_finish_return): Use it
> >>>        * gimple-parser.cc (c_finish_gimple_return): Use it
> >>>
> >>> gcc/config/mingw/ChangeLog:
> >>>
> >>>        * mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons
> >>>
> >>> gcc/cp/ChangeLog:
> >>>
> >>>        * coroutines.cc (finish_co_return_stmt): Use it
> >>>        * typeck.cc (check_return_expr): Use it
> >>>
> >>> gcc/doc/ChangeLog:
> >>>
> >>>        * invoke.texi: Document new options
> >>>
> >>>   From 4daf884f8bbc1e318ba93121a6fdf4139da80b64 Mon Sep 17 00:00:00 2001
> >>> From: TheShermanTanker <tanksherman27@gmail.com>
> >>> Date: Wed, 29 May 2024 21:32:08 +0800
> >>> Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with extra
> >>>    tuneability
> >>
> >> The rationale and ChangeLog entries should be part of the commit message
> >> (and so the git format-patch output).
> >>
> >>>
> >>> Signed-off-by: TheShermanTanker <tanksherman27@gmail.com>
> >>
> >> A DCO sign-off can't use a pseudonym, sorry; please either sign off
> >> using your real name or file a copyright assignment for the pseudonym
> >> with the FSF.
> >>
> >> See https://gcc.gnu.org/contribute.html#legal for more detail.
> >>
> >>> ---
> >>>    gcc/c-family/c.opt         |  8 ++++++++
> >>>    gcc/c/c-typeck.cc          |  2 +-
> >>>    gcc/c/gimple-parser.cc     |  2 +-
> >>>    gcc/config/mingw/mingw32.h |  6 +++---
> >>>    gcc/cp/coroutines.cc       |  2 +-
> >>>    gcc/cp/typeck.cc           |  2 +-
> >>>    gcc/doc/invoke.texi        | 13 +++++++++++++
> >>>    gcc/tree-cfg.cc            |  2 +-
> >>>    8 files changed, 29 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> >>> index fb34c3b7031..32a2859fdcc 100644
> >>> --- a/gcc/c-family/c.opt
> >>> +++ b/gcc/c-family/c.opt
> >>> @@ -886,6 +886,14 @@ Winvalid-constexpr
> >>>    C++ ObjC++ Var(warn_invalid_constexpr) Init(-1) Warning
> >>>    Warn when a function never produces a constant expression.
> >>>
> >>> +Winvalid-noreturn=explicit
> >>> +C ObjC C++ ObjC++ Warning
> >>> +Warn when a function marked noreturn returns explicitly.
> >>> +
> >>> +Winvalid-noreturn=implicit
> >>> +C ObjC C++ ObjC++ Warning
> >>> +Warn when a function marked noreturn returns implicitly.
> >>> +
> >>>    Winvalid-offsetof
> >>>    C++ ObjC++ Var(warn_invalid_offsetof) Init(1) Warning
> >>>    Warn about invalid uses of the \"offsetof\" macro.
> >>> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> >>> index ad4c7add562..1941fbc44cb 100644
> >>> --- a/gcc/c/c-typeck.cc
> >>> +++ b/gcc/c/c-typeck.cc
> >>> @@ -11468,7 +11468,7 @@ c_finish_return (location_t loc, tree retval, tree origtype)
> >>>      location_t xloc = expansion_point_location_if_in_system_header (loc);
> >>>
> >>>      if (TREE_THIS_VOLATILE (current_function_decl))
> >>> -    warning_at (xloc, 0,
> >>> +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
> >>>                "function declared %<noreturn%> has a %<return%> statement");
> >>>
> >>>      if (retval)
> >>> diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
> >>> index d156d83cd37..1acaf75f844 100644
> >>> --- a/gcc/c/gimple-parser.cc
> >>> +++ b/gcc/c/gimple-parser.cc
> >>> @@ -2593,7 +2593,7 @@ c_finish_gimple_return (location_t loc, tree retval)
> >>>      location_t xloc = expansion_point_location_if_in_system_header (loc);
> >>>
> >>>      if (TREE_THIS_VOLATILE (current_function_decl))
> >>> -    warning_at (xloc, 0,
> >>> +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
> >>>                "function declared %<noreturn%> has a %<return%> statement");
> >>>
> >>>      if (! retval)
> >>> diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h
> >>> index fa6e307476c..a69926133b1 100644
> >>> --- a/gcc/config/mingw/mingw32.h
> >>> +++ b/gcc/config/mingw/mingw32.h
> >>> @@ -35,9 +35,9 @@ along with GCC; see the file COPYING3.  If not see
> >>>         | MASK_MS_BITFIELD_LAYOUT)
> >>>
> >>>    #ifdef TARGET_USING_MCFGTHREAD
> >>> -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__");
> >>> +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__")
> >>>    #elif defined(TARGET_USE_PTHREAD_BY_DEFAULT)
> >>> -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__");
> >>> +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__")
> >>>    #else
> >>>    #define DEFINE_THREAD_MODEL
> >>>    #endif
> >>> @@ -60,7 +60,7 @@ along with GCC; see the file COPYING3.  If not see
> >>>          builtin_define_std ("WIN64");                         \
> >>>          builtin_define ("_WIN64");                            \
> >>>        }                                                       \
> >>> -      DEFINE_THREAD_MODEL                                    \
> >>> +      DEFINE_THREAD_MODEL;                                   \
> >>>        }                                                               \
> >>>      while (0)
> >>>
> >>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> >>> index 97bc211ff67..53e56bd4959 100644
> >>> --- a/gcc/cp/coroutines.cc
> >>> +++ b/gcc/cp/coroutines.cc
> >>> @@ -1375,7 +1375,7 @@ 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,
> >>> +    warning_at (kw, OPT_Winvalid_noreturn_explicit,
> >>>                "function declared %<noreturn%> has a"
> >>>                " %<co_return%> statement");
> >>>
> >>> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> >>> index 1b7a31d32f3..74cc59bfb87 100644
> >>> --- a/gcc/cp/typeck.cc
> >>> +++ b/gcc/cp/typeck.cc
> >>> @@ -11062,7 +11062,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
> >>>         (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");
> >>> +    warning (OPT_Winvalid_noreturn_explicit, "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 2cba380718b..27d880fd4f0 100644
> >>> --- a/gcc/doc/invoke.texi
> >>> +++ b/gcc/doc/invoke.texi
> >>> @@ -376,6 +376,7 @@ Objective-C and Objective-C++ Dialects}.
> >>>    -Winfinite-recursion
> >>>    -Winit-self  -Winline  -Wno-int-conversion  -Wint-in-bool-context
> >>>    -Wno-int-to-pointer-cast  -Wno-invalid-memory-model
> >>> +-Winvalid-noreturn=explicit  -Winvalid-noreturn=implicit
> >>>    -Winvalid-pch  -Winvalid-utf8  -Wno-unicode  -Wjump-misses-init
> >>>    -Wlarger-than=@var{byte-size}  -Wlogical-not-parentheses  -Wlogical-op
> >>>    -Wlong-long  -Wno-lto-type-mismatch -Wmain  -Wmaybe-uninitialized
> >>> @@ -10482,6 +10483,18 @@ an error. @option{Wint-to-pointer-cast} is enabled by default.
> >>>    Suppress warnings from casts from a pointer to an integer type of a
> >>>    different size.
> >>>
> >>> +@opindex Winvalid-noreturn=explicit
> >>> +@opindex Wno-invalid-noreturn=explicit
> >>> +@item -Winvalid-noreturn=explicit
> >>> +Warn if a function marked noreturn returns explicitly, that is, it
> >>> +contains a return statement.
> >>> +
> >>> +@opindex Winvalid-noreturn=implicit
> >>> +@opindex Wno-invalid-noreturn=implicit
> >>> +@item -Winvalid-noreturn=implicit
> >>> +Warn if a function marked noreturn returns implicitly, that is, it
> >>> +has a path in its control flow that can reach the end of its body.
> >>> +
> >>>    @opindex Winvalid-pch
> >>>    @opindex Wno-invalid-pch
> >>>    @item -Winvalid-pch
> >>> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> >>> index 7fb7b92966b..cfe91038dd1 100644
> >>> --- a/gcc/tree-cfg.cc
> >>> +++ b/gcc/tree-cfg.cc
> >>> @@ -9817,7 +9817,7 @@ pass_warn_function_return::execute (function *fun)
> >>>        }
> >>>          if (location == UNKNOWN_LOCATION)
> >>>        location = cfun->function_end_locus;
> >>> -      warning_at (location, 0, "%<noreturn%> function does return");
> >>> +      warning_at (location, OPT_Winvalid_noreturn_implicit, "%<noreturn%> function does return");
> >>>        }
> >>>
> >>>      /* If we see "return;" in some basic block, then we do reach the end
> >>
> >
>

Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability

[Jason Merrill]

On 6/10/24 03:13, Julian Waters wrote:
> Hi Jason,
> 
> Thanks for the reply. I'm a little bit overwhelmed with university at
> the moment, would it be ok if I delay implementing this a little bit?

Sure, we're still early in GCC 15 development, no time pressure.

> On Tue, Jun 4, 2024 at 1:04 AM Jason Merrill <jason@redhat.com> wrote:
>>
>> On 6/1/24 11:31, Julian Waters wrote:
>>> Hi Jason,
>>>
>>> Thanks for the reply! I'll address your comments soon. I have a
>>> question, if there is an option defined in c.opt as an Enum, like
>>> fstrong-eval-order, and the -no variant of the option is passed, would
>>> the Var somehow reflect the negated option? Eg
>>>
>>> Winvalid-noreturn=
>>> C ObjC C++ ObjC++ Var(warn_invalid_noreturn) Joined
>>> Enum(invalid_noreturn) Warning
>>>
>>> Enum
>>> Name(invalid_noreturn) Type(int)
>>>
>>> EnumValue
>>> Enum(invalid_noreturn) String(explicit) Value(0)
>>
>> -fstrong-eval-order has
>>
>> fstrong-eval-order
>> C++ ObjC++ Common Alias(fstrong-eval-order=, all, none)
>>
>> to represent that plain -fstrong-eval-order is equivalent to
>> -fstrong-eval-order=all, and -fno-strong-eval-order is equivalent to =none.
>>
>>> Would warn_invalid_noreturn then != 0 if
>>> -Wno-invalid-noreturn=explicit is passed? Or is there a way to make a
>>> warning call depend on 2 different OPT_ entries?
>>
>> Typically = options will specify RejectNegative so the driver will
>> reject e.g. -Wno-invalid-noreturn=explicit.
>>
>> Jason
>>
>>> best regards,
>>> Julian
>>>
>>> On Sat, Jun 1, 2024 at 4:57 AM Jason Merrill <jason@redhat.com> wrote:
>>>>
>>>> On 5/29/24 09:58, Julian Waters wrote:
>>>>> Currently, gcc warns about noreturn marked functions that return both explicitly and implicitly, with no way to turn this warning off. clang does have an option for these classes of warnings, -Winvalid-noreturn. However, we can do better. Instead of just having 1 option that switches the warnings for both on and off, we can define an extra layer of granularity, and have a separate options for implicit returns and explicit returns, as in -Winvalid-return=explicit and -Winvalid-noreturn=implicit. This patch adds both to gcc, for compatibility with clang.
>>>>
>>>> Thanks!
>>>>
>>>>> Do note that I am relatively new to gcc's codebase, and as such couldn't figure out how to cleanly define a general -Winvalid-noreturn warning that switch both on and off, for better compatibility with clang. If someone should point out how to do so, I'll happily rewrite my patch.
>>>>
>>>> See -fstrong-eval-order for an example of an option that can be used
>>>> with or without =arg.
>>>>
>>>>> I also do not have write access to gcc, and will need help pushing this patch once the green light is given
>>>>
>>>> Good to know, I can take care of that.
>>>>
>>>>> best regards,
>>>>> Julian
>>>>>
>>>>> gcc/c-family/ChangeLog:
>>>>>
>>>>>         * c.opt: Introduce -Winvalid-noreturn=explicit and -Winvalid-noreturn=implicit
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>>         * tree-cfg.cc (pass_warn_function_return::execute): Use it
>>>>>
>>>>> gcc/c/ChangeLog:
>>>>>
>>>>>         * c-typeck.cc (c_finish_return): Use it
>>>>>         * gimple-parser.cc (c_finish_gimple_return): Use it
>>>>>
>>>>> gcc/config/mingw/ChangeLog:
>>>>>
>>>>>         * mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>>         * coroutines.cc (finish_co_return_stmt): Use it
>>>>>         * typeck.cc (check_return_expr): Use it
>>>>>
>>>>> gcc/doc/ChangeLog:
>>>>>
>>>>>         * invoke.texi: Document new options
>>>>>
>>>>>    From 4daf884f8bbc1e318ba93121a6fdf4139da80b64 Mon Sep 17 00:00:00 2001
>>>>> From: TheShermanTanker <tanksherman27@gmail.com>
>>>>> Date: Wed, 29 May 2024 21:32:08 +0800
>>>>> Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with extra
>>>>>     tuneability
>>>>
>>>> The rationale and ChangeLog entries should be part of the commit message
>>>> (and so the git format-patch output).
>>>>
>>>>>
>>>>> Signed-off-by: TheShermanTanker <tanksherman27@gmail.com>
>>>>
>>>> A DCO sign-off can't use a pseudonym, sorry; please either sign off
>>>> using your real name or file a copyright assignment for the pseudonym
>>>> with the FSF.
>>>>
>>>> See https://gcc.gnu.org/contribute.html#legal for more detail.
>>>>
>>>>> ---
>>>>>     gcc/c-family/c.opt         |  8 ++++++++
>>>>>     gcc/c/c-typeck.cc          |  2 +-
>>>>>     gcc/c/gimple-parser.cc     |  2 +-
>>>>>     gcc/config/mingw/mingw32.h |  6 +++---
>>>>>     gcc/cp/coroutines.cc       |  2 +-
>>>>>     gcc/cp/typeck.cc           |  2 +-
>>>>>     gcc/doc/invoke.texi        | 13 +++++++++++++
>>>>>     gcc/tree-cfg.cc            |  2 +-
>>>>>     8 files changed, 29 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>>>> index fb34c3b7031..32a2859fdcc 100644
>>>>> --- a/gcc/c-family/c.opt
>>>>> +++ b/gcc/c-family/c.opt
>>>>> @@ -886,6 +886,14 @@ Winvalid-constexpr
>>>>>     C++ ObjC++ Var(warn_invalid_constexpr) Init(-1) Warning
>>>>>     Warn when a function never produces a constant expression.
>>>>>
>>>>> +Winvalid-noreturn=explicit
>>>>> +C ObjC C++ ObjC++ Warning
>>>>> +Warn when a function marked noreturn returns explicitly.
>>>>> +
>>>>> +Winvalid-noreturn=implicit
>>>>> +C ObjC C++ ObjC++ Warning
>>>>> +Warn when a function marked noreturn returns implicitly.
>>>>> +
>>>>>     Winvalid-offsetof
>>>>>     C++ ObjC++ Var(warn_invalid_offsetof) Init(1) Warning
>>>>>     Warn about invalid uses of the \"offsetof\" macro.
>>>>> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
>>>>> index ad4c7add562..1941fbc44cb 100644
>>>>> --- a/gcc/c/c-typeck.cc
>>>>> +++ b/gcc/c/c-typeck.cc
>>>>> @@ -11468,7 +11468,7 @@ c_finish_return (location_t loc, tree retval, tree origtype)
>>>>>       location_t xloc = expansion_point_location_if_in_system_header (loc);
>>>>>
>>>>>       if (TREE_THIS_VOLATILE (current_function_decl))
>>>>> -    warning_at (xloc, 0,
>>>>> +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
>>>>>                 "function declared %<noreturn%> has a %<return%> statement");
>>>>>
>>>>>       if (retval)
>>>>> diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
>>>>> index d156d83cd37..1acaf75f844 100644
>>>>> --- a/gcc/c/gimple-parser.cc
>>>>> +++ b/gcc/c/gimple-parser.cc
>>>>> @@ -2593,7 +2593,7 @@ c_finish_gimple_return (location_t loc, tree retval)
>>>>>       location_t xloc = expansion_point_location_if_in_system_header (loc);
>>>>>
>>>>>       if (TREE_THIS_VOLATILE (current_function_decl))
>>>>> -    warning_at (xloc, 0,
>>>>> +    warning_at (xloc, OPT_Winvalid_noreturn_explicit,
>>>>>                 "function declared %<noreturn%> has a %<return%> statement");
>>>>>
>>>>>       if (! retval)
>>>>> diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h
>>>>> index fa6e307476c..a69926133b1 100644
>>>>> --- a/gcc/config/mingw/mingw32.h
>>>>> +++ b/gcc/config/mingw/mingw32.h
>>>>> @@ -35,9 +35,9 @@ along with GCC; see the file COPYING3.  If not see
>>>>>          | MASK_MS_BITFIELD_LAYOUT)
>>>>>
>>>>>     #ifdef TARGET_USING_MCFGTHREAD
>>>>> -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__");
>>>>> +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_MCFGTHREAD__")
>>>>>     #elif defined(TARGET_USE_PTHREAD_BY_DEFAULT)
>>>>> -#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__");
>>>>> +#define DEFINE_THREAD_MODEL  builtin_define ("__USING_POSIXTHREAD__")
>>>>>     #else
>>>>>     #define DEFINE_THREAD_MODEL
>>>>>     #endif
>>>>> @@ -60,7 +60,7 @@ along with GCC; see the file COPYING3.  If not see
>>>>>           builtin_define_std ("WIN64");                         \
>>>>>           builtin_define ("_WIN64");                            \
>>>>>         }                                                       \
>>>>> -      DEFINE_THREAD_MODEL                                    \
>>>>> +      DEFINE_THREAD_MODEL;                                   \
>>>>>         }                                                               \
>>>>>       while (0)
>>>>>
>>>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
>>>>> index 97bc211ff67..53e56bd4959 100644
>>>>> --- a/gcc/cp/coroutines.cc
>>>>> +++ b/gcc/cp/coroutines.cc
>>>>> @@ -1375,7 +1375,7 @@ 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,
>>>>> +    warning_at (kw, OPT_Winvalid_noreturn_explicit,
>>>>>                 "function declared %<noreturn%> has a"
>>>>>                 " %<co_return%> statement");
>>>>>
>>>>> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
>>>>> index 1b7a31d32f3..74cc59bfb87 100644
>>>>> --- a/gcc/cp/typeck.cc
>>>>> +++ b/gcc/cp/typeck.cc
>>>>> @@ -11062,7 +11062,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>>>>>          (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");
>>>>> +    warning (OPT_Winvalid_noreturn_explicit, "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 2cba380718b..27d880fd4f0 100644
>>>>> --- a/gcc/doc/invoke.texi
>>>>> +++ b/gcc/doc/invoke.texi
>>>>> @@ -376,6 +376,7 @@ Objective-C and Objective-C++ Dialects}.
>>>>>     -Winfinite-recursion
>>>>>     -Winit-self  -Winline  -Wno-int-conversion  -Wint-in-bool-context
>>>>>     -Wno-int-to-pointer-cast  -Wno-invalid-memory-model
>>>>> +-Winvalid-noreturn=explicit  -Winvalid-noreturn=implicit
>>>>>     -Winvalid-pch  -Winvalid-utf8  -Wno-unicode  -Wjump-misses-init
>>>>>     -Wlarger-than=@var{byte-size}  -Wlogical-not-parentheses  -Wlogical-op
>>>>>     -Wlong-long  -Wno-lto-type-mismatch -Wmain  -Wmaybe-uninitialized
>>>>> @@ -10482,6 +10483,18 @@ an error. @option{Wint-to-pointer-cast} is enabled by default.
>>>>>     Suppress warnings from casts from a pointer to an integer type of a
>>>>>     different size.
>>>>>
>>>>> +@opindex Winvalid-noreturn=explicit
>>>>> +@opindex Wno-invalid-noreturn=explicit
>>>>> +@item -Winvalid-noreturn=explicit
>>>>> +Warn if a function marked noreturn returns explicitly, that is, it
>>>>> +contains a return statement.
>>>>> +
>>>>> +@opindex Winvalid-noreturn=implicit
>>>>> +@opindex Wno-invalid-noreturn=implicit
>>>>> +@item -Winvalid-noreturn=implicit
>>>>> +Warn if a function marked noreturn returns implicitly, that is, it
>>>>> +has a path in its control flow that can reach the end of its body.
>>>>> +
>>>>>     @opindex Winvalid-pch
>>>>>     @opindex Wno-invalid-pch
>>>>>     @item -Winvalid-pch
>>>>> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
>>>>> index 7fb7b92966b..cfe91038dd1 100644
>>>>> --- a/gcc/tree-cfg.cc
>>>>> +++ b/gcc/tree-cfg.cc
>>>>> @@ -9817,7 +9817,7 @@ pass_warn_function_return::execute (function *fun)
>>>>>         }
>>>>>           if (location == UNKNOWN_LOCATION)
>>>>>         location = cfun->function_end_locus;
>>>>> -      warning_at (location, 0, "%<noreturn%> function does return");
>>>>> +      warning_at (location, OPT_Winvalid_noreturn_implicit, "%<noreturn%> function does return");
>>>>>         }
>>>>>
>>>>>       /* If we see "return;" in some basic block, then we do reach the end
>>>>
>>>
>>
>