On 7/19/24 2:29 AM, Julian Waters wrote:
Attempting to resend with a different client, plain text enabled...
Still corrupted by word wrap and quoted-printable encoding,
unfortunately. Probably the easiest solution is to send the patch as an
attachment rather than pasted into the message.
I’ve managed to address your review comments, and rewrite the patch to
include both -Winvalid-noreturn and -Winvalid-noreturn=
Hmm, I keep suggesting that you follow the pattern of
-fstrong-eval-order, and this patch moves away from that pattern without
any discussion. Could you say something about why you decided not to?
but have trouble
figuring out how to format invoke.texi and where to add the
documentation for the new warning options.
For documentation, I don't see any proposed change, so I'm not sure
where to start on formatting advice. I would expect the documentation
to go in alphabetical order under @node Warning Options.
I’ve also made this a Common
option, since it’s come to my attention that the warning is not specific
to c-family, but is also used by other languages too (See tree-cfg.cc).
Sounds good.
gcc/ChangeLog:
* common.opt: Introduce -Winvalid-noreturn and -Winvalid-noreturn=
* opts.cc (common_handle_option): Handle new option
* flag-types.h: New flags for -Winvalid-noreturn=
* tree-cfg.cc (pass_warn_function_return::execute): Use it
Conventionally, change descriptions should end with a period.
gcc/config/mingw/ChangeLog:
* mingw32.h (EXTRA_OS_CPP_BUILTINS): Fix semicolons
This change should not be part of this patch. It also seems unnecessary
to me, but you can propose it separately if you want.
@@ -667,6 +672,14 @@ Winvalid-memory-model
Common Var(warn_invalid_memory_model) Init(1) Warning
Warn when an atomic memory model parameter is known to be outside the
valid range.
+Winvalid-noreturn
+Common Warning
+Warn when code marked noreturn returns implicitly and/or explicitly.
+
+Winvalid-noreturn=
+Common Joined Warning
This should have RejectNegative.
diff --git a/gcc/flag-types.h b/gcc/flag-types.h
index 1e497f0bb91..591acd2a7d2 100644
--- a/gcc/flag-types.h
+++ b/gcc/flag-types.h
@@ -375,6 +375,13 @@ enum incremental_link {
INCREMENTAL_LINK_LTO
};
+/* Kind of noreturn to ignore when reporting invalid noreturns */
Missing period.
+enum class invalid_noreturn_kind {
+ NONE,
+ IMPLICIT,
+ EXPLICIT
+};
The inverse sense of this enum is confusing, since it isn't reflected in
the name. When I see "invalid_noreturn_kind" I think it expresses which
category to warn about. Can we reverse it?
/* Different trace modes. */
enum sanitize_coverage_code {
/* Trace PC. */
diff --git a/gcc/opts.cc b/gcc/opts.cc
index be90a632338..19d31c262d7 100644
--- a/gcc/opts.cc
+++ b/gcc/opts.cc
@@ -2860,6 +2860,25 @@ common_handle_option (struct gcc_options *opts,
dc->m_fatal_errors = value;
break;
+ case OPT_Winvalid_noreturn_:
+ if (lang_mask == CL_DRIVER)
+break;
+
+ if (value)
+warning (0, "%<-Winvalid-noreturn=%> is enabled by default, ignoring");
We don't in general warn about redundant flags in general. They might
be intended to override e.g. a previous -Wno-invalid-noreturn.
+ else if (strcmp (arg, "explicit"))
+{
+ opts->x_flag_invalid_noreturn = invalid_noreturn_kind::EXPLICIT;
If I write -Winvalid-noreturn=explicit, I would expect to get warnings
about explicit returns, but as discussed above this seems to mean
ignoring explicit returns.
+}
+ else if (strcmp (arg, "implicit"))
+{
+ opts->x_flag_invalid_noreturn = invalid_noreturn_kind::IMPLICIT;
+}
+ else
+error_at (loc, "Unknown option for %<-Winvalid-noreturn=%>");
Seems like we also need options for warning about all or none.
I see no handling for -W{no-,}invalid-noreturn; the if (value) above
won't handle it because those options don't hit this case; you would
need a separate case OPT_Winvalid_noreturn: or use the Alias property in
the common.opt entry.
You also need testcases for all the different option forms and their
effects.
Jason
