Re: Make diagnostic pragmas use option handlers, clean up -Werror=

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Nov 12, 2010 at 11:40 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> Following up on my patch
> <http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01174.html>, and
> relative to a tree with
> <http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01317.html> (pending
> review) applied, this patch further cleans up -Werror= and #pragma GCC
> diagnostic handling.
>
> enable_warning_as_error is made to take gcc_options parameters rather
> than hardcoding use of global_options. ?The core of that function is
> made into the new control_warning_option, which is used by #pragma GCC
> diagnostic as well as -Werror=.
>
> As well as eliminating a hardcoding of global_options in option
> handling, this sharing of code fixes various bugs by causing #pragma
> GCC diagnostic to use option handlers; one such case is illustrated by
> the testcase added.
>
> Note that I left the linear search through options in #pragma GCC
> diagnostic handling (which should use find_opt) as is. ?I also did not
> do anything about how this pragma can be used with *any* CLVC_BOOLEAN
> option, not just warning options - I'm sure it must be possible to
> produce ICEs that way by changing options at this point and bypassing
> all the consistency checks and post-option processing done on the
> command line.
>
> Bootstrapped with no regressions on x86_64-unknown-linux-gnu. ?OK to
> commit (the non-front-end parts)?

Ok.

Thanks,
Richard.

> 2010-11-12 ?Joseph Myers ?<joseph@codesourcery.com>
>
> ? ? ? ?* opts-common.c (control_warning_option): New.
> ? ? ? ?* opts.c (set_default_handlers): New.
> ? ? ? ?(decode_options): Use set_default_handlers and
> ? ? ? ?control_warning_option.
> ? ? ? ?(common_handle_option): Update call to enable_warning_as_error.
> ? ? ? ?(enable_warning_as_error): Take gcc_options parameters. ?Use
> ? ? ? ?control_warning_option.
> ? ? ? ?* opts.h (set_default_handlers, control_warning_option): Declare.
>
> c-family:
> 2010-11-12 ?Joseph Myers ?<joseph@codesourcery.com>
>
> ? ? ? ?* c-common.h (c_family_lang_mask): Declare.
> ? ? ? ?* c-opts.c (c_family_lang_mask): Make extern.
> ? ? ? ?* c-pragma.c (handle_pragma_diagnostic): Use
> ? ? ? ?control_warning_option.
>
> testsuite:
> 2010-11-12 ?Joseph Myers ?<joseph@codesourcery.com>
>
> ? ? ? ?* gcc.dg/pragma-diag-2.c: New test.
>
> diff -rupN --exclude=.svn gcc-mainline-0/gcc/c-family/c-common.h gcc-mainline/gcc/c-family/c-common.h
> --- gcc-mainline-0/gcc/c-family/c-common.h ? ? ?2010-11-12 07:44:53.000000000 -0800
> +++ gcc-mainline/gcc/c-family/c-common.h ? ? ? ?2010-11-12 12:51:45.000000000 -0800
> @@ -762,6 +762,7 @@ extern void set_compound_literal_name (t
>
> ?extern tree build_va_arg (location_t, tree, tree);
>
> +extern const unsigned int c_family_lang_mask;
> ?extern unsigned int c_common_option_lang_mask (void);
> ?extern void c_common_initialize_diagnostics (diagnostic_context *);
> ?extern bool c_common_complain_wrong_lang_p (const struct cl_option *);
> diff -rupN --exclude=.svn gcc-mainline-0/gcc/c-family/c-opts.c gcc-mainline/gcc/c-family/c-opts.c
> --- gcc-mainline-0/gcc/c-family/c-opts.c ? ? ? ?2010-11-12 07:45:36.000000000 -0800
> +++ gcc-mainline/gcc/c-family/c-opts.c ?2010-11-12 12:51:45.000000000 -0800
> @@ -132,7 +132,7 @@ static struct deferred_opt
> ?} *deferred_opts;
>
>
> -static const unsigned int
> +extern const unsigned int
> ?c_family_lang_mask = (CL_C | CL_CXX | CL_ObjC | CL_ObjCXX);
>
> ?/* Defer option CODE with argument ARG. ?*/
> @@ -467,6 +467,10 @@ c_common_handle_option (size_t scode, co
> ? ? ? cpp_opts->warn_invalid_pch = value;
> ? ? ? break;
>
> + ? ?case OPT_Wlong_long:
> + ? ? ?cpp_opts->cpp_warn_long_long = value;
> + ? ? ?break;
> +
> ? ? case OPT_Wmissing_include_dirs:
> ? ? ? cpp_opts->warn_missing_include_dirs = value;
> ? ? ? break;
> diff -rupN --exclude=.svn gcc-mainline-0/gcc/c-family/c-pragma.c gcc-mainline/gcc/c-family/c-pragma.c
> --- gcc-mainline-0/gcc/c-family/c-pragma.c ? ? ?2010-11-12 07:14:15.000000000 -0800
> +++ gcc-mainline/gcc/c-family/c-pragma.c ? ? ? ?2010-11-12 12:51:45.000000000 -0800
> @@ -720,6 +720,7 @@ handle_pragma_diagnostic(cpp_reader *ARG
> ? enum cpp_ttype token;
> ? diagnostic_t kind;
> ? tree x;
> + ?struct cl_option_handlers handlers;
>
> ? token = pragma_lex (&x);
> ? if (token != CPP_NAME)
> @@ -748,18 +749,14 @@ handle_pragma_diagnostic(cpp_reader *ARG
> ? if (token != CPP_STRING)
> ? ? GCC_BAD ("missing option after %<#pragma GCC diagnostic%> kind");
> ? option_string = TREE_STRING_POINTER (x);
> + ?set_default_handlers (&handlers);
> ? for (option_index = 0; option_index < cl_options_count; option_index++)
> ? ? if (strcmp (cl_options[option_index].opt_text, option_string) == 0)
> ? ? ? {
> - ? ? ? void *flag_var = option_flag_var (option_index, &global_options);
> -
> - ? ? ? /* This overrides -Werror, for example. ?*/
> - ? ? ? diagnostic_classify_diagnostic (global_dc, option_index, kind, input_location);
> - ? ? ? /* This makes sure the option is enabled, like -Wfoo would do. ?*/
> - ? ? ? if (cl_options[option_index].var_type == CLVC_BOOLEAN
> - ? ? ? ? ? && flag_var
> - ? ? ? ? ? && kind != DK_IGNORED)
> - ? ? ? ? ? *(int *) flag_var = 1;
> + ? ? ? control_warning_option (option_index, (int) kind, kind != DK_IGNORED,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? input_location, c_family_lang_mask, &handlers,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &global_options, &global_options_set,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? global_dc);
> ? ? ? ?return;
> ? ? ? }
> ? GCC_BAD ("unknown option after %<#pragma GCC diagnostic%> kind");
> diff -rupN --exclude=.svn gcc-mainline-0/gcc/opts-common.c gcc-mainline/gcc/opts-common.c
> --- gcc-mainline-0/gcc/opts-common.c ? ?2010-11-12 08:03:34.000000000 -0800
> +++ gcc-mainline/gcc/opts-common.c ? ? ?2010-11-12 12:56:36.000000000 -0800
> @@ -977,3 +977,34 @@ option_flag_var (int opt_index, struct g
> ? ? return NULL;
> ? return (void *)(((char *) opts) + option->flag_var_offset);
> ?}
> +
> +/* Set a warning option OPT_INDEX (language mask LANG_MASK, option
> + ? handlers HANDLERS) to have diagnostic kind KIND for option
> + ? structures OPTS and OPTS_SET and diagnostic context DC (possibly
> + ? NULL), at location LOC (UNKNOWN_LOCATION for -Werror=). ?If IMPLY,
> + ? the warning option in question is implied at this point. ?This is
> + ? 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,
> + ? ? ? ? ? ? ? ? ? ? ? const struct cl_option_handlers *handlers,
> + ? ? ? ? ? ? ? ? ? ? ? struct gcc_options *opts,
> + ? ? ? ? ? ? ? ? ? ? ? struct gcc_options *opts_set,
> + ? ? ? ? ? ? ? ? ? ? ? diagnostic_context *dc)
> +{
> + ?if (cl_options[opt_index].alias_target != N_OPTS)
> + ? ?opt_index = cl_options[opt_index].alias_target;
> + ?if (opt_index == OPT_SPECIAL_ignore)
> + ? ?return;
> + ?if (dc)
> + ? ?diagnostic_classify_diagnostic (dc, opt_index, (diagnostic_t) kind, loc);
> + ?if (imply)
> + ? ?{
> + ? ? ?/* -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);
> + ? ?}
> +}
> diff -rupN --exclude=.svn gcc-mainline-0/gcc/opts.c gcc-mainline/gcc/opts.c
> --- gcc-mainline-0/gcc/opts.c ? 2010-11-12 07:49:56.000000000 -0800
> +++ gcc-mainline/gcc/opts.c ? ? 2010-11-12 12:55:33.000000000 -0800
> @@ -363,6 +363,8 @@ static void set_unsafe_math_optimization
> ?static void enable_warning_as_error (const char *arg, int value,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int lang_mask,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const struct cl_option_handlers *handlers,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct gcc_options *opts,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct gcc_options *opts_set,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? location_t loc,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? diagnostic_context *dc);
>
> @@ -1014,6 +1016,23 @@ default_options_optimization (struct gcc
>
> ?static void finish_options (struct gcc_options *, struct gcc_options *);
>
> +/* Set *HANDLERS to the default set of option handlers for use in the
> + ? compilers proper (not the driver). ?*/
> +void
> +set_default_handlers (struct cl_option_handlers *handlers)
> +{
> + ?handlers->unknown_option_callback = unknown_option_callback;
> + ?handlers->wrong_lang_callback = complain_wrong_lang;
> + ?handlers->post_handling_callback = post_handling_callback;
> + ?handlers->num_handlers = 3;
> + ?handlers->handlers[0].handler = lang_handle_option;
> + ?handlers->handlers[0].mask = initial_lang_mask;
> + ?handlers->handlers[1].handler = common_handle_option;
> + ?handlers->handlers[1].mask = CL_COMMON;
> + ?handlers->handlers[2].handler = target_handle_option;
> + ?handlers->handlers[2].mask = CL_TARGET;
> +}
> +
> ?/* Parse command line options and set default flag values. ?Do minimal
> ? ?options processing. ?The decoded options are in *DECODED_OPTIONS
> ? ?and *DECODED_OPTIONS_COUNT; settings go in OPTS, OPTS_SET and DC;
> @@ -1030,20 +1049,12 @@ decode_options (struct gcc_options *opts
>
> ? lang_mask = initial_lang_mask;
>
> - ?handlers.unknown_option_callback = unknown_option_callback;
> - ?handlers.wrong_lang_callback = complain_wrong_lang;
> - ?handlers.post_handling_callback = post_handling_callback;
> - ?handlers.num_handlers = 3;
> - ?handlers.handlers[0].handler = lang_handle_option;
> - ?handlers.handlers[0].mask = lang_mask;
> - ?handlers.handlers[1].handler = common_handle_option;
> - ?handlers.handlers[1].mask = CL_COMMON;
> - ?handlers.handlers[2].handler = target_handle_option;
> - ?handlers.handlers[2].mask = CL_TARGET;
> -
> - ?/* Enable -Werror=coverage-mismatch by default */
> - ?enable_warning_as_error ("coverage-mismatch", 1, lang_mask, &handlers,
> - ? ? ? ? ? ? ? ? ? ? ? ? ?loc, dc);
> + ?set_default_handlers (&handlers);
> +
> + ?/* Enable -Werror=coverage-mismatch by default. ?*/
> + ?control_warning_option (OPT_Wcoverage_mismatch, (int) DK_ERROR, true,
> + ? ? ? ? ? ? ? ? ? ? ? ? loc, lang_mask,
> + ? ? ? ? ? ? ? ? ? ? ? ? &handlers, opts, opts_set, dc);
>
> ? default_options_optimization (opts, opts_set,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?decoded_options, decoded_options_count,
> @@ -1783,7 +1794,8 @@ common_handle_option (struct gcc_options
> ? ? ? break;
>
> ? ? case OPT_Werror_:
> - ? ? ?enable_warning_as_error (arg, value, lang_mask, handlers, loc, dc);
> + ? ? ?enable_warning_as_error (arg, value, lang_mask, handlers,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?opts, opts_set, loc, dc);
> ? ? ? break;
>
> ? ? case OPT_Wlarger_than_:
> @@ -2412,13 +2424,15 @@ get_option_state (struct gcc_options *op
> ?}
>
> ?/* Enable (or disable if VALUE is 0) a warning option ARG (language
> - ? mask LANG_MASK, option handlers HANDLERS) as an error for
> - ? diagnostic context DC (possibly NULL), location LOC. ?This is used
> - ? by -Werror=. ?*/
> + ? mask LANG_MASK, option handlers HANDLERS) as an error for option
> + ? structures OPTS and OPTS_SET, diagnostic context DC (possibly
> + ? NULL), location LOC. ?This is used by -Werror=. ?*/
>
> ?static void
> ?enable_warning_as_error (const char *arg, int value, unsigned int lang_mask,
> ? ? ? ? ? ? ? ? ? ? ? ? const struct cl_option_handlers *handlers,
> + ? ? ? ? ? ? ? ? ? ? ? ?struct gcc_options *opts,
> + ? ? ? ? ? ? ? ? ? ? ? ?struct gcc_options *opts_set,
> ? ? ? ? ? ? ? ? ? ? ? ? location_t loc, diagnostic_context *dc)
> ?{
> ? char *new_option;
> @@ -2430,29 +2444,15 @@ enable_warning_as_error (const char *arg
> ? option_index = find_opt (new_option, lang_mask);
> ? if (option_index == OPT_SPECIAL_unknown)
> ? ? {
> - ? ? ?error ("-Werror=%s: No option -%s", arg, new_option);
> + ? ? ?error ("-Werror=%s: no option -%s", arg, new_option);
> ? ? }
> ? else
> ? ? {
> - ? ? ?const struct cl_option *option = &cl_options[option_index];
> ? ? ? const diagnostic_t kind = value ? DK_ERROR : DK_WARNING;
>
> - ? ? ?if (option->alias_target != N_OPTS)
> - ? ? ? option_index = option->alias_target;
> - ? ? ?if (option_index == OPT_SPECIAL_ignore)
> - ? ? ? return;
> - ? ? ?if (dc)
> - ? ? ? diagnostic_classify_diagnostic (dc, option_index, kind, loc);
> - ? ? ?if (kind == DK_ERROR)
> - ? ? ? {
> - ? ? ? ? const struct cl_option * const option = cl_options + option_index;
> -
> - ? ? ? ? /* -Werror=foo implies -Wfoo. ?*/
> - ? ? ? ? if (option->var_type == CLVC_BOOLEAN)
> - ? ? ? ? ? handle_generated_option (&global_options, &global_options_set,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?option_index, NULL, value, lang_mask,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?(int)kind, loc, handlers, dc);
> - ? ? ? }
> + ? ? ?control_warning_option (option_index, (int) kind, value,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? loc, lang_mask,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? handlers, opts, opts_set, dc);
> ? ? }
> ? free (new_option);
> ?}
> diff -rupN --exclude=.svn gcc-mainline-0/gcc/opts.h gcc-mainline/gcc/opts.h
> --- gcc-mainline-0/gcc/opts.h ? 2010-11-12 07:35:11.000000000 -0800
> +++ gcc-mainline/gcc/opts.h ? ? 2010-11-12 12:52:31.000000000 -0800
> @@ -223,6 +223,7 @@ extern void decode_cmdline_options_to_ar
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char **argv,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cl_decoded_option **decoded_options,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int *decoded_options_count);
> +extern void set_default_handlers (struct cl_option_handlers *handlers);
> ?extern void decode_options (struct gcc_options *opts,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct gcc_options *opts_set,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cl_decoded_option *decoded_options,
> @@ -255,5 +256,12 @@ extern void read_cmdline_option (struct
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int lang_mask,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 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,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int lang_mask,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const struct cl_option_handlers *handlers,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct gcc_options *opts,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct gcc_options *opts_set,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? diagnostic_context *dc);
> ?extern void print_ignored_options (void);
> ?#endif
> diff -rupN --exclude=.svn gcc-mainline-0/gcc/testsuite/gcc.dg/pragma-diag-2.c gcc-mainline/gcc/testsuite/gcc.dg/pragma-diag-2.c
> --- gcc-mainline-0/gcc/testsuite/gcc.dg/pragma-diag-2.c 1969-12-31 16:00:00.000000000 -0800
> +++ gcc-mainline/gcc/testsuite/gcc.dg/pragma-diag-2.c ? 2010-11-12 12:51:45.000000000 -0800
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=c89 -pedantic -Wno-long-long" } */
> +/* { dg-message "warnings being treated as errors" "" { target *-*-* } 0 } */
> +
> +int i = 0LL;
> +
> +#pragma GCC diagnostic error "-Wlong-long"
> +
> +int j = 1LL; /* { dg-error "long long" } */
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>