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

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(-)

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

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
>

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

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

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

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

Hi Jason,

Sorry for the long period of radio silence, but I'm finally done with
all my university coursework. The main issue I see here is how to
process the Winvalid-noreturn= entry. Can I define it in c.opt and
then process it in c_common_handle_option and then store whether the
-W or -Wno form was used in a bool somewhere? If I can do that
somehow, and then access the bool at the callsites where it is
required, I can figure out the rest on my own

best regards,
Julian

On Tue, Jun 11, 2024 at 10:26 AM Jason Merrill <jason@redhat.com> wrote:
>
> 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
> >>>>
> >>>
> >>
> >
>

On 7/9/24 6:46 AM, Julian Waters wrote:
> Hi Jason,
> 
> Sorry for the long period of radio silence, but I'm finally done with
> all my university coursework. The main issue I see here is how to
> process the Winvalid-noreturn= entry. Can I define it in c.opt and
> then process it in c_common_handle_option and then store whether the
> -W or -Wno form was used in a bool somewhere? If I can do that
> somehow, and then access the bool at the callsites where it is
> required, I can figure out the rest on my own

You should be able to handle setting the variable entirely in c.opt, 
following the pattern of -fstrong-eval-order.  The variable is defined 
by the Var(...) notation in c.opt, and the opts code handles setting it.

> On Tue, Jun 11, 2024 at 10:26 AM Jason Merrill <jason@redhat.com> wrote:
>>
>> 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
>>>>>>
>>>>>
>>>>
>>>
>>
>

Darn, not even sending it from my friend's mail account worked to combat
the patch formatting issues. Not only is the patch a complete wreck on the
mailing list archive, but my friend's mail account has now somehow merged
into mine on the list, resulting in a truly confusing author display name.
I really hope you've somehow gotten the patch intact on your end...

best regards,
Julian

On Fri, Jul 19, 2024 at 11:47 AM Tay Joseph <tanksherman27@gmail.com> wrote:

> Hi Jason,
>
>
>
> I’ve managed to address your review comments, and rewrite the patch to
> include both -Winvalid-noreturn and -Winvalid-noreturn= but have trouble
> figuring out how to format invoke.texi and where to add the documentation
> for the new warning options. I’ve also made this a Common option, since
> it’s come to my attention that the warning is not specific to c-family, but
> is also used by other languages too (See tree-cfg.cc). Here’s the current
> version of the patch, hope it’s good to go this time
>
>
>
> best regards,
>
> Julian
>
>
>
> From fe1b4d5747e05101410b6bb6db9430362e3977d9 Mon Sep 17 00:00:00 2001
>
> From: Julian Waters <tanksherman27@gmail.com>
>
> Date: Fri, 19 Jul 2024 11:22:38 +0800
>
> Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with
> extra
>
> tuneability
>
>
>
> 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, on top of the
> regular -Winvalid-noreturn. This patch adds both to gcc, for compatibility
> with clang.
>
>
>
> gcc/ChangeLog:
>
>
>
>         * common.opt: Introduce -Winvalid-noreturn and -Winvalid-noreturn=
>
>         * opts.cc (common_handle_option): Handle new option
>
>         * flag-types.h: New flags for -Winvalid-noreturn=
>
>         * 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
>
>
>
> Signed-off-by: Julian Waters <tanksherman27@gmail.com>
>
> ---
>
> gcc/c/c-typeck.cc          |  4 ++--
>
> gcc/c/gimple-parser.cc     |  4 ++--
>
> gcc/common.opt             | 13 +++++++++++++
>
> gcc/config/mingw/mingw32.h |  6 +++---
>
> gcc/cp/coroutines.cc       |  4 ++--
>
> gcc/cp/typeck.cc           |  4 ++--
>
> gcc/flag-types.h           |  7 +++++++
>
> gcc/opts.cc                | 19 +++++++++++++++++++
>
> gcc/tree-cfg.cc            |  3 ++-
>
> 9 files changed, 52 insertions(+), 12 deletions(-)
>
>
>
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
>
> index 7e0f01ed22b..0f8c8cfff2e 100644
>
> --- a/gcc/c/c-typeck.cc
>
> +++ b/gcc/c/c-typeck.cc
>
> @@ -11738,8 +11738,8 @@ 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,
>
> +  if (TREE_THIS_VOLATILE (current_function_decl) && flag_invalid_noreturn
> != invalid_noreturn_kind::EXPLICIT)
>
> +    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 d156d83cd37..8a684fe334a 100644
>
> --- a/gcc/c/gimple-parser.cc
>
> +++ b/gcc/c/gimple-parser.cc
>
> @@ -2592,8 +2592,8 @@ 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,
>
> +  if (TREE_THIS_VOLATILE (current_function_decl) && flag_invalid_noreturn
> != invalid_noreturn_kind::EXPLICIT)
>
> +    warning_at (xloc, OPT_Winvalid_noreturn,
>
>                      "function declared %<noreturn%> has a %<return%>
> statement");
>
>    if (! retval)
>
> diff --git a/gcc/common.opt b/gcc/common.opt
>
> index ea39f87ae71..d44ff0231c3 100644
>
> --- a/gcc/common.opt
>
> +++ b/gcc/common.opt
>
> @@ -94,6 +94,11 @@ void *flag_instrument_functions_exclude_files
>
> Variable
>
> void *flag_ignored_attributes
>
> +; The noreturn kind to ignore when warning for noreturn code that
>
> +; does return.
>
> +Variable
>
> +invalid_noreturn_kind flag_invalid_noreturn = invalid_noreturn_kind::NONE
>
> +
>
> ; Generic structs (e.g. templates not explicitly specialized)
>
> ; may not have a compilation unit associated with them, and so
>
> ; may need to be treated differently from ordinary structs.
>
> @@ -667,6 +672,14 @@ Winvalid-memory-model
>
> Common Var(warn_invalid_memory_model) Init(1) Warning
>
> Warn when an atomic memory model parameter is known to be outside the
> valid range.
>
> +Winvalid-noreturn
>
> +Common Warning
>
> +Warn when code marked noreturn returns implicitly and/or explicitly.
>
> +
>
> +Winvalid-noreturn=
>
> +Common Joined Warning
>
> +Warn when code marked noreturn returns in the manner specified.
>
> +
>
> Wlarger-than-
>
> Common RejectNegative Joined Warning Undocumented Alias(Wlarger-than=)
>
> diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h
>
> index 0dfe8e995b6..48eadfa2c2c 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 f350fc33e9b..f49ff82b1ee 100644
>
> --- a/gcc/cp/coroutines.cc
>
> +++ b/gcc/cp/coroutines.cc
>
> @@ -1374,8 +1374,8 @@ 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,
>
> +  if (TREE_THIS_VOLATILE (current_function_decl) && flag_invalid_noreturn
> != invalid_noreturn_kind::EXPLICIT)
>
> +    warning_at (kw, OPT_Winvalid_noreturn,
>
>                      "function declared %<noreturn%> has a"
>
>                      " %<co_return%> statement");
>
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
>
> index 8df8b871676..260a9f99b08 100644
>
> --- a/gcc/cp/typeck.cc
>
> +++ b/gcc/cp/typeck.cc
>
> @@ -11080,8 +11080,8 @@ 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) && flag_invalid_noreturn
> != invalid_noreturn_kind::EXPLICIT)
>
> +    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/flag-types.h b/gcc/flag-types.h
>
> index 1e497f0bb91..591acd2a7d2 100644
>
> --- a/gcc/flag-types.h
>
> +++ b/gcc/flag-types.h
>
> @@ -375,6 +375,13 @@ enum incremental_link {
>
>    INCREMENTAL_LINK_LTO
>
> };
>
> +/* Kind of noreturn to ignore when reporting invalid noreturns  */
>
> +enum class invalid_noreturn_kind {
>
> +  NONE,
>
> +  IMPLICIT,
>
> +  EXPLICIT
>
> +};
>
> +
>
> /* Different trace modes.  */
>
> enum sanitize_coverage_code {
>
>    /* Trace PC.  */
>
> diff --git a/gcc/opts.cc b/gcc/opts.cc
>
> index be90a632338..19d31c262d7 100644
>
> --- a/gcc/opts.cc
>
> +++ b/gcc/opts.cc
>
> @@ -2860,6 +2860,25 @@ common_handle_option (struct gcc_options *opts,
>
>        dc->m_fatal_errors = value;
>
>        break;
>
> +    case OPT_Winvalid_noreturn_:
>
> +      if (lang_mask == CL_DRIVER)
>
> +         break;
>
> +
>
> +      if (value)
>
> +         warning (0, "%<-Winvalid-noreturn=%> is enabled by default,
> ignoring");
>
> +      else if (strcmp (arg, "explicit"))
>
> +         {
>
> +           opts->x_flag_invalid_noreturn =
> invalid_noreturn_kind::EXPLICIT;
>
> +         }
>
> +      else if (strcmp (arg, "implicit"))
>
> +         {
>
> +           opts->x_flag_invalid_noreturn =
> invalid_noreturn_kind::IMPLICIT;
>
> +         }
>
> +      else
>
> +         error_at (loc, "Unknown option for %<-Winvalid-noreturn=%>");
>
> +
>
> +      break;
>
> +
>
>      case OPT_Wstack_usage_:
>
>        opts->x_flag_stack_usage_info = value != -1;
>
>        break;
>
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
>
> index e6fd1294b95..f28dabc2bbf 100644
>
> --- a/gcc/tree-cfg.cc
>
> +++ b/gcc/tree-cfg.cc
>
> @@ -9820,7 +9820,8 @@ 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 (flag_invalid_noreturn != invalid_noreturn_kind::IMPLICIT)
>
> +         warning_at (location, 0, "%<noreturn%> function does return");
>
>      }
>
>    /* If we see "return;" in some basic block, then we do reach the end
>
> --
>
> 2.45.1
>
>
>
>
>
> Sent from Mail <https://go.microsoft.com/fwlink/?LinkId=550986> for
> Windows
>
>
>
> *From: *Jason Merrill <jason@redhat.com>
> *Sent: *Thursday, 18 July 2024 3:27 am
> *To: *Julian Waters <tanksherman27@gmail.com>; gcc-patches@gcc.gnu.org
> *Subject: *Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag
> from clang with extra tuneability
>
>
>
> On 7/9/24 6:46 AM, Julian Waters wrote:
>
> > Hi Jason,
>
> >
>
> > Sorry for the long period of radio silence, but I'm finally done with
>
> > all my university coursework. The main issue I see here is how to
>
> > process the Winvalid-noreturn= entry. Can I define it in c.opt and
>
> > then process it in c_common_handle_option and then store whether the
>
> > -W or -Wno form was used in a bool somewhere? If I can do that
>
> > somehow, and then access the bool at the callsites where it is
>
> > required, I can figure out the rest on my own
>
>
>
> You should be able to handle setting the variable entirely in c.opt,
>
> following the pattern of -fstrong-eval-order.  The variable is defined
>
> by the Var(...) notation in c.opt, and the opts code handles setting it.
>
>
>
> > On Tue, Jun 11, 2024 at 10:26 AM Jason Merrill <jason@redhat.com> wrote:
>
> >>
>
> >> 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
>
> >>>>>>
>
> >>>>>
>
> >>>>
>
> >>>
>
> >>
>
> >
>
>
>
>
>

Julian Waters <tanksherman27@gmail.com> writes:

> Darn, not even sending it from my friend's mail account worked to combat the patch formatting issues. Not only is the
> patch a complete wreck on the mailing list archive, but my friend's mail account has now somehow merged into mine on the
> list, resulting in a truly confusing author display name. I really hope you've somehow gotten the patch intact on your
> end...

Just use `git send-email` and nothing else, and make sure to use
plain-text emails when writing anything in your mail client (not HTML).

>
> best regards,
> Julian
> [...]

Attempting to resend with a different client, plain text enabled...

Hi Jason,

I’ve managed to address your review comments, and rewrite the patch to
include both -Winvalid-noreturn and -Winvalid-noreturn= but have trouble
figuring out how to format invoke.texi and where to add the documentation
for the new warning options. I’ve also made this a Common option, since
it’s come to my attention that the warning is not specific to c-family, but
is also used by other languages too (See tree-cfg.cc). Here’s the current
version of the patch, hope it’s good to go this time

best regards,
Julian

From fe1b4d5747e05101410b6bb6db9430362e3977d9 Mon Sep 17 00:00:00 2001
From: Julian Waters <tanksherman27@gmail.com>
Date: Fri, 19 Jul 2024 11:22:38 +0800
Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with extra
 tuneability

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, on top of the
regular -Winvalid-noreturn. This patch adds both to gcc, for compatibility
with clang.

gcc/ChangeLog:

        * common.opt: Introduce -Winvalid-noreturn and -Winvalid-noreturn=
        * opts.cc (common_handle_option): Handle new option
        * flag-types.h: New flags for -Winvalid-noreturn=
        * 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

Signed-off-by: Julian Waters <tanksherman27@gmail.com>
---
 gcc/c/c-typeck.cc          |  4 ++--
 gcc/c/gimple-parser.cc     |  4 ++--
 gcc/common.opt             | 13 +++++++++++++
 gcc/config/mingw/mingw32.h |  6 +++---
 gcc/cp/coroutines.cc       |  4 ++--
 gcc/cp/typeck.cc           |  4 ++--
 gcc/flag-types.h           |  7 +++++++
 gcc/opts.cc                | 19 +++++++++++++++++++
 gcc/tree-cfg.cc            |  3 ++-
 9 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 7e0f01ed22b..0f8c8cfff2e 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -11738,8 +11738,8 @@ 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,
+  if (TREE_THIS_VOLATILE (current_function_decl) && flag_invalid_noreturn
!= invalid_noreturn_kind::EXPLICIT)
+    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 d156d83cd37..8a684fe334a 100644
--- a/gcc/c/gimple-parser.cc
+++ b/gcc/c/gimple-parser.cc
@@ -2592,8 +2592,8 @@ 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,
+  if (TREE_THIS_VOLATILE (current_function_decl) && flag_invalid_noreturn
!= invalid_noreturn_kind::EXPLICIT)
+    warning_at (xloc, OPT_Winvalid_noreturn,
  "function declared %<noreturn%> has a %<return%> statement");

   if (! retval)
diff --git a/gcc/common.opt b/gcc/common.opt
index ea39f87ae71..d44ff0231c3 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -94,6 +94,11 @@ void *flag_instrument_functions_exclude_files
 Variable
 void *flag_ignored_attributes

+; The noreturn kind to ignore when warning for noreturn code that
+; does return.
+Variable
+invalid_noreturn_kind flag_invalid_noreturn = invalid_noreturn_kind::NONE
+
 ; Generic structs (e.g. templates not explicitly specialized)
 ; may not have a compilation unit associated with them, and so
 ; may need to be treated differently from ordinary structs.
@@ -667,6 +672,14 @@ Winvalid-memory-model
 Common Var(warn_invalid_memory_model) Init(1) Warning
 Warn when an atomic memory model parameter is known to be outside the
valid range.

+Winvalid-noreturn
+Common Warning
+Warn when code marked noreturn returns implicitly and/or explicitly.
+
+Winvalid-noreturn=
+Common Joined Warning
+Warn when code marked noreturn returns in the manner specified.
+
 Wlarger-than-
 Common RejectNegative Joined Warning Undocumented Alias(Wlarger-than=)

diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h
index 0dfe8e995b6..48eadfa2c2c 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 f350fc33e9b..f49ff82b1ee 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1374,8 +1374,8 @@ 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,
+  if (TREE_THIS_VOLATILE (current_function_decl) && flag_invalid_noreturn
!= invalid_noreturn_kind::EXPLICIT)
+    warning_at (kw, OPT_Winvalid_noreturn,
  "function declared %<noreturn%> has a"
  " %<co_return%> statement");

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 8df8b871676..260a9f99b08 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -11080,8 +11080,8 @@ 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) && flag_invalid_noreturn
!= invalid_noreturn_kind::EXPLICIT)
+    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/flag-types.h b/gcc/flag-types.h
index 1e497f0bb91..591acd2a7d2 100644
--- a/gcc/flag-types.h
+++ b/gcc/flag-types.h
@@ -375,6 +375,13 @@ enum incremental_link {
   INCREMENTAL_LINK_LTO
 };

+/* Kind of noreturn to ignore when reporting invalid noreturns  */
+enum class invalid_noreturn_kind {
+  NONE,
+  IMPLICIT,
+  EXPLICIT
+};
+
 /* Different trace modes.  */
 enum sanitize_coverage_code {
   /* Trace PC.  */
diff --git a/gcc/opts.cc b/gcc/opts.cc
index be90a632338..19d31c262d7 100644
--- a/gcc/opts.cc
+++ b/gcc/opts.cc
@@ -2860,6 +2860,25 @@ common_handle_option (struct gcc_options *opts,
       dc->m_fatal_errors = value;
       break;

+    case OPT_Winvalid_noreturn_:
+      if (lang_mask == CL_DRIVER)
+ break;
+
+      if (value)
+ warning (0, "%<-Winvalid-noreturn=%> is enabled by default, ignoring");
+      else if (strcmp (arg, "explicit"))
+ {
+   opts->x_flag_invalid_noreturn = invalid_noreturn_kind::EXPLICIT;
+ }
+      else if (strcmp (arg, "implicit"))
+ {
+   opts->x_flag_invalid_noreturn = invalid_noreturn_kind::IMPLICIT;
+ }
+      else
+ error_at (loc, "Unknown option for %<-Winvalid-noreturn=%>");
+
+      break;
+
     case OPT_Wstack_usage_:
       opts->x_flag_stack_usage_info = value != -1;
       break;
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index e6fd1294b95..f28dabc2bbf 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -9820,7 +9820,8 @@ 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 (flag_invalid_noreturn != invalid_noreturn_kind::IMPLICIT)
+ warning_at (location, 0, "%<noreturn%> function does return");
     }

   /* If we see "return;" in some basic block, then we do reach the end

On 7/19/24 2:29 AM, Julian Waters wrote:
> Attempting to resend with a different client, plain text enabled...

Still corrupted by word wrap and quoted-printable encoding, 
unfortunately.  Probably the easiest solution is to send the patch as an 
attachment rather than pasted into the message.

> I’ve managed to address your review comments, and rewrite the patch to 
> include both -Winvalid-noreturn and -Winvalid-noreturn=

Hmm, I keep suggesting that you follow the pattern of 
-fstrong-eval-order, and this patch moves away from that pattern without 
any discussion.  Could you say something about why you decided not to?

> but have trouble 
> figuring out how to format invoke.texi and where to add the 
> documentation for the new warning options.

For documentation, I don't see any proposed change, so I'm not sure 
where to start on formatting advice.  I would expect the documentation 
to go in alphabetical order under @node Warning Options.

> I’ve also made this a Common 
> option, since it’s come to my attention that the warning is not specific 
> to c-family, but is also used by other languages too (See tree-cfg.cc). 

Sounds good.

> gcc/ChangeLog:
> 
>          * common.opt: Introduce -Winvalid-noreturn and -Winvalid-noreturn=
>          * opts.cc (common_handle_option): Handle new option
>          * flag-types.h: New flags for -Winvalid-noreturn=
>          * tree-cfg.cc (pass_warn_function_return::execute): Use it

Conventionally, change descriptions should end with a period.

> gcc/config/mingw/ChangeLog:
> 
>          * mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons

This change should not be part of this patch.  It also seems unnecessary 
to me, but you can propose it separately if you want.

> @@ -667,6 +672,14 @@ Winvalid-memory-model
>   Common Var(warn_invalid_memory_model) Init(1) Warning
>   Warn when an atomic memory model parameter is known to be outside the 
> valid range.
> +Winvalid-noreturn
> +Common Warning
> +Warn when code marked noreturn returns implicitly and/or explicitly.
> +
> +Winvalid-noreturn=
> +Common Joined Warning

This should have RejectNegative.

> diff --git a/gcc/flag-types.h b/gcc/flag-types.h
> index 1e497f0bb91..591acd2a7d2 100644
> --- a/gcc/flag-types.h
> +++ b/gcc/flag-types.h
> @@ -375,6 +375,13 @@ enum incremental_link {
>     INCREMENTAL_LINK_LTO
>   };
> +/* Kind of noreturn to ignore when reporting invalid noreturns  */

Missing period.

> +enum class invalid_noreturn_kind {
> +  NONE,
> +  IMPLICIT,
> +  EXPLICIT
> +};

The inverse sense of this enum is confusing, since it isn't reflected in 
the name.  When I see "invalid_noreturn_kind" I think it expresses which 
category to warn about.  Can we reverse it?

>   /* Different trace modes.  */
>   enum sanitize_coverage_code {
>     /* Trace PC.  */
> diff --git a/gcc/opts.cc b/gcc/opts.cc
> index be90a632338..19d31c262d7 100644
> --- a/gcc/opts.cc
> +++ b/gcc/opts.cc
> @@ -2860,6 +2860,25 @@ common_handle_option (struct gcc_options *opts,
>         dc->m_fatal_errors = value;
>         break;
> +    case OPT_Winvalid_noreturn_:
> +      if (lang_mask == CL_DRIVER)
> +break;
> +
> +      if (value)
> +warning (0, "%<-Winvalid-noreturn=%> is enabled by default, ignoring");

We don't in general warn about redundant flags in general.  They might 
be intended to override e.g. a previous -Wno-invalid-noreturn.

> +      else if (strcmp (arg, "explicit"))
> +{
> +  opts->x_flag_invalid_noreturn = invalid_noreturn_kind::EXPLICIT;

If I write -Winvalid-noreturn=explicit, I would expect to get warnings 
about explicit returns, but as discussed above this seems to mean 
ignoring explicit returns.

> +}
> +      else if (strcmp (arg, "implicit"))
> +{
> +  opts->x_flag_invalid_noreturn = invalid_noreturn_kind::IMPLICIT;
> +}
> +      else
> +error_at (loc, "Unknown option for %<-Winvalid-noreturn=%>");

Seems like we also need options for warning about all or none.

I see no handling for -W{no-,}invalid-noreturn; the if (value) above 
won't handle it because those options don't hit this case; you would 
need a separate case OPT_Winvalid_noreturn: or use the Alias property in 
the common.opt entry.

You also need testcases for all the different option forms and their 
effects.

Jason

HI Jason,

I was hoping to have -Wno-invalid-noreturn=explicit directly disable
explicit noreturn warnings for instance, following the style of the
-Wno-attributes=vendor:: option, while the pattern in
-fstrong-eval-order seems to be that of using RejectNegative and
handling each specified positive case rather than disabling the
unwanted case, which would mean one would have to write
-Winvalid-noreturn=explicit to avoid implicit noreturns, which at
least I found to be less clear. If there was a way to only allow the
-Wno form of a warning option, I feel it would be simpler to implement
this (A positive form like -Winvalid-noreturn=explicit wouldn't make
much sense since gcc already warns for it even without the option, in
my opinion), but I don't think there's a RejectPositive variant of
RejectNegative, unfortunately. I can change it to be more like
-fstrong-eval-order if needed, but I feel that is less clear than just
allowing the user to disable the warning variant directly

I'm a little unsure about the noreturn warnings being a Common warning
now actually, any warnings implemented under the gcc directory and not
under c/c-family/cp is a Common warning right? Just need some
confirmation to be sure

I'll revert the semicolon fix for mingw and propose that separately,
but I'll leave further discussion about the other review comments
raised for this patch for a later time, since it seems to me that I'll
be rewriting the patch again and possibly removing some code that was
reviewed, so I'll save it for when the patch is more stable

best regards,
Julian