Re: Split up option decoding code

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

On Fri, Jun 11, 2010 at 8:28 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> In the process of sharing option processing machinery between the
> driver and the compilers proper to improve multilib selection, I
> expect an intermediate stage where the basic machinery is shared but
> much of the logic associated with individual options isn't; the logic
> for individual options can then be shared incrementally (parts of it,
> e.g. anything setting up trees or RTL in response to options, not
> being needed in the driver at all) until everything needed for
> multilib selection is shared.
>
> This patch moves towards that stage by separating the code that
> converts part of the argv array into an option and its operand from
> the code that processes the option further to change state in the
> compiler; the former goes in opts-common.c (much of the latter will go
> there eventually as well). ?My plan is to follow this up by having an
> initial pass converting the argv array to an array of
> cl_decoded_option structures, with subsequent passes in both the
> driver and the compilers proper working on that structure. ?There are
> several existing passes over argv, and these passes often do not work
> reliably in the presence of option arguments that look like options
> themselves, so this will improve consistency in option handling
> between the driver and the compiler proper and within each of those.
>
> The driver's use of common machinery can then start with
> decode_cmdline_option using SWITCH_TAKES_ARG and WORD_SWITCH_TAKES_ARG
> for unknown options so that an accurate array is produced in the
> driver (and used by prune_options), before actually adding driver
> options (and then miscellaneous options from specs) to the .opt files
> so that it uses the machinery more usefully to convert options to
> features. ?I expect also to convert the translate_options code in
> gcc.c to something better integrated with .opt files.
>
> Bootstrapped with no regressions on x86_64-unknown-linux-gnu. ?OK to
> commit?

Ok.

Thanks,
Richard.

> 2010-06-11 ?Joseph Myers ?<joseph@codesourcery.com>
>
> ? ? ? ?* opts-common.c: Include options.h.
> ? ? ? ?(integral_argument): Move from opts.c.
> ? ? ? ?(decode_cmdline_option): New. ?Based on read_cmdline_option.
> ? ? ? ?* opts.c (integral_argument): Move to opts-common.c.
> ? ? ? ?(read_cmdline_option): Move most contents to
> ? ? ? ?decode_cmdline_option. ?Use %qs in diagnostics.
> ? ? ? ?* opts.h (CL_ERR_DISABLED, CL_ERR_MISSING_ARG, CL_ERR_WRONG_LANG,
> ? ? ? ?CL_ERR_UINT_ARG, struct cl_decoded_option, integral_argument,
> ? ? ? ?decode_cmdline_option): New.
>
> testsuite:
> 2010-06-11 ?Joseph Myers ?<joseph@codesourcery.com>
>
> ? ? ? ?* gcc.dg/funroll-loops-all.c: Update expected error.
>
> Index: opts-common.c
> ===================================================================
> --- opts-common.c ? ? ? (revision 160599)
> +++ opts-common.c ? ? ? (working copy)
> @@ -1,5 +1,5 @@
> ?/* Command line option handling.
> - ? Copyright (C) 2006, 2007, 2008 Free Software Foundation, Inc.
> + ? Copyright (C) 2006, 2007, 2008, 2010 Free Software Foundation, Inc.
>
> ?This file is part of GCC.
>
> @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.
> ?#include "intl.h"
> ?#include "coretypes.h"
> ?#include "opts.h"
> +#include "options.h"
>
> ?/* Perform a binary search to find which option the command-line INPUT
> ? ?matches. ?Returns its index in the option array, and N_OPTS
> @@ -107,6 +108,138 @@ find_opt (const char *input, int lang_ma
> ? return match_wrong_lang;
> ?}
>
> +/* If ARG is a non-negative integer made up solely of digits, return its
> + ? value, otherwise return -1. ?*/
> +
> +int
> +integral_argument (const char *arg)
> +{
> + ?const char *p = arg;
> +
> + ?while (*p && ISDIGIT (*p))
> + ? ?p++;
> +
> + ?if (*p == '\0')
> + ? ?return atoi (arg);
> +
> + ?return -1;
> +}
> +
> +/* Decode the switch beginning at ARGV for the language indicated by
> + ? LANG_MASK, into the structure *DECODED. ?Returns the number of
> + ? switches consumed. ?*/
> +
> +unsigned int
> +decode_cmdline_option (const char **argv, unsigned int lang_mask,
> + ? ? ? ? ? ? ? ? ? ? ?struct cl_decoded_option *decoded)
> +{
> + ?size_t opt_index;
> + ?const char *opt, *arg = 0;
> + ?char *dup = 0;
> + ?int value = 1;
> + ?unsigned int result = 1;
> + ?const struct cl_option *option;
> + ?int errors = 0;
> +
> + ?opt = argv[0];
> +
> + ?opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
> + ?if (opt_index == cl_options_count
> + ? ? ?&& (opt[1] == 'W' || opt[1] == 'f' || opt[1] == 'm')
> + ? ? ?&& opt[2] == 'n' && opt[3] == 'o' && opt[4] == '-')
> + ? ?{
> + ? ? ?/* Drop the "no-" from negative switches. ?*/
> + ? ? ?size_t len = strlen (opt) - 3;
> +
> + ? ? ?dup = XNEWVEC (char, len + 1);
> + ? ? ?dup[0] = '-';
> + ? ? ?dup[1] = opt[1];
> + ? ? ?memcpy (dup + 2, opt + 5, len - 2 + 1);
> + ? ? ?opt = dup;
> + ? ? ?value = 0;
> + ? ? ?opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
> + ? ?}
> +
> + ?if (opt_index == cl_options_count)
> + ? ?goto done;
> +
> + ?option = &cl_options[opt_index];
> +
> + ?/* Reject negative form of switches that don't take negatives as
> + ? ? unrecognized. ?*/
> + ?if (!value && (option->flags & CL_REJECT_NEGATIVE))
> + ? ?{
> + ? ? ?opt_index = cl_options_count;
> + ? ? ?goto done;
> + ? ?}
> +
> + ?/* Check to see if the option is disabled for this configuration. ?*/
> + ?if (option->flags & CL_DISABLED)
> + ? ?errors |= CL_ERR_DISABLED;
> +
> + ?/* Sort out any argument the switch takes. ?*/
> + ?if (option->flags & CL_JOINED)
> + ? ?{
> + ? ? ?/* Have arg point to the original switch. ?This is because
> + ? ? ? ?some code, such as disable_builtin_function, expects its
> + ? ? ? ?argument to be persistent until the program exits. ?*/
> + ? ? ?arg = argv[0] + cl_options[opt_index].opt_len + 1;
> + ? ? ?if (!value)
> + ? ? ? arg += strlen ("no-");
> +
> + ? ? ?if (*arg == '\0' && !(option->flags & CL_MISSING_OK))
> + ? ? ? {
> + ? ? ? ? if (option->flags & CL_SEPARATE)
> + ? ? ? ? ? {
> + ? ? ? ? ? ? arg = argv[1];
> + ? ? ? ? ? ? result = 2;
> + ? ? ? ? ? ? if (arg == NULL)
> + ? ? ? ? ? ? ? result = 1;
> + ? ? ? ? ? }
> + ? ? ? ? else
> + ? ? ? ? ? /* Missing argument. ?*/
> + ? ? ? ? ? arg = NULL;
> + ? ? ? }
> + ? ?}
> + ?else if (option->flags & CL_SEPARATE)
> + ? ?{
> + ? ? ?arg = argv[1];
> + ? ? ?result = 2;
> + ? ? ?if (arg == NULL)
> + ? ? ? result = 1;
> + ? ?}
> +
> + ?/* Check if this is a switch for a different front end. ?*/
> + ?if (!(option->flags & (lang_mask | CL_COMMON | CL_TARGET)))
> + ? ?errors |= CL_ERR_WRONG_LANG;
> + ?else if ((option->flags & CL_TARGET)
> + ? ? ? ? ?&& (option->flags & CL_LANG_ALL)
> + ? ? ? ? ?&& !(option->flags & lang_mask))
> + ? ?/* Complain for target flag language mismatches if any languages
> + ? ? ? are specified. ?*/
> + ? ? ?errors |= CL_ERR_WRONG_LANG;
> +
> + ?if (arg == NULL && (option->flags & (CL_JOINED | CL_SEPARATE)))
> + ? ?errors |= CL_ERR_MISSING_ARG;
> +
> + ?/* If the switch takes an integer, convert it. ?*/
> + ?if (arg && (option->flags & CL_UINTEGER))
> + ? ?{
> + ? ? ?value = integral_argument (arg);
> + ? ? ?if (value == -1)
> + ? ? ? errors |= CL_ERR_UINT_ARG;
> + ? ?}
> +
> + done:
> + ?if (dup)
> + ? ?free (dup);
> + ?decoded->opt_index = opt_index;
> + ?decoded->arg = arg;
> + ?decoded->value = value;
> + ?decoded->errors = errors;
> + ?return result;
> +}
> +
> ?/* Return true if NEXT_OPT_IDX cancels OPT_IDX. ?Return false if the
> ? ?next one is the same as ORIG_NEXT_OPT_IDX. ?*/
>
> Index: testsuite/gcc.dg/funroll-loops-all.c
> ===================================================================
> --- testsuite/gcc.dg/funroll-loops-all.c ? ? ? ?(revision 160599)
> +++ testsuite/gcc.dg/funroll-loops-all.c ? ? ? ?(working copy)
> @@ -1,4 +1,4 @@
> ?/* PR 17594 */
> ?/* { dg-do compile } */
> ?/* { dg-options "-funroll-loops-all" } */
> -/* { dg-error "unrecognized command line option \"-funroll-loops-all\"" "" { target *-*-* } 0 } */
> +/* { dg-error "unrecognized command line option '-funroll-loops-all'" "" { target *-*-* } 0 } */
> Index: opts.c
> ===================================================================
> --- opts.c ? ? ?(revision 160599)
> +++ opts.c ? ? ?(working copy)
> @@ -381,22 +381,6 @@ static void complain_wrong_lang (const c
> ?static void set_debug_level (enum debug_info_type type, int extended,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *arg);
>
> -/* If ARG is a non-negative integer made up solely of digits, return its
> - ? value, otherwise return -1. ?*/
> -static int
> -integral_argument (const char *arg)
> -{
> - ?const char *p = arg;
> -
> - ?while (*p && ISDIGIT (*p))
> - ? ?p++;
> -
> - ?if (*p == '\0')
> - ? ?return atoi (arg);
> -
> - ?return -1;
> -}
> -
> ?/* Return a malloced slash-separated list of languages in MASK. ?*/
> ?static char *
> ?write_langs (unsigned int mask)
> @@ -536,131 +520,61 @@ handle_option (int opt_index, int value,
> ?static unsigned int
> ?read_cmdline_option (const char **argv, unsigned int lang_mask)
> ?{
> - ?size_t opt_index;
> - ?const char *opt, *arg = 0;
> - ?char *dup = 0;
> - ?int value = 1;
> - ?unsigned int result = 0;
> + ?struct cl_decoded_option decoded;
> + ?unsigned int result;
> + ?const char *opt;
> ? const struct cl_option *option;
>
> ? opt = argv[0];
>
> - ?opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
> - ?if (opt_index == cl_options_count
> - ? ? ?&& (opt[1] == 'W' || opt[1] == 'f' || opt[1] == 'm')
> - ? ? ?&& opt[2] == 'n' && opt[3] == 'o' && opt[4] == '-')
> - ? ?{
> - ? ? ?/* Drop the "no-" from negative switches. ?*/
> - ? ? ?size_t len = strlen (opt) - 3;
> -
> - ? ? ?dup = XNEWVEC (char, len + 1);
> - ? ? ?dup[0] = '-';
> - ? ? ?dup[1] = opt[1];
> - ? ? ?memcpy (dup + 2, opt + 5, len - 2 + 1);
> - ? ? ?opt = dup;
> - ? ? ?value = 0;
> - ? ? ?opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
> - ? ? ?if (opt_index == cl_options_count && opt[1] == 'W')
> - ? ? ? {
> - ? ? ? ? /* We don't generate warnings for unknown -Wno-* options
> - ? ? ? ? ? ? unless we issue diagnostics. ?*/
> + ?result = decode_cmdline_option (argv, lang_mask, &decoded);
> + ?if (decoded.opt_index == cl_options_count)
> + ? ?{
> + ? ? ?if (opt[1] == 'W' && opt[2] == 'n' && opt[3] == 'o' && opt[4] == '-')
> + ? ? ? /* We don't generate warnings for unknown -Wno-* options
> + ? ? ? ? ?unless we issue diagnostics. ?*/
> ? ? ? ? ?postpone_unknown_option_warning (argv[0]);
> - ? ? ? ? result = 1;
> - ? ? ? ? goto done;
> - ? ? ? }
> + ? ? ?else
> + ? ? ? error ("unrecognized command line option %qs", opt);
> + ? ? ?return result;
> ? ? }
>
> - ?if (opt_index == cl_options_count)
> - ? ?goto done;
> -
> - ?option = &cl_options[opt_index];
> -
> - ?/* Reject negative form of switches that don't take negatives as
> - ? ? unrecognized. ?*/
> - ?if (!value && (option->flags & CL_REJECT_NEGATIVE))
> - ? ?goto done;
> -
> - ?/* We've recognized this switch. ?*/
> - ?result = 1;
> + ?option = &cl_options[decoded.opt_index];
>
> - ?/* Check to see if the option is disabled for this configuration. ?*/
> - ?if (option->flags & CL_DISABLED)
> + ?if (decoded.errors & CL_ERR_DISABLED)
> ? ? {
> ? ? ? error ("command line option %qs"
> ? ? ? ? ? ? " is not supported by this configuration", opt);
> ? ? ? goto done;
> ? ? }
>
> - ?/* Sort out any argument the switch takes. ?*/
> - ?if (option->flags & CL_JOINED)
> - ? ?{
> - ? ? ?/* Have arg point to the original switch. ?This is because
> - ? ? ? ?some code, such as disable_builtin_function, expects its
> - ? ? ? ?argument to be persistent until the program exits. ?*/
> - ? ? ?arg = argv[0] + cl_options[opt_index].opt_len + 1;
> - ? ? ?if (!value)
> - ? ? ? arg += strlen ("no-");
> -
> - ? ? ?if (*arg == '\0' && !(option->flags & CL_MISSING_OK))
> - ? ? ? {
> - ? ? ? ? if (option->flags & CL_SEPARATE)
> - ? ? ? ? ? {
> - ? ? ? ? ? ? arg = argv[1];
> - ? ? ? ? ? ? result = 2;
> - ? ? ? ? ? }
> - ? ? ? ? else
> - ? ? ? ? ? /* Missing argument. ?*/
> - ? ? ? ? ? arg = NULL;
> - ? ? ? }
> - ? ?}
> - ?else if (option->flags & CL_SEPARATE)
> - ? ?{
> - ? ? ?arg = argv[1];
> - ? ? ?result = 2;
> - ? ?}
> -
> - ?/* Now we've swallowed any potential argument, complain if this
> - ? ? is a switch for a different front end. ?*/
> - ?if (!(option->flags & (lang_mask | CL_COMMON | CL_TARGET)))
> + ?if (decoded.errors & CL_ERR_WRONG_LANG)
> ? ? {
> ? ? ? complain_wrong_lang (argv[0], option, lang_mask);
> ? ? ? goto done;
> ? ? }
> - ?else if ((option->flags & CL_TARGET)
> - ? ? ? ? ?&& (option->flags & CL_LANG_ALL)
> - ? ? ? ? ?&& !(option->flags & lang_mask))
> +
> + ?if (decoded.errors & CL_ERR_MISSING_ARG)
> ? ? {
> - ? ? ?/* Complain for target flag language mismatches if any languages
> - ? ? ? ?are specified. ?*/
> - ? ? ?complain_wrong_lang (argv[0], option, lang_mask);
> + ? ? ?if (!lang_hooks.missing_argument (opt, decoded.opt_index))
> + ? ? ? error ("missing argument to %qs", opt);
> ? ? ? goto done;
> ? ? }
>
> - ?if (arg == NULL && (option->flags & (CL_JOINED | CL_SEPARATE)))
> + ?if (decoded.errors & CL_ERR_UINT_ARG)
> ? ? {
> - ? ? ?if (!lang_hooks.missing_argument (opt, opt_index))
> - ? ? ? error ("missing argument to \"%s\"", opt);
> + ? ? ?error ("argument to %qs should be a non-negative integer",
> + ? ? ? ? ? ?option->opt_text);
> ? ? ? goto done;
> ? ? }
>
> - ?/* If the switch takes an integer, convert it. ?*/
> - ?if (arg && (option->flags & CL_UINTEGER))
> - ? ?{
> - ? ? ?value = integral_argument (arg);
> - ? ? ?if (value == -1)
> - ? ? ? {
> - ? ? ? ? error ("argument to \"%s\" should be a non-negative integer",
> - ? ? ? ? ? ? ? ?option->opt_text);
> - ? ? ? ? goto done;
> - ? ? ? }
> - ? ?}
> + ?gcc_assert (!decoded.errors);
>
> - ?if (!handle_option (opt_index, value, arg, lang_mask, DK_UNSPECIFIED))
> - ? ?result = 0;
> + ?if (!handle_option (decoded.opt_index, decoded.value, decoded.arg,
> + ? ? ? ? ? ? ? ? ? ? lang_mask, DK_UNSPECIFIED))
> + ? ?error ("unrecognized command line option %qs", opt);
>
> ?done:
> - ?if (dup)
> - ? ?free (dup);
> ? return result;
> ?}
>
> @@ -780,12 +694,6 @@ read_cmdline_options (unsigned int argc,
> ? ? ? ?}
>
> ? ? ? n = read_cmdline_option (argv + i, lang_mask);
> -
> - ? ? ?if (!n)
> - ? ? ? {
> - ? ? ? ? n = 1;
> - ? ? ? ? error ("unrecognized command line option \"%s\"", opt);
> - ? ? ? }
> ? ? }
> ?}
>
> Index: opts.h
> ===================================================================
> --- opts.h ? ? ?(revision 160599)
> +++ opts.h ? ? ?(working copy)
> @@ -1,5 +1,5 @@
> ?/* Command line option handling.
> - ? Copyright (C) 2002, 2003, 2004, 2005, 2007, 2008
> + ? Copyright (C) 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010
> ? ?Free Software Foundation, Inc.
>
> ?This file is part of GCC.
> @@ -90,6 +90,34 @@ extern const unsigned int cl_lang_count;
> ?#define CL_UINTEGER ? ? ? ? ? ?(1 << 29) /* Argument is an integer >=0. ?*/
> ?#define CL_UNDOCUMENTED ? ? ? ? ? ? ? ?(1 << 30) /* Do not output with --help. ?*/
>
> +/* Possible ways in which a command-line option may be erroneous.
> + ? These do not include not being known at all; an option index of
> + ? cl_options_count is used for that. ?*/
> +
> +#define CL_ERR_DISABLED ? ? ? ? ? ? ? ?(1 << 0) /* Disabled in this configuration. ?*/
> +#define CL_ERR_MISSING_ARG ? ? (1 << 1) /* Argument required but missing. ?*/
> +#define CL_ERR_WRONG_LANG ? ? ?(1 << 2) /* Option for wrong language. ?*/
> +#define CL_ERR_UINT_ARG ? ? ? ? ? ? ? ?(1 << 3) /* Bad unsigned integer argument. ?*/
> +
> +/* Structure describing the result of decoding an option. ?*/
> +
> +struct cl_decoded_option
> +{
> + ?/* The index of this option, or cl_options_count if not known. ?*/
> + ?size_t opt_index;
> +
> + ?/* The string argument, or NULL if none. ?*/
> + ?const char *arg;
> +
> + ?/* For a boolean option, 1 for the true case and 0 for the "no-"
> + ? ? case. ?For an unsigned integer option, the value of the
> + ? ? argument. ?1 in all other cases. ?*/
> + ?int value;
> +
> + ?/* Any flags describing errors detected in this option. ?*/
> + ?int errors;
> +};
> +
> ?/* Input file names. ?*/
>
> ?extern const char **in_fnames;
> @@ -99,6 +127,10 @@ extern const char **in_fnames;
> ?extern unsigned num_in_fnames;
>
> ?size_t find_opt (const char *input, int lang_mask);
> +extern int integral_argument (const char *arg);
> +extern unsigned int decode_cmdline_option (const char **argv,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int lang_mask,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cl_decoded_option *decoded);
> ?extern void prune_options (int *argcp, char ***argvp);
> ?extern void decode_options (unsigned int argc, const char **argv);
> ?extern int option_enabled (int opt_idx);
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>