[i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)

On 05/09/16 18:25, Jakub Jelinek wrote:
> Hi!
>
> While most of the i386.opt -m....= options have enum args and thus
> cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy= (and also
> -mrecip=) don't use that, with the CPU strings being maintained inside of a
> function rather than in some *.def file that could be also sourced into the
> *.opt or something (and similarly for the strategies).
>
> This patch adds inform calls that handle those similarly to what
> cmdline_handle_error does for the options with enum values.
> In addition, it adds %qs instead of %s in a couple of spaces, and
> stops reporting incorrect attribute option("march=...") when it is
> target("march=...") etc.

Something like the following should avoid a lot of (future) duplication (untested):

On Mon, Sep 05, 2016 at 08:42:37PM +0100, Manuel López-Ibáñez wrote:
> Something like the following should avoid a lot of (future) duplication (untested):

You're right.  I've missed that I actually push the candidates into a vector
anyway, so the concatenation can be done in a common code.

> Index: opts-common.c
> ===================================================================
> --- opts-common.c	(revision 239995)
> +++ opts-common.c	(working copy)
> @@ -1069,6 +1069,40 @@
>    decoded->errors = 0;
>  }
> 
> +static void
> +candidates_to_string(char *s, const auto_vec <const char *> *candidates)

Formatting, missing space before (.

> +{
> +  int i;
> +  const char *candidate;
> +  char *p = s;
> +  FOR_EACH_VEC_ELT (*candidates, i, candidate)
> +    {
> +      gcc_assert (candidate);
> +      size_t arglen = strlen (candidate);
> +      memcpy (p, candidate, arglen);
> +      p[arglen] = ' ';
> +      p += arglen + 1;
> +    }
> +  p[-1] = 0;
> +}

As I think this isn't performance sensitive code, I think it would be better
to simplify the callers and compute total_len inside of the following
function (i.e. walk the vector 3 times (once in unrecognized_argument_error
to compute total_len, once in candidates_to_string, once in
find_closest_string).

> +
> +void
> +unrecognized_argument_error (location_t loc, const char *opt, const char *arg,
> +			     const auto_vec <const char*> &candidates,
> +			     size_t total_len)
> +{
> +  error_at (loc, "argument %qs to %qs not recognized", arg, opt);
> +
> +  char *s = XALLOCAVEC (char, total_len);
> +  candidates_to_string (s, &candidates);
> +  const char *hint = find_closest_string (arg, &candidates);
> +  if (hint)
> +    inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
> +	    opt, s, hint);
> +  else
> +    inform (loc, "valid arguments to %qs are: %s", opt, s);
> +}
> +
>  /* 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.
> @@ -1107,40 +1141,22 @@
>    if (errors & CL_ERR_ENUM_ARG)
>      {
>        const struct cl_enum *e = &cl_enums[option->var_enum];
> -      unsigned int i;
> -      size_t len;
> -      char *s, *p;
> -
>        if (e->unknown_error)
>  	error_at (loc, e->unknown_error, arg);
>        else
> -	error_at (loc, "unrecognized argument in option %qs", opt);
> -
> -      len = 0;
> -      for (i = 0; e->values[i].arg != NULL; i++)
> -	len += strlen (e->values[i].arg) + 1;
> -
> -      auto_vec <const char *> candidates;
> -      s = XALLOCAVEC (char, len);
> -      p = s;
> -      for (i = 0; e->values[i].arg != NULL; i++)
>  	{
> -	  if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
> -	    continue;
> -	  size_t arglen = strlen (e->values[i].arg);
> -	  memcpy (p, e->values[i].arg, arglen);
> -	  p[arglen] = ' ';
> -	  p += arglen + 1;
> -	  candidates.safe_push (e->values[i].arg);
> +	  size_t len = 0;
> +	  unsigned int i;
> +	  auto_vec <const char *> candidates;
> +	  for (i = 0; e->values[i].arg != NULL; i++)
> +	    {
> +	      if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
> +		continue;
> +	      len += strlen (e->values[i].arg) + 1;
> +	      candidates.safe_push (e->values[i].arg);
> +	    }
> +	  unrecognized_argument_error (loc, opt, arg, candidates, len);
>  	}
> -      p[-1] = 0;
> -      const char *hint = find_closest_string (arg, &candidates);
> -      if (hint)
> -	inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
> -		option->opt_text, s, hint);
> -      else
> -	inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
> -
>        return true;
>      }
> 

Plus obviously the unrecognized_argument_error needs to be declared in some
header file.

That said, for x86 -march/-mtune uses this is problematic, as it uses either
%<-march=%> argument or target("march=") attribute wording there depending
on whether it is a command line option or target attribute.  Though, it is
not good this way for translation anyway.  Perhaps use XNEWVEC instead of
XALLOCAVEC for the all options string, and have the helper function just
return that + hint, inform by itself and then free the string?

	Jakub