On Sat, Jun 1, 2024 at 11:32 AM Julian Waters <tanksherma...@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 <ja...@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 <tanksherma...@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 <tanksherma...@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 > >
