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 <ja...@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 <ja...@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 <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







Reply via email to