* [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability @ 2024-05-29 13:58 Julian Waters 2024-05-31 20:57 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: Julian Waters @ 2024-05-29 13:58 UTC (permalink / raw) To: gcc-patches Currently, gcc warns about noreturn marked functions that return both explicitly and implicitly, with no way to turn this warning off. clang does have an option for these classes of warnings, -Winvalid-noreturn. However, we can do better. Instead of just having 1 option that switches the warnings for both on and off, we can define an extra layer of granularity, and have a separate options for implicit returns and explicit returns, as in -Winvalid-return=explicit and -Winvalid-noreturn=implicit. This patch adds both to gcc, for compatibility with clang. Do note that I am relatively new to gcc's codebase, and as such couldn't figure out how to cleanly define a general -Winvalid-noreturn warning that switch both on and off, for better compatibility with clang. If someone should point out how to do so, I'll happily rewrite my patch. I also do not have write access to gcc, and will need help pushing this patch once the green light is given best regards, Julian gcc/c-family/ChangeLog: * c.opt: Introduce -Winvalid-noreturn=explicit and -Winvalid-noreturn=implicit gcc/ChangeLog: * tree-cfg.cc (pass_warn_function_return::execute): Use it gcc/c/ChangeLog: * c-typeck.cc (c_finish_return): Use it * gimple-parser.cc (c_finish_gimple_return): Use it gcc/config/mingw/ChangeLog: * mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons gcc/cp/ChangeLog: * coroutines.cc (finish_co_return_stmt): Use it * typeck.cc (check_return_expr): Use it gcc/doc/ChangeLog: * invoke.texi: Document new options From 4daf884f8bbc1e318ba93121a6fdf4139da80b64 Mon Sep 17 00:00:00 2001 From: TheShermanTanker <tanksherman27@gmail.com> Date: Wed, 29 May 2024 21:32:08 +0800 Subject: [PATCH] Introduce the -Winvalid-noreturn flag from clang with extra tuneability Signed-off-by: TheShermanTanker <tanksherman27@gmail.com> --- gcc/c-family/c.opt | 8 ++++++++ gcc/c/c-typeck.cc | 2 +- gcc/c/gimple-parser.cc | 2 +- gcc/config/mingw/mingw32.h | 6 +++--- gcc/cp/coroutines.cc | 2 +- gcc/cp/typeck.cc | 2 +- gcc/doc/invoke.texi | 13 +++++++++++++ gcc/tree-cfg.cc | 2 +- 8 files changed, 29 insertions(+), 8 deletions(-) diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index fb34c3b7031..32a2859fdcc 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -886,6 +886,14 @@ Winvalid-constexpr C++ ObjC++ Var(warn_invalid_constexpr) Init(-1) Warning Warn when a function never produces a constant expression. +Winvalid-noreturn=explicit +C ObjC C++ ObjC++ Warning +Warn when a function marked noreturn returns explicitly. + +Winvalid-noreturn=implicit +C ObjC C++ ObjC++ Warning +Warn when a function marked noreturn returns implicitly. + Winvalid-offsetof C++ ObjC++ Var(warn_invalid_offsetof) Init(1) Warning Warn about invalid uses of the \"offsetof\" macro. diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index ad4c7add562..1941fbc44cb 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -11468,7 +11468,7 @@ c_finish_return (location_t loc, tree retval, tree origtype) location_t xloc = expansion_point_location_if_in_system_header (loc); if (TREE_THIS_VOLATILE (current_function_decl)) - warning_at (xloc, 0, + warning_at (xloc, OPT_Winvalid_noreturn_explicit, "function declared %<noreturn%> has a %<return%> statement"); if (retval) diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc index d156d83cd37..1acaf75f844 100644 --- a/gcc/c/gimple-parser.cc +++ b/gcc/c/gimple-parser.cc @@ -2593,7 +2593,7 @@ c_finish_gimple_return (location_t loc, tree retval) location_t xloc = expansion_point_location_if_in_system_header (loc); if (TREE_THIS_VOLATILE (current_function_decl)) - warning_at (xloc, 0, + warning_at (xloc, OPT_Winvalid_noreturn_explicit, "function declared %<noreturn%> has a %<return%> statement"); if (! retval) diff --git a/gcc/config/mingw/mingw32.h b/gcc/config/mingw/mingw32.h index fa6e307476c..a69926133b1 100644 --- a/gcc/config/mingw/mingw32.h +++ b/gcc/config/mingw/mingw32.h @@ -35,9 +35,9 @@ along with GCC; see the file COPYING3. If not see | MASK_MS_BITFIELD_LAYOUT) #ifdef TARGET_USING_MCFGTHREAD -#define DEFINE_THREAD_MODEL builtin_define ("__USING_MCFGTHREAD__"); +#define DEFINE_THREAD_MODEL builtin_define ("__USING_MCFGTHREAD__") #elif defined(TARGET_USE_PTHREAD_BY_DEFAULT) -#define DEFINE_THREAD_MODEL builtin_define ("__USING_POSIXTHREAD__"); +#define DEFINE_THREAD_MODEL builtin_define ("__USING_POSIXTHREAD__") #else #define DEFINE_THREAD_MODEL #endif @@ -60,7 +60,7 @@ along with GCC; see the file COPYING3. If not see builtin_define_std ("WIN64"); \ builtin_define ("_WIN64"); \ } \ - DEFINE_THREAD_MODEL \ + DEFINE_THREAD_MODEL; \ } \ while (0) diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 97bc211ff67..53e56bd4959 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -1375,7 +1375,7 @@ finish_co_return_stmt (location_t kw, tree expr) /* Makes no sense for a co-routine really. */ if (TREE_THIS_VOLATILE (current_function_decl)) - warning_at (kw, 0, + warning_at (kw, OPT_Winvalid_noreturn_explicit, "function declared %<noreturn%> has a" " %<co_return%> statement"); diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 1b7a31d32f3..74cc59bfb87 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -11062,7 +11062,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling) (This is a G++ extension, used to get better code for functions that call the `volatile' function.) */ if (TREE_THIS_VOLATILE (current_function_decl)) - warning (0, "function declared %<noreturn%> has a %<return%> statement"); + warning (OPT_Winvalid_noreturn_explicit, "function declared %<noreturn%> has a %<return%> statement"); /* Check for various simple errors. */ if (DECL_DESTRUCTOR_P (current_function_decl)) diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 2cba380718b..27d880fd4f0 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -376,6 +376,7 @@ Objective-C and Objective-C++ Dialects}. -Winfinite-recursion -Winit-self -Winline -Wno-int-conversion -Wint-in-bool-context -Wno-int-to-pointer-cast -Wno-invalid-memory-model +-Winvalid-noreturn=explicit -Winvalid-noreturn=implicit -Winvalid-pch -Winvalid-utf8 -Wno-unicode -Wjump-misses-init -Wlarger-than=@var{byte-size} -Wlogical-not-parentheses -Wlogical-op -Wlong-long -Wno-lto-type-mismatch -Wmain -Wmaybe-uninitialized @@ -10482,6 +10483,18 @@ an error. @option{Wint-to-pointer-cast} is enabled by default. Suppress warnings from casts from a pointer to an integer type of a different size. +@opindex Winvalid-noreturn=explicit +@opindex Wno-invalid-noreturn=explicit +@item -Winvalid-noreturn=explicit +Warn if a function marked noreturn returns explicitly, that is, it +contains a return statement. + +@opindex Winvalid-noreturn=implicit +@opindex Wno-invalid-noreturn=implicit +@item -Winvalid-noreturn=implicit +Warn if a function marked noreturn returns implicitly, that is, it +has a path in its control flow that can reach the end of its body. + @opindex Winvalid-pch @opindex Wno-invalid-pch @item -Winvalid-pch diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc index 7fb7b92966b..cfe91038dd1 100644 --- a/gcc/tree-cfg.cc +++ b/gcc/tree-cfg.cc @@ -9817,7 +9817,7 @@ pass_warn_function_return::execute (function *fun) } if (location == UNKNOWN_LOCATION) location = cfun->function_end_locus; - warning_at (location, 0, "%<noreturn%> function does return"); + warning_at (location, OPT_Winvalid_noreturn_implicit, "%<noreturn%> function does return"); } /* If we see "return;" in some basic block, then we do reach the end -- 2.41.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability 2024-05-29 13:58 [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability Julian Waters @ 2024-05-31 20:57 ` Jason Merrill 2024-06-01 15:31 ` Julian Waters 0 siblings, 1 reply; 7+ messages in thread From: Jason Merrill @ 2024-05-31 20:57 UTC (permalink / raw) To: Julian Waters, gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability 2024-05-31 20:57 ` Jason Merrill @ 2024-06-01 15:31 ` Julian Waters 2024-06-01 18:18 ` Eric Gallager 2024-06-03 17:04 ` Jason Merrill 0 siblings, 2 replies; 7+ messages in thread From: Julian Waters @ 2024-06-01 15:31 UTC (permalink / raw) To: Jason Merrill, gcc-patches 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability 2024-06-01 15:31 ` Julian Waters @ 2024-06-01 18:18 ` Eric Gallager 2024-06-03 17:04 ` Jason Merrill 1 sibling, 0 replies; 7+ messages in thread From: Eric Gallager @ 2024-06-01 18:18 UTC (permalink / raw) To: Julian Waters; +Cc: Jason Merrill, gcc-patches 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability 2024-06-01 15:31 ` Julian Waters 2024-06-01 18:18 ` Eric Gallager @ 2024-06-03 17:04 ` Jason Merrill 2024-06-10 7:13 ` Julian Waters 1 sibling, 1 reply; 7+ messages in thread From: Jason Merrill @ 2024-06-03 17:04 UTC (permalink / raw) To: Julian Waters, gcc-patches 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 >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability 2024-06-03 17:04 ` Jason Merrill @ 2024-06-10 7:13 ` Julian Waters 2024-06-11 2:26 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: Julian Waters @ 2024-06-10 7:13 UTC (permalink / raw) To: Jason Merrill, gcc-patches 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 > >> > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability 2024-06-10 7:13 ` Julian Waters @ 2024-06-11 2:26 ` Jason Merrill 0 siblings, 0 replies; 7+ messages in thread From: Jason Merrill @ 2024-06-11 2:26 UTC (permalink / raw) To: Julian Waters, gcc-patches 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 >>>> >>> >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-06-11 2:26 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-05-29 13:58 [PATCH] c-family: Introduce the -Winvalid-noreturn flag from clang with extra tuneability Julian Waters 2024-05-31 20:57 ` Jason Merrill 2024-06-01 15:31 ` Julian Waters 2024-06-01 18:18 ` Eric Gallager 2024-06-03 17:04 ` Jason Merrill 2024-06-10 7:13 ` Julian Waters 2024-06-11 2:26 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).