[PATCH] Offer suggestions for misspelled attributes (PR c/70186)

[PATCH] Offer suggestions for misspelled attributes (PR c/70186)

[David Malcolm]

Our -Wattributes warnings can be rather cryptic.  The following patch
improves this warning:

../../src/pr70186.c:1:8: warning: 'visbility' attribute directive ignored [-Wattributes]
 struct S *foo __attribute__ ((visbility("hidden")));
        ^

by adding suggestions when unrecognized attributes are encountered,
turning it into this:

../../src/pr70186.c:1:8: warning: 'visbility' attribute directive ignored; did you mean 'visibility'? [-Wattributes]
 struct S *foo __attribute__ ((visbility("hidden")));
        ^

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

Rather late for gcc 7, sorry.  Is this OK for next stage 1,
or OK for stage 3 now? (if this looks low-enough risk)

gcc/ChangeLog:
	PR c/70186
	* attribs.c: Include "spellcheck.h".
	(extract_attribute_substring): Fix leading comment.  Return a bool
	signifying if truncation for underscores occurred.
	(struct edit_distance_traits<struct substring *>): New class.
	(fuzzy_lookup_scoped_attribute_spec): New function, based on
	lookup_scoped_attribute_spec.
	(decl_attributes): Move warning about ignored attributes to..
	(warn_about_unidentified_attribute): ...this new function, and
	potentially offer suggestions for misspelled attributes.

gcc/testsuite/ChangeLog:
	PR c/70186
	* c-c++-common/spellcheck-attributes.c: New test case.
	* g++.dg/cpp0x/spellcheck-attributes.C: New test case.
---
 gcc/attribs.c                                      | 115 ++++++++++++++++++---
 gcc/testsuite/c-c++-common/spellcheck-attributes.c |   5 +
 gcc/testsuite/g++.dg/cpp0x/spellcheck-attributes.C |  18 ++++
 3 files changed, 126 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/spellcheck-attributes.c
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/spellcheck-attributes.C

diff --git a/gcc/attribs.c b/gcc/attribs.c
index e66349a..ff5ce0d 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "stor-layout.h"
 #include "langhooks.h"
 #include "plugin.h"
+#include "spellcheck.h"
 
 /* Table of the tables of attributes (common, language, format, machine)
    searched.  */
@@ -97,10 +98,11 @@ static const struct attribute_spec empty_attribute_table[] =
   { NULL, 0, 0, false, false, false, NULL, false }
 };
 
-/* Return base name of the attribute.  Ie '__attr__' is turned into 'attr'.
-   To avoid need for copying, we simply return length of the string.  */
+/* Extract base name of the attribute.  Ie '__attr__' is turned into 'attr'.
+   To avoid need for copying, we simply update length of the string.
+   Return true if the string was truncated, false otherwise.  */
 
-static void
+static bool
 extract_attribute_substring (struct substring *str)
 {
   if (str->length > 4 && str->str[0] == '_' && str->str[1] == '_'
@@ -108,7 +110,9 @@ extract_attribute_substring (struct substring *str)
     {
       str->length -= 4;
       str->str += 2;
+      return true;
     }
+  return false;
 }
 
 /* Insert an array of attributes ATTRIBUTES into a namespace.  This
@@ -343,6 +347,101 @@ get_attribute_namespace (const_tree attr)
   return get_identifier ("gnu");
 }
 
+/* Specialization of edit_distance_traits for struct substring.  */
+
+template <>
+struct edit_distance_traits<struct substring *>
+{
+  static size_t get_length (struct substring *substr)
+  {
+    gcc_assert (substr);
+    return substr->length;
+  }
+
+  static const char *get_string (struct substring *substr)
+  {
+    gcc_assert (substr);
+    return substr->str;
+  }
+};
+
+/* Look for near matches for the scoped attribute with namespace NS and
+   name NAME.
+   Return the best matching attribute name, or NULL if none is found.
+   If it returns non-NULL then *UNDERSCORES is written to, with true
+   iff leading and trailing underscores were stripped from NAME
+   before the match.  */
+
+static const char *
+fuzzy_lookup_scoped_attribute_spec (const_tree ns, const_tree name,
+				    bool *underscores)
+{
+  struct substring attr;
+  scoped_attributes *attrs;
+
+  const char *ns_str = (ns != NULL_TREE) ? IDENTIFIER_POINTER (ns): NULL;
+
+  attrs = find_attribute_namespace (ns_str);
+
+  if (attrs == NULL)
+    return NULL;
+
+  attr.str = IDENTIFIER_POINTER (name);
+  attr.length = IDENTIFIER_LENGTH (name);
+  *underscores = extract_attribute_substring (&attr);
+
+  best_match <struct substring *, const char *> bm (&attr);
+
+  hash_table<attribute_hasher> *h = attrs->attribute_hash;
+  for (hash_table<attribute_hasher>::iterator iter = h->begin ();
+       iter != h->end (); ++iter)
+    bm.consider ((*iter)->name);
+  return bm.get_best_meaningful_candidate ();
+}
+
+/* Warn about attribute A being unrecognized with name NAME (an identifier),
+   within namespace NS (an identifier, or NULL_TREE).
+   Issue a hint if it appears to be misspelled.  */
+
+static void
+warn_about_unidentified_attribute (tree a, tree ns, tree name)
+{
+  bool underscores;
+  const char *hint
+    = fuzzy_lookup_scoped_attribute_spec (ns, name, &underscores);
+
+  if (ns == NULL_TREE || !cxx11_attribute_p (a))
+    if (hint)
+      if (underscores)
+	warning (OPT_Wattributes,
+		 ("%qE attribute directive ignored"
+		  "; did you mean %<__%s__%>?"),
+		 name, hint);
+      else
+	warning (OPT_Wattributes,
+		 ("%qE attribute directive ignored"
+		  "; did you mean %qs?"),
+		 name, hint);
+    else
+      warning (OPT_Wattributes, "%qE attribute directive ignored",
+	       name);
+  else
+    if (hint)
+      if (underscores)
+	warning (OPT_Wattributes,
+		 ("%<%E::%E%> scoped attribute directive"
+		  " ignored; did you mean %<%E::__%s__%>?"),
+		 ns, name, ns, hint);
+      else
+	warning (OPT_Wattributes,
+		 ("%<%E::%E%> scoped attribute directive ignored"
+		  "; did you mean %<%E::%s%>?"),
+		 ns, name, ns, hint);
+    else
+      warning (OPT_Wattributes,
+	       "%<%E::%E%> scoped attribute directive ignored",
+	       ns, name);
+}
 
 /* Process the attributes listed in ATTRIBUTES and install them in *NODE,
    which is either a DECL (including a TYPE_DECL) or a TYPE.  If a DECL,
@@ -431,15 +530,7 @@ decl_attributes (tree *node, tree attributes, int flags)
       if (spec == NULL)
 	{
 	  if (!(flags & (int) ATTR_FLAG_BUILT_IN))
-	    {
-	      if (ns == NULL_TREE || !cxx11_attribute_p (a))
-		warning (OPT_Wattributes, "%qE attribute directive ignored",
-			 name);
-	      else
-		warning (OPT_Wattributes,
-			 "%<%E::%E%> scoped attribute directive ignored",
-			 ns, name);
-	    }
+	    warn_about_unidentified_attribute (a, ns, name);
 	  continue;
 	}
       else if (list_length (args) < spec->min_length
diff --git a/gcc/testsuite/c-c++-common/spellcheck-attributes.c b/gcc/testsuite/c-c++-common/spellcheck-attributes.c
new file mode 100644
index 0000000..a0da879
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/spellcheck-attributes.c
@@ -0,0 +1,5 @@
+struct S *s1 __attribute__ ((visbility("hidden")));
+/* { dg-warning ".visbility. attribute directive ignored; did you mean .visibility." "" { target *-*-* } .-1 } */
+
+struct S *s2 __attribute__ ((__visbility__("hidden")));
+/* { dg-warning ".__visbility__. attribute directive ignored; did you mean .__visibility__." "" { target *-*-* } .-1 } */
diff --git a/gcc/testsuite/g++.dg/cpp0x/spellcheck-attributes.C b/gcc/testsuite/g++.dg/cpp0x/spellcheck-attributes.C
new file mode 100644
index 0000000..ca72608
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/spellcheck-attributes.C
@@ -0,0 +1,18 @@
+// { dg-do compile { target c++11 } }
+
+int
+test ()
+{
+  [[gnu::unused]] lab_1:
+
+  [[gnu::unuse]] lab_2:
+    /* { dg-warning ".gnu::unuse. scoped attribute directive ignored; did you mean .gnu::unused.." "" { target *-*-* } .-1 } */
+    return 0;
+
+  [[gnu::__unused__]] lab_3:
+
+  [[gnu::__unuse__]] lab_4:
+    /* { dg-warning ".gnu::__unuse__. scoped attribute directive ignored; did you mean .gnu::__unused__.." "" { target *-*-* } .-1 } */
+    return 0;
+
+}
-- 
1.8.5.3

Re: [PATCH] Offer suggestions for misspelled attributes (PR c/70186)

[Jakub Jelinek]

Hi!

On Mon, Dec 19, 2016 at 11:51:29AM -0500, David Malcolm wrote:
> +/* Look for near matches for the scoped attribute with namespace NS and
> +   name NAME.
> +   Return the best matching attribute name, or NULL if none is found.
> +   If it returns non-NULL then *UNDERSCORES is written to, with true
> +   iff leading and trailing underscores were stripped from NAME
> +   before the match.  */
> +
> +static const char *
> +fuzzy_lookup_scoped_attribute_spec (const_tree ns, const_tree name,
> +				    bool *underscores)
> +{
> +  struct substring attr;
> +  scoped_attributes *attrs;
> +
> +  const char *ns_str = (ns != NULL_TREE) ? IDENTIFIER_POINTER (ns): NULL;
> +
> +  attrs = find_attribute_namespace (ns_str);
> +
> +  if (attrs == NULL)
> +    return NULL;
> +
> +  attr.str = IDENTIFIER_POINTER (name);
> +  attr.length = IDENTIFIER_LENGTH (name);
> +  *underscores = extract_attribute_substring (&attr);
> +
> +  best_match <struct substring *, const char *> bm (&attr);
> +
> +  hash_table<attribute_hasher> *h = attrs->attribute_hash;
> +  for (hash_table<attribute_hasher>::iterator iter = h->begin ();
> +       iter != h->end (); ++iter)
> +    bm.consider ((*iter)->name);
> +  return bm.get_best_meaningful_candidate ();

Does this consider the decl_req, type_req and fn_type_req flags in the
attribute tables (i.e. that for attribute on a VAR_DECL it doesn't suggest
attributes that apply to functions only, on FUNCTION_DECL suggest attributes
that apply only to non-function decls, etc.)?
Does it ignore internal only attributes (i.e. attributes containing spaces
or * characters in their names)?
Say
void foo (void) __attribute ((omp_declare_target));
should not suggest the omp declare target attribute, since users can't type
it in.

	Jakub