Re: [PATCH] Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)
On 12/07/2015 11:41 PM, Jakub Jelinek wrote: On Mon, Dec 07, 2015 at 04:11:48PM +0100, Bernd Schmidt wrote: Let's document arguments; for the ones identical to read_cmdline_option an explicit pointer there is sufficient, but errors is new. This also needs an update to the function comment. Other than that I'm ok with this. This area could probably be restructured a bit but for now I think this is good enough. So like this? Bootstrapped/regtested on x86_64-linux and i686-linux. Yes, thanks. Bernd
Re: [PATCH] Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)
On Mon, Dec 07, 2015 at 04:11:48PM +0100, Bernd Schmidt wrote: > Let's document arguments; for the ones identical to read_cmdline_option an > explicit pointer there is sufficient, but errors is new. > This also needs an update to the function comment. > > Other than that I'm ok with this. This area could probably be restructured a > bit but for now I think this is good enough. So like this? Bootstrapped/regtested on x86_64-linux and i686-linux. 2015-12-07 Jakub Jelinek PR c/48088 PR c/68657 * common.opt (Wframe-larger-than=): Add Warning. * opts.h (control_warning_option): Add ARG argument. * opts-common.c (cmdline_handle_error): New function. (read_cmdline_option): Use it. (control_warning_option): Likewise. Add ARG argument. If non-NULL, decode it if needed and pass through to handle_generated_option. Handle CLVC_ENUM like CLVC_BOOLEAN. * opts.c (common_handle_option): Adjust control_warning_option caller. (enable_warning_as_error): Likewise. c-family/ * c.opt (Wfloat-conversion, Wsign-conversion): Add Warning. * c-pragma.c (handle_pragma_diagnostic): Adjust control_warning_option caller. ada/ * gcc-interface/trans.c (Pragma_to_gnu): Adjust control_warning_option caller. testsuite/ * c-c++-common/pr68657-1.c: New test. * c-c++-common/pr68657-2.c: New test. * c-c++-common/pr68657-3.c: New test. * gcc.dg/cpp/warn-normalized-3.c: Use -Werror=normalized=nfc instead of -Werror=normalized= in dg-options. --- gcc/common.opt.jj 2015-12-06 12:20:38.496706114 +0100 +++ gcc/common.opt 2015-12-07 13:56:34.167539666 +0100 @@ -581,7 +581,7 @@ Common Var(flag_fatal_errors) Exit on the first error occurred. Wframe-larger-than= -Common RejectNegative Joined UInteger +Common RejectNegative Joined UInteger Warning -Wframe-larger-than= Warn if a function's stack frame requires more than bytes. Wfree-nonheap-object --- gcc/opts.h.jj 2015-12-04 18:55:34.150960576 +0100 +++ gcc/opts.h 2015-12-07 13:56:34.189539357 +0100 @@ -363,7 +363,7 @@ extern void read_cmdline_option (struct const struct cl_option_handlers *handlers, diagnostic_context *dc); extern void control_warning_option (unsigned int opt_index, int kind, - bool imply, location_t loc, + const char *arg, bool imply, location_t loc, unsigned int lang_mask, const struct cl_option_handlers *handlers, struct gcc_options *opts, --- gcc/opts-common.c.jj2015-12-04 18:55:34.097961330 +0100 +++ gcc/opts-common.c 2015-12-07 18:44:00.764807029 +0100 @@ -1021,62 +1021,42 @@ generate_option_input_file (const char * decoded->errors = 0; } -/* Handle the switch DECODED (location LOC) for the language indicated - by LANG_MASK, using the handlers in *HANDLERS and setting fields in - OPTS and OPTS_SET and using diagnostic context DC (if not NULL) for - diagnostic options. */ - -void -read_cmdline_option (struct gcc_options *opts, -struct gcc_options *opts_set, -struct cl_decoded_option *decoded, -location_t loc, -unsigned int lang_mask, -const struct cl_option_handlers *handlers, -diagnostic_context *dc) +/* Perform diagnostics for read_cmdline_option and control_warning_option + functions. Returns true if an error has been diagnosed. + LOC and LANG_MASK arguments like in read_cmdline_option. + OPTION is the option to report diagnostics for, OPT the name + of the option as text, ARG the argument of the option (for joined + options), ERRORS is bitmask of CL_ERR_* values. */ + +static bool +cmdline_handle_error (location_t loc, const struct cl_option *option, + const char *opt, const char *arg, int errors, + unsigned int lang_mask) { - const struct cl_option *option; - const char *opt = decoded->orig_option_with_args_text; - - if (decoded->warn_message) -warning_at (loc, 0, decoded->warn_message, opt); - - if (decoded->opt_index == OPT_SPECIAL_unknown) -{ - if (handlers->unknown_option_callback (decoded)) - error_at (loc, "unrecognized command line option %qs", decoded->arg); - return; -} - - if (decoded->opt_index == OPT_SPECIAL_ignore) -return; - - option = &cl_options[decoded->opt_index]; - - if (decoded->errors & CL_ERR_DISABLED) + if (errors & CL_ERR_DISABLED) { error_at (loc, "command line option %qs" - " is not supported by this configuration", opt); - return; +" is not supported by this configuration", opt); + return true; }
Re: [PATCH] Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)
On 12/07/2015 02:44 PM, Jakub Jelinek wrote: So like this? +/* Perform diagnostics for read_cmdline_option and control_warning_option + functions. Returns true if an error has been diagnosed. */ Let's document arguments; for the ones identical to read_cmdline_option an explicit pointer there is sufficient, but errors is new. @@ -1332,8 +1348,8 @@ get_option_state (struct gcc_options *op used by -Werror= and #pragma GCC diagnostic. */ void -control_warning_option (unsigned int opt_index, int kind, bool imply, - location_t loc, unsigned int lang_mask, +control_warning_option (unsigned int opt_index, int kind, const char *arg, + bool imply, location_t loc, unsigned int lang_mask, This also needs an update to the function comment. Other than that I'm ok with this. This area could probably be restructured a bit but for now I think this is good enough. Bernd
Re: [PATCH] Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)
On Mon, Dec 07, 2015 at 11:24:02AM +0100, Bernd Schmidt wrote: > On 12/04/2015 08:36 PM, Jakub Jelinek wrote: > >On Fri, Dec 04, 2015 at 06:19:19PM +, Manuel López-Ibáñez wrote: > >>My guess is that the first error_at should use arg instead of > >>option->opt_text to be equivalent. Of course, ideally, this code would > >>not be duplicated, but rather merged "somehow". > > > >Consider that fixed. As for duplication, as one operates on > >cl_decoded_option and the other not etc., this is harder, plus > >the missing and non-int cases are IMHO short enough that it is not worth > >trying hard to avoid the duplication. > >For the enum case which is larger, it is maybe worth adding > >a helper routine for it, which would need probably only > >location_t loc, const struct cl_enum *e, const char *opt, unsigned int > >lang_mask > >arguments. Can try that on Monday. > > Maybe you can split the error printing code out of read_cmdline_option. For > the original patch I noticed the duplication but figured it was not enough > to really worry about, but for the error handling I think we should make an > effort. So like this? 2015-12-07 Jakub Jelinek PR c/48088 PR c/68657 * common.opt (Wframe-larger-than=): Add Warning. * opts.h (control_warning_option): Add ARG argument. * opts-common.c (cmdline_handle_error): New function. (read_cmdline_option): Use it. (control_warning_option): Likewise. Add ARG argument. If non-NULL, decode it if needed and pass through to handle_generated_option. Handle CLVC_ENUM like CLVC_BOOLEAN. * opts.c (common_handle_option): Adjust control_warning_option caller. (enable_warning_as_error): Likewise. c-family/ * c.opt (Wfloat-conversion, Wsign-conversion): Add Warning. * c-pragma.c (handle_pragma_diagnostic): Adjust control_warning_option caller. ada/ * gcc-interface/trans.c (Pragma_to_gnu): Adjust control_warning_option caller. testsuite/ * c-c++-common/pr68657-1.c: New test. * c-c++-common/pr68657-2.c: New test. * c-c++-common/pr68657-3.c: New test. --- gcc/common.opt.jj 2015-12-06 12:20:38.496706114 +0100 +++ gcc/common.opt 2015-12-07 13:56:34.167539666 +0100 @@ -581,7 +581,7 @@ Common Var(flag_fatal_errors) Exit on the first error occurred. Wframe-larger-than= -Common RejectNegative Joined UInteger +Common RejectNegative Joined UInteger Warning -Wframe-larger-than= Warn if a function's stack frame requires more than bytes. Wfree-nonheap-object --- gcc/opts.h.jj 2015-12-04 18:55:34.150960576 +0100 +++ gcc/opts.h 2015-12-07 13:56:34.189539357 +0100 @@ -363,7 +363,7 @@ extern void read_cmdline_option (struct const struct cl_option_handlers *handlers, diagnostic_context *dc); extern void control_warning_option (unsigned int opt_index, int kind, - bool imply, location_t loc, + const char *arg, bool imply, location_t loc, unsigned int lang_mask, const struct cl_option_handlers *handlers, struct gcc_options *opts, --- gcc/opts-common.c.jj2015-12-04 18:55:34.097961330 +0100 +++ gcc/opts-common.c 2015-12-07 14:34:01.714160852 +0100 @@ -1021,62 +1021,38 @@ generate_option_input_file (const char * decoded->errors = 0; } -/* Handle the switch DECODED (location LOC) for the language indicated - by LANG_MASK, using the handlers in *HANDLERS and setting fields in - OPTS and OPTS_SET and using diagnostic context DC (if not NULL) for - diagnostic options. */ +/* Perform diagnostics for read_cmdline_option and control_warning_option + functions. Returns true if an error has been diagnosed. */ -void -read_cmdline_option (struct gcc_options *opts, -struct gcc_options *opts_set, -struct cl_decoded_option *decoded, -location_t loc, -unsigned int lang_mask, -const struct cl_option_handlers *handlers, -diagnostic_context *dc) +static bool +cmdline_handle_error (location_t loc, const struct cl_option *option, + const char *opt, const char *arg, int errors, + unsigned int lang_mask) { - const struct cl_option *option; - const char *opt = decoded->orig_option_with_args_text; - - if (decoded->warn_message) -warning_at (loc, 0, decoded->warn_message, opt); - - if (decoded->opt_index == OPT_SPECIAL_unknown) -{ - if (handlers->unknown_option_callback (decoded)) - error_at (loc, "unrecognized command line option %qs", decoded->arg); - return; -} - - if (decoded->opt_index == OPT_SPECIAL_ignore) -return; - - option = &cl_options[decoded->opt_index]; - - if
Re: [PATCH] Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)
On 12/04/2015 08:36 PM, Jakub Jelinek wrote: On Fri, Dec 04, 2015 at 06:19:19PM +, Manuel López-Ibáñez wrote: My guess is that the first error_at should use arg instead of option->opt_text to be equivalent. Of course, ideally, this code would not be duplicated, but rather merged "somehow". Consider that fixed. As for duplication, as one operates on cl_decoded_option and the other not etc., this is harder, plus the missing and non-int cases are IMHO short enough that it is not worth trying hard to avoid the duplication. For the enum case which is larger, it is maybe worth adding a helper routine for it, which would need probably only location_t loc, const struct cl_enum *e, const char *opt, unsigned int lang_mask arguments. Can try that on Monday. Maybe you can split the error printing code out of read_cmdline_option. For the original patch I noticed the duplication but figured it was not enough to really worry about, but for the error handling I think we should make an effort. Bernd
Re: [PATCH] Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)
On Fri, Dec 04, 2015 at 06:19:19PM +, Manuel López-Ibáñez wrote: > On 4 December 2015 at 17:53, Jakub Jelinek wrote: > > + > > + if (e->unknown_error) > > + error_at (loc, e->unknown_error, option->opt_text); > > + else > > + error_at (loc, "unrecognized argument in option %qs", > > + option->opt_text); > > The same code that handles command-line options has: > > if (e->unknown_error) > error_at (loc, e->unknown_error, decoded->arg); > else > error_at (loc, "unrecognized argument in option %qs", opt); > > My guess is that the first error_at should use arg instead of > option->opt_text to be equivalent. Of course, ideally, this code would > not be duplicated, but rather merged "somehow". Consider that fixed. As for duplication, as one operates on cl_decoded_option and the other not etc., this is harder, plus the missing and non-int cases are IMHO short enough that it is not worth trying hard to avoid the duplication. For the enum case which is larger, it is maybe worth adding a helper routine for it, which would need probably only location_t loc, const struct cl_enum *e, const char *opt, unsigned int lang_mask arguments. Can try that on Monday. Jakub
Re: [PATCH] Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)
On 4 December 2015 at 17:53, Jakub Jelinek wrote: > + > + if (e->unknown_error) > + error_at (loc, e->unknown_error, option->opt_text); > + else > + error_at (loc, "unrecognized argument in option %qs", > + option->opt_text); The same code that handles command-line options has: if (e->unknown_error) error_at (loc, e->unknown_error, decoded->arg); else error_at (loc, "unrecognized argument in option %qs", opt); My guess is that the first error_at should use arg instead of option->opt_text to be equivalent. Of course, ideally, this code would not be duplicated, but rather merged "somehow". Cheers, Manuel.
Re: [PATCH] Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)
On Fri, Dec 04, 2015 at 06:01:58PM +0100, Bernd Schmidt wrote: > I think marking stuff with Warning as appropriate qualifies as obvious. > > On 12/04/2015 05:37 PM, Jakub Jelinek wrote: > >+ /* If the switch takes an integer, convert it. */ > >+ if (arg && cl_options[opt_index].cl_uinteger) > >+{ > >+ value = integral_argument (arg); > >+ if (value == -1) > >+return; > >+} > > So does this issue an error message anywhere or just silently drop the > option on the floor if the argument is invalid? Silently accepted. Following updated patch accepts e.g.: -Wlarger-than=5 -Werror=larger-than=5 -Wnormalized=none -Werror=normalized=none and rejects (with the same diagnostics between -WXXX=YYY and -Werror=XXX=YYY): -Wlarger-than=none -Werror=larger-than=none -Wlarger-than= -Werror=larger-than= -Wnormalized=all -Werror=normalized=all -Wnormalized= -Werror=normalized= (as examples of Warning UInteger Joined and Warning Enum Joined options). > >+ /* If the switch takes an enumerated argument, convert it. */ > >+ if (arg && (cl_options[opt_index].var_type == CLVC_ENUM)) > > Unnecessary parens. Fixed. 2015-12-04 Jakub Jelinek PR c/48088 PR c/68657 * common.opt (Wframe-larger-than=): Add Warning. * opts.h (control_warning_option): Add ARG argument. * opts-common.c (control_warning_option): Likewise. If non-NULL, decode it if needed and pass through to handle_generated_option. Handle CLVC_ENUM like CLVC_BOOLEAN. * opts.c (common_handle_option): Adjust control_warning_option caller. (enable_warning_as_error): Likewise. c-family/ * c.opt (Wfloat-conversion, Wsign-conversion): Add Warning. * c-pragma.c (handle_pragma_diagnostic): Adjust control_warning_option caller. ada/ * gcc-interface/trans.c (Pragma_to_gnu): Adjust control_warning_option caller. testsuite/ * c-c++-common/pr68657-1.c: New test. * c-c++-common/pr68657-2.c: New test. * c-c++-common/pr68657-3.c: New test. --- gcc/common.opt.jj 2015-12-04 17:19:01.873180339 +0100 +++ gcc/common.opt 2015-12-04 18:07:31.901544973 +0100 @@ -576,7 +576,7 @@ Common Var(flag_fatal_errors) Exit on the first error occurred. Wframe-larger-than= -Common RejectNegative Joined UInteger +Common RejectNegative Joined UInteger Warning -Wframe-larger-than= Warn if a function's stack frame requires more than bytes. Wfree-nonheap-object --- gcc/opts.h.jj 2015-12-04 17:19:01.939179455 +0100 +++ gcc/opts.h 2015-12-04 18:07:31.901544973 +0100 @@ -363,7 +363,7 @@ extern void read_cmdline_option (struct const struct cl_option_handlers *handlers, diagnostic_context *dc); extern void control_warning_option (unsigned int opt_index, int kind, - bool imply, location_t loc, + const char *arg, bool imply, location_t loc, unsigned int lang_mask, const struct cl_option_handlers *handlers, struct gcc_options *opts, --- gcc/opts-common.c.jj2015-12-04 17:19:01.854180594 +0100 +++ gcc/opts-common.c 2015-12-04 18:44:15.956606061 +0100 @@ -1332,8 +1332,8 @@ get_option_state (struct gcc_options *op used by -Werror= and #pragma GCC diagnostic. */ void -control_warning_option (unsigned int opt_index, int kind, bool imply, - location_t loc, unsigned int lang_mask, +control_warning_option (unsigned int opt_index, int kind, const char *arg, + bool imply, location_t loc, unsigned int lang_mask, const struct cl_option_handlers *handlers, struct gcc_options *opts, struct gcc_options *opts_set, @@ -1347,10 +1347,89 @@ control_warning_option (unsigned int opt diagnostic_classify_diagnostic (dc, opt_index, (diagnostic_t) kind, loc); if (imply) { + const struct cl_option *option = &cl_options[opt_index]; + /* -Werror=foo implies -Wfoo. */ - if (cl_options[opt_index].var_type == CLVC_BOOLEAN) - handle_generated_option (opts, opts_set, -opt_index, NULL, 1, lang_mask, -kind, loc, handlers, dc); + if (option->var_type == CLVC_BOOLEAN || option->var_type == CLVC_ENUM) + { + int value = 1; + + if (arg && *arg == '\0' && !option->cl_missing_ok) + arg = NULL; + + if ((option->flags & CL_JOINED) && arg == NULL) + { + if (option->missing_argument_error) + error_at (loc, option->missing_argument_error, + option->opt_text); + else + error_at (loc, "missing argument to %qs", option->op
Re: [PATCH] Fix -Werror= handling for Joined warnings, add a few missing Warning keywords (PRs c/48088, c/68657)
I think marking stuff with Warning as appropriate qualifies as obvious. On 12/04/2015 05:37 PM, Jakub Jelinek wrote: + /* If the switch takes an integer, convert it. */ + if (arg && cl_options[opt_index].cl_uinteger) + { + value = integral_argument (arg); + if (value == -1) + return; + } So does this issue an error message anywhere or just silently drop the option on the floor if the argument is invalid? + /* If the switch takes an enumerated argument, convert it. */ + if (arg && (cl_options[opt_index].var_type == CLVC_ENUM)) Unnecessary parens. Bernd