| Message ID | e7ee58c567233aab1b36b9d09f79af6d0108a98b.1699879818.git.fweimer@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Turn some C warnings into errors by default | expand |
On 11/13/23 06:09, Florian Weimer wrote: > It turns out that permerror_opt is not directly usable for > -fpermissive in the C front end. The front end uses pedwarn > extensively, and pedwarns are not overridable by -Wno-* options, > yet permerrors are. Add new pedpermerror helpers that turn into > pedwarns if -pedantic-errors is active. > > Due to the dependency on flag_pedantic_errors, the new helpers > are specific to the C-family front ends. For implementing the > rich location variant, export emit_diagnostic_valist from > gcc/diagnostic.cc in parallel to its location_t variant. > > gcc/ > > * diagnostic-core.h (emit_diagnostic_valist): Declare function. > * diagnostic.cc (emit_diagnostic_valist): Define it. > > gcc/c-family/ > > * c-common.h (pedpermerror): Declare functions. > * c-warn.cc (pedpermerror): Define them. OK jeff
* Florian Weimer: > It turns out that permerror_opt is not directly usable for > -fpermissive in the C front end. The front end uses pedwarn > extensively, and pedwarns are not overridable by -Wno-* options, > yet permerrors are. Add new pedpermerror helpers that turn into > pedwarns if -pedantic-errors is active. > > Due to the dependency on flag_pedantic_errors, the new helpers > are specific to the C-family front ends. For implementing the > rich location variant, export emit_diagnostic_valist from > gcc/diagnostic.cc in parallel to its location_t variant. > > gcc/ > > * diagnostic-core.h (emit_diagnostic_valist): Declare function. > * diagnostic.cc (emit_diagnostic_valist): Define it. > > gcc/c-family/ > > * c-common.h (pedpermerror): Declare functions. > * c-warn.cc (pedpermerror): Define them. Jason suggested off-list that this shouldn't be necessary, and the description of -pedantic-errors is wrong (it is possible to undo -pedantic-errors effects with -Wno-error=…). The permerror_opt interface should already do what I need. It turns out that I was very unlucky and picked -Wreturn-type for my tests. This: long i = "abc"; volatile j; int f (void) { return; } Gives, with GCC 13: $ gcc -pedantic-errors -Wno-error=implicit-int -Wno-error=int-conversion -Wno-error=return-type test.c test.c:1:10: warning: initialization of ‘long int’ from ‘char *’ makes integer from pointer without a cast [-Wint-conversion] 1 | long i = "abc"; | ^~~~~ test.c:2:10: warning: type defaults to ‘int’ in declaration of ‘j’ [-Wimplicit-int] 2 | volatile j; | ^ test.c: In function ‘f’: test.c:3:16: error: ‘return’ with no value, in function returning non-void 3 | int f (void) { return; } | ^~~~~~ test.c:3:5: note: declared here 3 | int f (void) { return; } | ^ This happens because we drop the OPT_Wreturn_type in some cases: if (flag_isoc99) warned_here = pedwarn (loc, warn_return_type >= 0 ? OPT_Wreturn_type : 0, "%<return%> with no value, in function returning non-void"); else warned_here = warning_at (loc, OPT_Wreturn_type, "%<return%> with no value, in function returning non-void"); And for the other direction: if (TREE_CODE (TREE_TYPE (retval)) != VOID_TYPE) warned_here = pedwarn (xloc, warn_return_type >= 0 ? OPT_Wreturn_type : 0, "%<return%> with a value, in function returning void"); I think with the -Wreturn-mismatch split, we can drop the warn_return_type >= 0 condition, and then permerror_opt should indeed do the right thing. I'll write the kitchen sink test now, use that to verify this theory, and repost as appropriate. Thanks, Florian
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index b57e83d7c5d..789e0bf2459 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -1486,6 +1486,10 @@ extern void warn_for_address_or_pointer_of_packed_member (tree, tree); extern void warn_parm_array_mismatch (location_t, tree, tree); extern void maybe_warn_sizeof_array_div (location_t, tree, tree, tree, tree); extern void do_warn_array_compare (location_t, tree_code, tree, tree); +extern bool pedpermerror (location_t, int, const char *, + ...) ATTRIBUTE_GCC_DIAG(3,4); +extern bool pedpermerror (rich_location *, int, const char *, + ...) ATTRIBUTE_GCC_DIAG(3,4); /* Places where an lvalue, or modifiable lvalue, may be required. Used to select diagnostic messages in lvalue_error and diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc index b1bd8ba9f42..475a3e4b42e 100644 --- a/gcc/c-family/c-warn.cc +++ b/gcc/c-family/c-warn.cc @@ -3932,3 +3932,37 @@ check_for_xor_used_as_pow (location_t lhs_loc, tree lhs_val, lhs_uhwi, lhs_uhwi); } } + +/* If !flag_pedantic_errors, equivalent to permerror_opt, otherwise to + pedwarn. */ + +bool +pedpermerror (location_t location, int opt, const char *gmsgid, ...) +{ + auto_diagnostic_group d; + va_list ap; + va_start (ap, gmsgid); + rich_location richloc (line_table, location); + bool ret = emit_diagnostic_valist (flag_pedantic_errors + ? DK_PEDWARN : DK_PERMERROR, + location, opt, gmsgid, &ap); + va_end (ap); + return ret; +} + +/* Same as "pedpermerror" above, but at RICHLOC. */ + +bool +pedpermerror (rich_location *richloc, int opt, const char *gmsgid, ...) +{ + gcc_assert (richloc); + + auto_diagnostic_group d; + va_list ap; + va_start (ap, gmsgid); + bool ret = emit_diagnostic_valist (flag_pedantic_errors + ? DK_PEDWARN : DK_PERMERROR, + richloc, opt, gmsgid, &ap); + va_end (ap); + return ret; +} diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h index 04eba3d140e..eac775fb573 100644 --- a/gcc/diagnostic-core.h +++ b/gcc/diagnostic-core.h @@ -121,6 +121,9 @@ extern bool emit_diagnostic (diagnostic_t, location_t, int, const char *, ...) ATTRIBUTE_GCC_DIAG(4,5); extern bool emit_diagnostic (diagnostic_t, rich_location *, int, const char *, ...) ATTRIBUTE_GCC_DIAG(4,5); +extern bool emit_diagnostic_valist (diagnostic_t, rich_location *, int, + const char *, + va_list *) ATTRIBUTE_GCC_DIAG (4,0); extern bool emit_diagnostic_valist (diagnostic_t, location_t, int, const char *, va_list *) ATTRIBUTE_GCC_DIAG (4,0); extern bool seen_error (void); diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc index addd6606eaa..6e73343dbdb 100644 --- a/gcc/diagnostic.cc +++ b/gcc/diagnostic.cc @@ -1829,6 +1829,13 @@ emit_diagnostic_valist (diagnostic_t kind, location_t location, int opt, return diagnostic_impl (&richloc, NULL, opt, gmsgid, ap, kind); } +bool +emit_diagnostic_valist (diagnostic_t kind, rich_location *richloc, int opt, + const char *gmsgid, va_list *ap) +{ + return diagnostic_impl (richloc, NULL, opt, gmsgid, ap, kind); +} + /* An informative note at LOCATION. Use this for additional details on an error message. */ void