[PATCH] PR driver/69265 and 69453: improved suggestions for various misspelled options

[David Malcolm <dmalcolm at redhat dot com>]

On Wed, 2016-02-10 at 17:25 +0100, Bernd Schmidt wrote:
> On 02/09/2016 09:44 PM, David Malcolm wrote:
> > This is a bug in a new feature, so it isn't a regression as such,
> > but
> > it's fairly visible, and I believe the fix is relatively low-risk
> > (error-handling of typos of command-line options).
> >
> > This also now covers PR driver/69453 (and its duplicate PR
> > driver/69642), so people *are* running into this.
>
> I think the patch looks reasonable (I expect it needs slight
> adjustment
> after an earlier sanitizer options change).

It did (r232826 was the change in question).

> Whether it's OK or not at
> this stage is something I think I'll want to ask a RM. My inclination
> would be yes.
>
> A small improvement might be calculating the candidates array only
> once
> when making the first suggestion and not freeing it.

Done.

> BTW, I've also run
> into a case of an unhelpful suggestion:
>
> ./cc1 ~/hw.c -fno-if-convert
> cc1: error: unrecognized command line option â-fno-if-convertâ
>
> which should instead suggest fno-if-conversion.

It actually handled that, with the patch.  I've added a test case
for that (and for the other cases mentioned in PR driver/69453 and its
dup).

Changes in this version:
* rebased to today's trunk (in particular, r232826 reworked the sanitizer
  options)
* added testcases for PR driver/69453 "-Wno-", including the one cited by
  Bernd.
* only build the list of candidates once (the first time there's an unrecognized
  option), storing it in a new field of class driver
* add hint for options taking arguments, adding the various enum values
  to the candidate strings, so e.g.:
    -tls-model=global-dynamic
  can be corrected to
    -ftls-model=global-dynamic
  whereas previously no suggestion was offered

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu;
adds 16 PASS results to gcc.sum.

Is this OK for trunk in stage 4? (it's not a regression, but as noted
before it's somewhat user-visible and relatively low-risk, I believe).

Dave

Blurb from original patch follows:
As of r230285 (b279775faf3c56b554ecd38159b70ea7f2d37e0b; PR driver/67613)
the driver provides suggestions for misspelled options.

This works well for some options e.g.

 $ gcc -static-libfortran test.f95
 gcc: error: unrecognized command line option '-static-libfortran';
 did you mean '-static-libgfortran'?

but as reported in PR driver/69265 it can generate poor suggestions:

 $ c++ -sanitize=address foo.cc
 c++: error: unrecognized command line option â-sanitize=addressâ;
 did you mean â-Wframe-addressâ?

The root cause is that the current implementation only considers
cl_options[].opt_text, and has no knowledge of the arguments to
-fsanitize (and hence doesn't consider the "address" text when
computing edit distances).

It also fails to consider the alternate ways of spelling options
e.g. "-Wno-" vs "-W".

The following patch addresses these issues by building a vec of
candidates from cl_options[].opt_text, rather than just using
the latter.

gcc/ChangeLog:
	PR driver/69265
	PR driver/69453
	* gcc.c (driver::driver): Initialize m_option_suggestions.
	(driver::~driver): Clean up m_option_suggestions.
	(suggest_option): Convert to...
	(driver::suggest_option): ...this, and split out into
	driver::build_option_suggestions and find_closest_string.
	(driver::build_option_suggestions): New function, from
	first half of suggest_option.  Special-case
	OPT_fsanitize_ and OPT_fsanitize_recover_, making use of
	the sanitizer_opts array.  For options of enum types, add the
	various enum values to the candidate strings.
	(driver::handle_unrecognized_options): Remove "const".
	* gcc.h (driver::handle_unrecognized_options): Likewise.
	(driver::build_option_suggestions): New decl.
	(driver::suggest_option): New decl.
	(driver::m_option_suggestions): New field.
	* opts-common.c (add_misspelling_candidates): New function.
	* opts.c (sanitizer_opts): Remove decl of struct sanitizer_opts_s
	and make non-static.
	* opts.h (sanitizer_opts): New array decl.
	(add_misspelling_candidates): New function decl.
	* spellcheck.c (find_closest_string): New function.
	* spellcheck.h (find_closest_string): New function decl.

gcc/testsuite/ChangeLog:
	PR driver/69265
	PR driver/69453
	* gcc.dg/spellcheck-options-3.c: New test case.
	* gcc.dg/spellcheck-options-4.c: New test case.
	* gcc.dg/spellcheck-options-5.c: New test case.
	* gcc.dg/spellcheck-options-6.c: New test case.
	* gcc.dg/spellcheck-options-7.c: New test case.
	* gcc.dg/spellcheck-options-8.c: New test case.
	* gcc.dg/spellcheck-options-9.c: New test case.
	* gcc.dg/spellcheck-options-10.c: New test case.
---
 gcc/gcc.c                                    | 112 ++++++++++++++++++++-------
 gcc/gcc.h                                    |   5 +-
 gcc/opts-common.c                            |  41 ++++++++++
 gcc/opts.c                                   |   7 +-
 gcc/opts.h                                   |  11 +++
 gcc/spellcheck.c                             |  46 +++++++++++
 gcc/spellcheck.h                             |   4 +
 gcc/testsuite/gcc.dg/spellcheck-options-10.c |   6 ++
 gcc/testsuite/gcc.dg/spellcheck-options-3.c  |   6 ++
 gcc/testsuite/gcc.dg/spellcheck-options-4.c  |   6 ++
 gcc/testsuite/gcc.dg/spellcheck-options-5.c  |   6 ++
 gcc/testsuite/gcc.dg/spellcheck-options-6.c  |   6 ++
 gcc/testsuite/gcc.dg/spellcheck-options-7.c  |   6 ++
 gcc/testsuite/gcc.dg/spellcheck-options-8.c  |   6 ++
 gcc/testsuite/gcc.dg/spellcheck-options-9.c  |   6 ++
 15 files changed, 239 insertions(+), 35 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-10.c
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-3.c
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-4.c
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-5.c
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-6.c
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-7.c
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-8.c
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-options-9.c

diff --git a/gcc/gcc.c b/gcc/gcc.c
index 683b30f..99fa5e3 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -7135,7 +7135,8 @@ compare_files (char *cmpfile[])
 
 driver::driver (bool can_finalize, bool debug) :
   explicit_link_files (NULL),
-  decoded_options (NULL)
+  decoded_options (NULL),
+  m_option_suggestions (NULL)
 {
   env.init (can_finalize, debug);
 }
@@ -7144,6 +7145,14 @@ driver::~driver ()
 {
   XDELETEVEC (explicit_link_files);
   XDELETEVEC (decoded_options);
+  if (m_option_suggestions)
+    {
+      int i;
+      char *str;
+      FOR_EACH_VEC_ELT (*m_option_suggestions, i, str)
+	free (str);
+      delete m_option_suggestions;
+    }
 }
 
 /* driver::main is implemented as a series of driver:: method calls.  */
@@ -7632,49 +7641,96 @@ driver::maybe_putenv_OFFLOAD_TARGETS () const
   offload_targets = NULL;
 }
 
-/* Helper function for driver::handle_unrecognized_options.
+/* Helper function for driver::suggest_option.  Populate
+   m_option_suggestions with candidate strings for misspelled options.
+   The strings will be freed by the driver's dtor.  */
 
-   Given an unrecognized option BAD_OPT (without the leading dash),
-   locate the closest reasonable matching option (again, without the
-   leading dash), or NULL.  */
-
-static const char *
-suggest_option (const char *bad_opt)
+void
+driver::build_option_suggestions (void)
 {
-  const cl_option *best_option = NULL;
-  edit_distance_t best_distance = MAX_EDIT_DISTANCE;
+  gcc_assert (m_option_suggestions == NULL);
+  m_option_suggestions = new auto_vec <char *> ();
+
+  /* We build a vec of m_option_suggestions, using add_misspelling_candidates
+     to add copies of strings, without a leading dash.  */
 
   for (unsigned int i = 0; i < cl_options_count; i++)
     {
-      edit_distance_t dist = levenshtein_distance (bad_opt,
-						   cl_options[i].opt_text + 1);
-      if (dist < best_distance)
+      const struct cl_option *option = &cl_options[i];
+      const char *opt_text = option->opt_text;
+      switch (i)
 	{
-	  best_distance = dist;
-	  best_option = &cl_options[i];
+	default:
+	  if (option->var_type == CLVC_ENUM)
+	    {
+	      const struct cl_enum *e = &cl_enums[option->var_enum];
+	      for (unsigned j = 0; e->values[j].arg != NULL; j++)
+		{
+		  char *with_arg = concat (opt_text, e->values[j].arg, NULL);
+		  add_misspelling_candidates (m_option_suggestions, with_arg);
+		  free (with_arg);
+		}
+	    }
+	  else
+	    add_misspelling_candidates (m_option_suggestions, opt_text);
+	  break;
+
+	case OPT_fsanitize_:
+	case OPT_fsanitize_recover_:
+	  /* -fsanitize= and -fsanitize-recover= can take
+	     a comma-separated list of arguments.  Given that combinations
+	     are supported, we can't add all potential candidates to the
+	     vec, but if we at least add them individually without commas,
+	     we should do a better job e.g. correcting
+	       "-sanitize=address"
+	     to
+	       "-fsanitize=address"
+	     rather than to "-Wframe-address" (PR driver/69265).  */
+	  {
+	    for (int j = 0; sanitizer_opts[j].name != NULL; ++j)
+	      {
+		/* Get one arg at a time e.g. "-fsanitize=address".  */
+		char *with_arg = concat (opt_text,
+					 sanitizer_opts[j].name,
+					 NULL);
+		/* Add with_arg and all of its variant spellings e.g.
+		   "-fno-sanitize=address" to candidates (albeit without
+		   leading dashes).  */
+		add_misspelling_candidates (m_option_suggestions, with_arg);
+		free (with_arg);
+	      }
+	  }
+	  break;
 	}
     }
+}
 
-  if (!best_option)
-    return NULL;
+/* Helper function for driver::handle_unrecognized_options.
 
-  /* If more than half of the letters were misspelled, the suggestion is
-     likely to be meaningless.  */
-  if (best_option)
-    {
-      unsigned int cutoff = MAX (strlen (bad_opt),
-				 strlen (best_option->opt_text + 1)) / 2;
-      if (best_distance > cutoff)
-	return NULL;
-    }
+   Given an unrecognized option BAD_OPT (without the leading dash),
+   locate the closest reasonable matching option (again, without the
+   leading dash), or NULL.
+
+   The returned string is owned by the driver instance.  */
+
+const char *
+driver::suggest_option (const char *bad_opt)
+{
+  /* Lazily populate m_option_suggestions.  */
+  if (!m_option_suggestions)
+    build_option_suggestions ();
+  gcc_assert (m_option_suggestions);
 
-  return best_option->opt_text + 1;
+  /* "m_option_suggestions" is now populated.  Use it.  */
+  return find_closest_string
+    (bad_opt,
+     (auto_vec <const char *> *) m_option_suggestions);
 }
 
 /* Reject switches that no pass was interested in.  */
 
 void
-driver::handle_unrecognized_options () const
+driver::handle_unrecognized_options ()
 {
   for (size_t i = 0; (int) i < n_switches; i++)
     if (! switches[i].validated)
diff --git a/gcc/gcc.h b/gcc/gcc.h
index 4fa0f4f..cb7081f 100644
--- a/gcc/gcc.h
+++ b/gcc/gcc.h
@@ -45,7 +45,9 @@ class driver
   void putenv_COLLECT_GCC (const char *argv0) const;
   void maybe_putenv_COLLECT_LTO_WRAPPER () const;
   void maybe_putenv_OFFLOAD_TARGETS () const;
-  void handle_unrecognized_options () const;
+  void build_option_suggestions (void);
+  const char *suggest_option (const char *bad_opt);
+  void handle_unrecognized_options ();
   int maybe_print_and_exit () const;
   bool prepare_infiles ();
   void do_spec_on_infiles () const;
@@ -57,6 +59,7 @@ class driver
   char *explicit_link_files;
   struct cl_decoded_option *decoded_options;
   unsigned int decoded_options_count;
+  auto_vec <char *> *m_option_suggestions;
 };
 
 /* The mapping of a spec function name to the C function that
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 38c8058..bb68982 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -365,6 +365,47 @@ static const struct option_map option_map[] =
     { "--no-", NULL, "-f", false, true }
   };
 
+/* Helper function for gcc.c's driver::suggest_option, for populating the
+   vec of suggestions for misspelled options.
+
+   option_map above provides various prefixes for spelling command-line
+   options, which decode_cmdline_option uses to map spellings of options
+   to specific options.  We want to do the reverse: to find all the ways
+   that a user could validly spell an option.
+
+   Given valid OPT_TEXT (with a leading dash), add it and all of its valid
+   variant spellings to CANDIDATES, each without a leading dash.
+
+   For example, given "-Wabi-tag", the following are added to CANDIDATES:
+     "Wabi-tag"
+     "Wno-abi-tag"
+     "-warn-abi-tag"
+     "-warn-no-abi-tag".
+
+   The added strings must be freed using free.  */
+
+void
+add_misspelling_candidates (auto_vec<char *> *candidates,
+			    const char *opt_text)
+{
+  gcc_assert (candidates);
+  gcc_assert (opt_text);
+  candidates->safe_push (xstrdup (opt_text + 1));
+  for (unsigned i = 0; i < ARRAY_SIZE (option_map); i++)
+    {
+      const char *opt0 = option_map[i].opt0;
+      const char *new_prefix = option_map[i].new_prefix;
+      size_t new_prefix_len = strlen (new_prefix);
+
+      if (strncmp (opt_text, new_prefix, new_prefix_len) == 0)
+	{
+	  char *alternative = concat (opt0 + 1, opt_text + new_prefix_len,
+				      NULL);
+	  candidates->safe_push (alternative);
+	}
+    }
+}
+
 /* Decode the switch beginning at ARGV for the language indicated by
    LANG_MASK (including CL_COMMON and CL_TARGET if applicable), into
    the structure *DECODED.  Returns the number of switches
diff --git a/gcc/opts.c b/gcc/opts.c
index 0a18c26..2f45312 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1434,12 +1434,7 @@ enable_fdo_optimizations (struct gcc_options *opts,
 }
 
 /* -f{,no-}sanitize{,-recover}= suboptions.  */
-static const struct sanitizer_opts_s
-{
-  const char *const name;
-  unsigned int flag;
-  size_t len;
-} sanitizer_opts[] =
+const struct sanitizer_opts_s sanitizer_opts[] =
 {
 #define SANITIZER_OPT(name, flags) { #name, flags, sizeof #name - 1 }
   SANITIZER_OPT (address, SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS),
diff --git a/gcc/opts.h b/gcc/opts.h
index 6e6dbea..fa479d8 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -404,4 +404,15 @@ extern void set_struct_debug_option (struct gcc_options *opts,
 				     const char *value);
 extern bool opt_enum_arg_to_value (size_t opt_index, const char *arg,
 				   int *value, unsigned int lang_mask);
+
+extern const struct sanitizer_opts_s
+{
+  const char *const name;
+  unsigned int flag;
+  size_t len;
+} sanitizer_opts[];
+
+extern void add_misspelling_candidates (auto_vec<char *> *candidates,
+					const char *base_option);
+
 #endif
diff --git a/gcc/spellcheck.c b/gcc/spellcheck.c
index e641a56..be540c0 100644
--- a/gcc/spellcheck.c
+++ b/gcc/spellcheck.c
@@ -119,3 +119,49 @@ levenshtein_distance (const char *s, const char *t)
 {
   return levenshtein_distance (s, strlen (s), t, strlen (t));
 }
+
+/* Given TARGET, a non-NULL string, and CANDIDATES, a non-NULL ptr to
+   an autovec of non-NULL strings, determine which element within
+   CANDIDATES has the lowest edit distance to TARGET.  If there are
+   multiple elements with the same minimal distance, the first in the
+   vector wins.
+
+   If more than half of the letters were misspelled, the suggestion is
+   likely to be meaningless, so return NULL for this case.  */
+
+const char *
+find_closest_string (const char *target,
+		     const auto_vec<const char *> *candidates)
+{
+  gcc_assert (target);
+  gcc_assert (candidates);
+
+  int i;
+  const char *candidate;
+  const char *best_candidate = NULL;
+  edit_distance_t best_distance = MAX_EDIT_DISTANCE;
+  size_t len_target = strlen (target);
+  FOR_EACH_VEC_ELT (*candidates, i, candidate)
+    {
+      gcc_assert (candidate);
+      edit_distance_t dist
+	= levenshtein_distance (target, len_target,
+				candidate, strlen (candidate));
+      if (dist < best_distance)
+	{
+	  best_distance = dist;
+	  best_candidate = candidate;
+	}
+    }
+
+  /* If more than half of the letters were misspelled, the suggestion is
+     likely to be meaningless.  */
+  if (best_candidate)
+    {
+      unsigned int cutoff = MAX (len_target, strlen (best_candidate)) / 2;
+      if (best_distance > cutoff)
+	return NULL;
+    }
+
+  return best_candidate;
+}
diff --git a/gcc/spellcheck.h b/gcc/spellcheck.h
index 4c662a7..040c33e 100644
--- a/gcc/spellcheck.h
+++ b/gcc/spellcheck.h
@@ -31,6 +31,10 @@ levenshtein_distance (const char *s, int len_s,
 extern edit_distance_t
 levenshtein_distance (const char *s, const char *t);
 
+extern const char *
+find_closest_string (const char *target,
+		     const auto_vec<const char *> *candidates);
+
 /* spellcheck-tree.c  */
 
 extern edit_distance_t
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-10.c b/gcc/testsuite/gcc.dg/spellcheck-options-10.c
new file mode 100644
index 0000000..1957205
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-10.c
@@ -0,0 +1,6 @@
+/* Verify that we include -Wno- variants when considering hints
+   for misspelled options (PR driver/69453).  */
+
+/* { dg-do compile } */
+/* { dg-options "-fno-if-convert" } */
+/* { dg-error "unrecognized command line option .-fno-if-convert.; did you mean .-fno-if-conversion.?"  "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-3.c b/gcc/testsuite/gcc.dg/spellcheck-options-3.c
new file mode 100644
index 0000000..4133df9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-3.c
@@ -0,0 +1,6 @@
+/* Verify that we provide simple suggestions for the arguments of
+   "-fsanitize=" when it is misspelled (PR driver/69265).  */
+
+/* { dg-do compile } */
+/* { dg-options "-sanitize=address" } */
+/* { dg-error "unrecognized command line option '-sanitize=address'; did you mean '-fsanitize=address'?"  "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-4.c b/gcc/testsuite/gcc.dg/spellcheck-options-4.c
new file mode 100644
index 0000000..252376f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-4.c
@@ -0,0 +1,6 @@
+/* Verify that we provide simple suggestions for the arguments of
+   "-fsanitize-recover=" when it is misspelled (PR driver/69265).  */
+
+/* { dg-do compile } */
+/* { dg-options "-sanitize-recover=integer-divide-by-0" } */
+/* { dg-error "unrecognized command line option '-sanitize-recover=integer-divide-by-0'; did you mean '-fsanitize-recover=integer-divide-by-zero'?"  "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-5.c b/gcc/testsuite/gcc.dg/spellcheck-options-5.c
new file mode 100644
index 0000000..9a02bb7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-5.c
@@ -0,0 +1,6 @@
+/* Verify that we provide suggestions (with arguments) for the "-fno-"
+   variant of "-fsanitize=" when it is misspelled (PR driver/69265).  */
+
+/* { dg-do compile } */
+/* { dg-options "-no-sanitize=all" } */
+/* { dg-error "unrecognized command line option '-no-sanitize=all'; did you mean '-fno-sanitize=all'?"  "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-6.c b/gcc/testsuite/gcc.dg/spellcheck-options-6.c
new file mode 100644
index 0000000..4d6bf0d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-6.c
@@ -0,0 +1,6 @@
+/* Verify that we can generate a suggestion of "--warn-no-abi-tag"
+   from c.opt's "Wabi-tag" (PR driver/69265).  */
+
+/* { dg-do compile } */
+/* { dg-options "-fwarn-no-abi-tag" } */
+/* { dg-error "unrecognized command line option '-fwarn-no-abi-tag'; did you mean '--warn-no-abi-tag'?"  "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-7.c b/gcc/testsuite/gcc.dg/spellcheck-options-7.c
new file mode 100644
index 0000000..ca893994
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-7.c
@@ -0,0 +1,6 @@
+/* Verify that we provide a hint if the user misspells an option that
+   takes an argument (PR driver/69265).  */
+
+/* { dg-do compile } */
+/* { dg-options "-tls-model=global-dynamic" } */
+/* { dg-error "unrecognized command line option '-tls-model=global-dynamic'; did you mean '-ftls-model=global-dynamic'?"  "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-8.c b/gcc/testsuite/gcc.dg/spellcheck-options-8.c
new file mode 100644
index 0000000..2cc6c1f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-8.c
@@ -0,0 +1,6 @@
+/* Verify that we include -Wno- variants when considering hints
+   for misspelled options (PR driver/69453).  */
+
+/* { dg-do compile } */
+/* { dg-options "--Wno-narrowing" } */
+/* { dg-error "unrecognized command line option '--Wno-narrowing'; did you mean '-Wno-narrowing'?"  "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-options-9.c b/gcc/testsuite/gcc.dg/spellcheck-options-9.c
new file mode 100644
index 0000000..768b6f8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-options-9.c
@@ -0,0 +1,6 @@
+/* Verify that we include -Wno- variants when considering hints
+   for misspelled options (PR driver/69453).  */
+
+/* { dg-do compile } */
+/* { dg-options "-fmo-unroll-loops" } */
+/* { dg-error "unrecognized command line option '-fmo-unroll-loops'; did you mean '-fno-unroll-loops'?"  "" { target *-*-* } 0 } */
-- 
1.8.5.3