[PATCH 2/3] C++: provide macro used-before-defined hint (PR c++/72786).

[PATCH 2/3] C++: provide macro used-before-defined hint (PR c++/72786).

[David Malcolm]

This patch uses the name_hint/deferred_diagnostic to provide
a message in the C++ frontend if a macro is used before it is defined
e.g.:

test.c:6:24: error: expected ‘;’ at end of member declaration
   virtual void clone() const OVERRIDE { }
                        ^~~~~
                             ;
test.c:6:30: error: ‘OVERRIDE’ does not name a type
   virtual void clone() const OVERRIDE { }
                              ^~~~~~~~
test.c:6:30: note: the macro ‘OVERRIDE’ had not yet been defined
test.c:15:0: note: it was later defined here
 #define OVERRIDE override

It's possible to do it from the C++ frontend as tokenization happens
up-front (and hence the macro already exists when the above is parsed);
I attempted to do it from the C frontend, but because the C frontend only
tokenizes on-demand during parsing, the macro isn't known about until
later.

gcc/cp/ChangeLog:
	PR c++/72786
	* name-lookup.c (class macro_use_before_def): New class.
	(lookup_name_fuzzy): Detect macro that were used before being
	defined, and report them as such.

gcc/ChangeLog:
	PR c++/72786
	* spellcheck.h (best_match::blithely_get_best_candidate): New
	accessor.

gcc/testsuite/ChangeLog:
	PR c++/72786
	* g++.dg/spellcheck-macro-ordering-2.C: New test case.
	* g++.dg/spellcheck-macro-ordering.C: Add dg-message directives
	for macro used-before-defined.

libcpp/ChangeLog:
	PR c++/72786
	* include/cpplib.h (cpp_macro_definition_location): New decl.
	* macro.c (cpp_macro_definition): New function.
---
 gcc/cp/name-lookup.c                               | 47 +++++++++++++++++++++-
 gcc/spellcheck.h                                   |  7 ++++
 gcc/testsuite/g++.dg/spellcheck-macro-ordering-2.C | 17 ++++++++
 gcc/testsuite/g++.dg/spellcheck-macro-ordering.C   |  3 +-
 libcpp/include/cpplib.h                            |  1 +
 libcpp/macro.c                                     |  8 ++++
 6 files changed, 80 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-macro-ordering-2.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index de8c267..93bea35 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -4952,12 +4952,46 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
     }
 }
 
+/* Subclass of deferred_diagnostic.  Notify the user that the
+   given macro was used before it was defined.
+   This can be done in the C++ frontend since tokenization happens
+   upfront.  */
+
+class macro_use_before_def : public deferred_diagnostic
+{
+ public:
+  /* Ctor.  LOC is the location of the usage.  MACRO is the
+     macro that was used.  */
+  macro_use_before_def (location_t loc, cpp_hashnode *macro)
+  : deferred_diagnostic (loc), m_macro (macro)
+  {
+    gcc_assert (macro);
+  }
+
+  void emit () OVERRIDE FINAL
+  {
+    source_location def_loc = cpp_macro_definition_location (m_macro);
+    if (def_loc != UNKNOWN_LOCATION)
+      {
+	inform (get_location (), "the macro %qs had not yet been defined",
+		(const char *)m_macro->ident.str);
+	inform (def_loc, "it was later defined here");
+      }
+  }
+
+ private:
+  cpp_hashnode *m_macro;
+};
+
+
 /* Search for near-matches for NAME within the current bindings, and within
    macro names, returning the best match as a const char *, or NULL if
-   no reasonable match is found. */
+   no reasonable match is found.
+
+   Use LOC for any deferred diagnostics.  */
 
 name_hint
-lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t)
+lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t loc)
 {
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
@@ -4987,6 +5021,15 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t)
   /* If a macro is the closest so far to NAME, consider it.  */
   if (best_macro)
     bm.consider ((const char *)best_macro->ident.str);
+  else if (bmm.get_best_distance () == 0)
+    {
+      /* If we have an exact match for a macro name, then the
+	 macro has been used before it was defined.  */
+      cpp_hashnode *macro = bmm.blithely_get_best_candidate ();
+      if (macro)
+	return name_hint (NULL,
+			  new macro_use_before_def (loc, macro));
+    }
 
   /* Try the "starts_decl_specifier_p" keywords to detect
      "singed" vs "signed" typos.  */
diff --git a/gcc/spellcheck.h b/gcc/spellcheck.h
index 2edc695..bad3c1e 100644
--- a/gcc/spellcheck.h
+++ b/gcc/spellcheck.h
@@ -178,6 +178,13 @@ class best_match
     return m_best_candidate;
   }
 
+  /* Get the closest candidate so far, without applying any filtering.  */
+
+  candidate_t blithely_get_best_candidate () const
+  {
+    return m_best_candidate;
+  }
+
   edit_distance_t get_best_distance () const { return m_best_distance; }
   size_t get_best_candidate_length () const { return m_best_candidate_len; }
 
diff --git a/gcc/testsuite/g++.dg/spellcheck-macro-ordering-2.C b/gcc/testsuite/g++.dg/spellcheck-macro-ordering-2.C
new file mode 100644
index 0000000..73c0f21
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-macro-ordering-2.C
@@ -0,0 +1,17 @@
+// PR c++/72786
+
+/* Example of undeffed macro.  */
+
+#define OVERRIDE override
+
+#undef OVERRIDE
+
+class DocTargetDriver {
+  virtual void clone() const OVERRIDE { } // { dg-line usage }
+  /* Offering "OVERRIDE" as a spelling suggestion for "OVERRIDE" would be
+     nonsensical.  */
+  // { dg-bogus "did you mean" "" { target *-*-* } usage }
+  // { dg-error "expected .;. at end of member declaration" "" { target *-*-* } usage }
+  // { dg-error ".OVERRIDE. does not name a type" "" { target *-*-* } usage }
+  // { dg-bogus "macro" "" { target *-*-* } usage }
+};
diff --git a/gcc/testsuite/g++.dg/spellcheck-macro-ordering.C b/gcc/testsuite/g++.dg/spellcheck-macro-ordering.C
index 3b888c6..a2c7bba 100644
--- a/gcc/testsuite/g++.dg/spellcheck-macro-ordering.C
+++ b/gcc/testsuite/g++.dg/spellcheck-macro-ordering.C
@@ -9,7 +9,8 @@ class DocTargetDriver {
   // { dg-bogus "did you mean" "" { target *-*-* } .-3 }
   // { dg-error "expected .;. at end of member declaration" "" { target *-*-* } .-4 }
   // { dg-error ".OVERRIDE. does not name a type" "" { target *-*-* } .-5 }
+  // { dg-message "the macro 'OVERRIDE' had not yet been defined" "" { target *-*-* } .-6 }
 };
 
 #define OVERRIDE override
-
+// { dg-message "it was later defined here" "" { target *-*-* } .-1 }
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index b843992..cf2fdc6 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -876,6 +876,7 @@ extern const cpp_token *cpp_get_token_with_location (cpp_reader *,
 extern bool cpp_fun_like_macro_p (cpp_hashnode *);
 extern const unsigned char *cpp_macro_definition (cpp_reader *,
 						  cpp_hashnode *);
+extern source_location cpp_macro_definition_location (cpp_hashnode *);
 extern void _cpp_backup_tokens (cpp_reader *, unsigned int);
 extern const cpp_token *cpp_peek_token (cpp_reader *, int);
 
diff --git a/libcpp/macro.c b/libcpp/macro.c
index de18c22..16fdd2a 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -3472,3 +3472,11 @@ cpp_macro_definition (cpp_reader *pfile, cpp_hashnode *node)
   *buffer = '\0';
   return pfile->macro_buffer;
 }
+
+/* Get the line at which the macro was defined.  */
+
+source_location
+cpp_macro_definition_location (cpp_hashnode *node)
+{
+  return node->value.macro->line;
+}
-- 
1.8.5.3

[PATCH 3/3] C: hints for missing stdlib includes for macros and types

[David Malcolm]

The C frontend already "knows" about many common functions in
the C standard library:

  test.c: In function 'test':
  test.c:3:3: warning: implicit declaration of function 'printf' [-Wimplicit-function-declaration]
     printf ("hello world\n");
     ^~~~~~
  test.c:3:3: warning: incompatible implicit declaration of built-in function 'printf'
  test.c:3:3: note: include '<stdio.h>' or provide a declaration of 'printf'

and which header file they are in.

However it doesn't know about various types and macros:

test.c:1:13: error: 'NULL' undeclared here (not in a function)
 void *ptr = NULL;
             ^~~~

This patch uses the name_hint/deferred_diagnostic machinery to
add hints for missing C standard library headers for some of the
most common type and macro names.

For example, the above becomes:
test.c:1:13: error: 'NULL' undeclared here (not in a function)
 void *ptr = NULL;
             ^~~~
test.c:1:13: note: 'NULL' is defined in header '<stddef.h>'; did you forget to '#include <stddef.h>'?

If the patch to add fix-it hints for missing #includes is approved:
  https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00321.html
then it's trivial to add a fix-it hint to the note.

gcc/c/ChangeLog:
	* c-decl.c (get_c_name_hint): New function.
	(class suggest_missing_header): New class.
	(lookup_name_fuzzy): Call get_c_name_hint and use it to
	suggest missing headers to the user.

gcc/testsuite/ChangeLog:
	* gcc.dg/spellcheck-stdlib.c: New test case.
---
 gcc/c/c-decl.c                           | 87 +++++++++++++++++++++++++++++++-
 gcc/testsuite/gcc.dg/spellcheck-stdlib.c | 55 ++++++++++++++++++++
 2 files changed, 140 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-stdlib.c

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 64075f9..d3c2bc5 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4000,6 +4000,78 @@ lookup_name_in_scope (tree name, struct c_scope *scope)
   return NULL_TREE;
 }
 
+/* Subroutine of suggest_missing_header::emit for handling unrecognized names
+   for some of the most common names within the C standard library.
+   Given non-NULL NAME, return the header name defining it within the C
+   standard library (with '<' and '>'), or NULL.  */
+
+static const char *
+get_c_name_hint (const char *name)
+{
+  struct std_name_hint
+  {
+    const char *name;
+    const char *header;
+  };
+  static const std_name_hint hints[] = {
+    /* <errno.h>.  */
+    {"errno", "<errno.h>"},
+
+    /* <stdarg.h>.  */
+    {"va_list", "<stdarg.h>"},
+
+    /* <stddef.h>.  */
+    {"NULL", "<stddef.h>"},
+    {"ptrdiff_t", "<stddef.h>"},
+    {"wchar_t", "<stddef.h>"},
+    {"size_t", "<stddef.h>"},
+
+    /* <stdio.h>.  */
+    {"BUFSIZ", "<stdio.h>"},
+    {"EOF", "<stdio.h>"},
+    {"FILE", "<stdio.h>"},
+    {"FILENAME_MAX", "<stdio.h>"},
+    {"fpos_t", "<stdio.h>"},
+    {"stderr", "<stdio.h>"},
+    {"stdin", "<stdio.h>"},
+    {"stdout", "<stdio.h>"}
+  };
+  const size_t num_hints = sizeof (hints) / sizeof (hints[0]);
+  for (size_t i = 0; i < num_hints; i++)
+    {
+      if (0 == strcmp (name, hints[i].name))
+	return hints[i].header;
+    }
+  return NULL;
+}
+
+/* Subclass of deferred_diagnostic for suggesting to the user
+   that they have missed a #include.  */
+
+class suggest_missing_header : public deferred_diagnostic
+{
+ public:
+  suggest_missing_header (location_t loc, const char *name,
+			  const char *header_hint)
+  : deferred_diagnostic (loc), m_name_str (name), m_header_hint (header_hint)
+  {
+    gcc_assert (name);
+    gcc_assert (header_hint);
+  }
+
+  void emit ()
+  {
+    inform (get_location (),
+	    "%qs is defined in header %qs;"
+	    " did you forget to %<#include %s%>?",
+	    m_name_str, m_header_hint, m_header_hint);
+  }
+
+ private:
+  const char *m_name_str;
+  const char *m_header_hint;
+};
+
 /* Look for the closest match for NAME within the currently valid
    scopes.
 
@@ -4014,13 +4086,24 @@ lookup_name_in_scope (tree name, struct c_scope *scope)
    identifier to the C frontend.
 
    It also looks for start_typename keywords, to detect "singed" vs "signed"
-   typos.  */
+   typos.
+
+   Use LOC for any deferred diagnostics.  */
 
 name_hint
-lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t)
+lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t loc)
 {
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
+  /* First, try some well-known names in the C standard library, in case
+     the user forgot a #include.  */
+  const char *header_hint = get_c_name_hint (IDENTIFIER_POINTER (name));
+  if (header_hint)
+    return name_hint (NULL,
+		      new suggest_missing_header (loc,
+						  IDENTIFIER_POINTER (name),
+						  header_hint));
+
   best_match<tree, tree> bm (name);
 
   /* Look within currently valid scopes.  */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-stdlib.c b/gcc/testsuite/gcc.dg/spellcheck-stdlib.c
new file mode 100644
index 0000000..85a21c3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-stdlib.c
@@ -0,0 +1,55 @@
+/* Missing <stddef.h>.  */
+
+void *ptr = NULL; /* { dg-error "'NULL' undeclared here" } */
+/* { dg-message "'NULL' is defined in header '<stddef.h>'; did you forget to '#include <stddef.h>'?" "" { target *-*-* } .-1 } */
+
+ptrdiff_t pd; /* { dg-error "unknown type name 'ptrdiff_t'" } */
+/* { dg-message "'ptrdiff_t' is defined in header '<stddef.h>'; did you forget to '#include <stddef.h>'?" "" { target *-*-* } .-1 } */
+
+wchar_t wc; /* { dg-error "unknown type name 'wchar_t'" } */
+/* { dg-message "'wchar_t' is defined in header '<stddef.h>'; did you forget to '#include <stddef.h>'?" "" { target *-*-* } .-1 } */
+
+size_t sz; /* { dg-error "unknown type name 'size_t'" } */
+/* { dg-message "'size_t' is defined in header '<stddef.h>'; did you forget to '#include <stddef.h>'?" "" { target *-*-* } .-1 } */
+
+/* Missing <stdio.h>.  */
+
+void test_stdio_h (void)
+{
+  FILE *f; /* { dg-error "unknown type name 'FILE'" } */
+  /* { dg-message "'FILE' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?" "" { target *-*-* } .-1 } */
+
+  char buf[BUFSIZ]; /* { dg-error "'BUFSIZ' undeclared" } */
+  /* { dg-message "'BUFSIZ' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?" "" { target *-*-* } .-1 } */
+
+  char buf2[FILENAME_MAX]; /* { dg-error "'FILENAME_MAX' undeclared" } */
+  /* { dg-message "'FILENAME_MAX' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?" "" { target *-*-* } .-1 } */
+
+  stderr; /* { dg-error "'stderr' undeclared" } */
+  /* { dg-message "'stderr' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?" "" { target *-*-* } .-1 } */
+
+  stdin; /* { dg-error "'stdin' undeclared" } */
+  /* { dg-message "'stdin' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?" "" { target *-*-* } .-1 } */
+
+  stdout; /* { dg-error "'stdout' undeclared" } */
+  /* { dg-message "'stdout' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?" "" { target *-*-* } .-1 } */
+
+  EOF; /* { dg-error "'EOF' undeclared" } */
+  /* { dg-message "'EOF' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?" "" { target *-*-* } .-1 } */
+}
+
+/* Missing <errno.h>.  */
+
+int test_errno_h (void)
+{
+  return errno; /* { dg-error "'errno' undeclared" } */
+  /* { dg-message "'errno' is defined in header '<errno.h>'; did you forget to '#include <errno.h>'?" "" { target *-*-* } .-1 } */
+}
+
+/* Missing <stdarg.h>.  */
+
+void test_stdarg_h (void)
+{
+  va_list ap; /* { dg-error "unknown type name 'va_list'" } */
+  /* { dg-message "'va_list' is defined in header '<stdarg.h>'; did you forget to '#include <stdarg.h>'?" "" { target *-*-* } .-1 } */
+}
-- 
1.8.5.3

[PATCH 1/3] c-family: add name_hint/deferred_diagnostic

[David Malcolm]

In various places we use lookup_name_fuzzy to provide a hint,
and can report messages of the form:
  error: unknown foo named 'bar'
or:
  error: unknown foo named 'bar'; did you mean 'SUGGESTION?

This patch provides a way for lookup_name_fuzzy to provide
both the suggestion above, and (optionally) additional hints
that can be printed e.g.

  note: did you forget to include <SOME_HEADER.h>?

This patch provides the mechanism and ports existing users
of lookup_name_fuzzy to the new return type.
There are no uses of such hints in this patch, but followup
patches provide various front-end specific uses of this.

gcc/c-family/ChangeLog:
	* c-common.h (class deferred_diagnostic): New class.
	(class name_hint): New class.
	(lookup_name_fuzzy): Convert return type from const char *
	to name_hint.  Add location_t param.

gcc/c/ChangeLog:
	* c-decl.c (implicit_decl_warning): Convert "hint" from
	const char * to name_hint.  Pass location to
	lookup_name_fuzzy.  Suppress any deferred diagnostic if the
	warning was not printed.
	(undeclared_variable): Likewise for "guessed_id".
	(lookup_name_fuzzy): Convert return type from const char *
	to name_hint.  Add location_t param.
	* c-parser.c (c_parser_declaration_or_fndef): Convert "hint" from
	const char * to name_hint.  Pass location to lookup_name_fuzzy.
	(c_parser_parameter_declaration): Pass location to
	lookup_name_fuzzy.

gcc/cp/ChangeLog:
	* name-lookup.c (suggest_alternatives_for): Convert "fuzzy_name" from
	const char * to name_hint, and rename to "hint".  Pass location to
	lookup_name_fuzzy.
	(lookup_name_fuzzy): Convert return type from const char *
	to name_hint.  Add location_t param.
	* parser.c (cp_parser_diagnose_invalid_type_name): Convert
	"suggestion" from const char * to name_hint, and rename to "hint".
	Pass location to lookup_name_fuzzy.
---
 gcc/c-family/c-common.h | 121 +++++++++++++++++++++++++++++++++++++++++++++++-
 gcc/c/c-decl.c          |  35 +++++++-------
 gcc/c/c-parser.c        |  16 ++++---
 gcc/cp/name-lookup.c    |  17 +++----
 gcc/cp/parser.c         |  12 ++---
 5 files changed, 163 insertions(+), 38 deletions(-)

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 138a0a6..83c1a68 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1009,7 +1009,126 @@ enum lookup_name_fuzzy_kind {
   /* Any name.  */
   FUZZY_LOOKUP_NAME
 };
-extern const char *lookup_name_fuzzy (tree, enum lookup_name_fuzzy_kind);
+
+/* A deferred_diagnostic is a wrapper around optional extra diagnostics
+   that we may want to bundle into a name_hint.
+
+   The emit method is called when no name_hint instances reference
+   the deferred_diagnostic.  In the simple case this is when the name_hint
+   goes out of scope, but a reference-counting scheme is used to allow
+   name_hint instances to be copied.  */
+
+class deferred_diagnostic
+{
+ public:
+  virtual ~deferred_diagnostic () {}
+  virtual void emit () = 0;
+
+  void incref () { m_refcnt++; }
+  void decref ()
+  {
+    if (--m_refcnt == 0)
+      {
+	if (!m_suppress)
+	  emit ();
+	delete this;
+      }
+  }
+
+  location_t get_location () const { return m_loc; }
+
+  /* Call this if the corresponding warning was not emitted,
+     in which case we should also not emit the deferred_diagnostic.  */
+  void suppress ()
+  {
+    m_suppress = true;
+  }
+
+ protected:
+  deferred_diagnostic (location_t loc)
+  : m_refcnt (0), m_loc (loc), m_suppress (false) {}
+
+ private:
+  int m_refcnt;
+  location_t m_loc;
+  bool m_suppress;
+};
+
+/* A name_hint is an optional string suggestion, along with an
+   optional deferred_diagnostic.
+   For example:
+
+       error: unknown foo named 'bar'
+
+   if the SUGGESTION is "baz", then one might print:
+
+       error: unknown foo named 'bar'; did you mean 'baz'?
+
+   and the deferred_diagnostic allows for additional (optional)
+   diagnostics e.g.:
+
+       note: did you check behind the couch?
+
+   The deferred_diagnostic is emitted when no name_hint instances reference
+   the deferred_diagnostic.  In the simple case this is when the name_hint
+   goes out of scope, but a reference-counting scheme is used to allow
+   name_hint instances to be copied.  */
+
+class name_hint
+{
+public:
+  name_hint () : m_suggestion (NULL), m_deferred (NULL) {}
+
+  name_hint (const char *suggestion, deferred_diagnostic *deferred)
+  : m_suggestion (suggestion), m_deferred (deferred)
+  {
+    if (m_deferred)
+      m_deferred->incref ();
+  }
+
+  name_hint (const name_hint &other)
+  : m_suggestion (other.m_suggestion), m_deferred (other.m_deferred)
+  {
+    if (m_deferred)
+      m_deferred->incref ();
+  }
+
+  name_hint& operator= (const name_hint &other)
+  {
+    m_suggestion = other.m_suggestion;
+    if (other.m_deferred)
+      other.m_deferred->incref ();
+    if (m_deferred)
+      m_deferred->decref ();
+    m_deferred = other.m_deferred;
+    return *this;
+  }
+
+  ~name_hint ()
+  {
+    if (m_deferred)
+      m_deferred->decref ();
+  }
+
+  const char *suggestion () const { return m_suggestion; }
+  operator bool () const { return m_suggestion != NULL; }
+
+  /* Call this on a name_hint if the corresponding warning was not emitted,
+     in which case we should also not emit the deferred_diagnostic.  */
+
+  void suppress ()
+  {
+    if (m_deferred)
+      m_deferred->suppress ();
+  }
+
+private:
+  const char *m_suggestion;
+  deferred_diagnostic *m_deferred;
+};
+
+extern name_hint lookup_name_fuzzy (tree, enum lookup_name_fuzzy_kind,
+				    location_t);
 
 extern bool vector_targets_convertible_p (const_tree t1, const_tree t2);
 extern bool vector_types_convertible_p (const_tree t1, const_tree t2, bool emit_lax_note);
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 64a1107..64075f9 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -3129,20 +3129,20 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
     return;
 
   bool warned;
-  const char *hint = NULL;
+  name_hint hint;
   if (!olddecl)
-    hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_FUNCTION_NAME);
+    hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_FUNCTION_NAME, loc);
 
   if (flag_isoc99)
     {
       if (hint)
 	{
 	  gcc_rich_location richloc (loc);
-	  richloc.add_fixit_replace (hint);
+	  richloc.add_fixit_replace (hint.suggestion ());
 	  warned = pedwarn_at_rich_loc
 	    (&richloc, OPT_Wimplicit_function_declaration,
 	     "implicit declaration of function %qE; did you mean %qs?",
-	     id, hint);
+	     id, hint.suggestion ());
 	}
       else
 	warned = pedwarn (loc, OPT_Wimplicit_function_declaration,
@@ -3151,11 +3151,11 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
   else if (hint)
     {
       gcc_rich_location richloc (loc);
-      richloc.add_fixit_replace (hint);
+      richloc.add_fixit_replace (hint.suggestion ());
       warned = warning_at_rich_loc
 	(&richloc, OPT_Wimplicit_function_declaration,
 	 G_("implicit declaration of function %qE; did you mean %qs?"),
-	 id, hint);
+	 id, hint.suggestion ());
     }
   else
     warned = warning_at (loc, OPT_Wimplicit_function_declaration,
@@ -3163,6 +3163,9 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
 
   if (olddecl && warned)
     locate_old_decl (olddecl);
+
+  if (!warned)
+    hint.suppress ();
 }
 
 /* This function represents mapping of a function code FCODE
@@ -3475,15 +3478,15 @@ undeclared_variable (location_t loc, tree id)
 
   if (current_function_decl == 0)
     {
-      const char *guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME);
+      name_hint guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME, loc);
       if (guessed_id)
 	{
 	  gcc_rich_location richloc (loc);
-	  richloc.add_fixit_replace (guessed_id);
+	  richloc.add_fixit_replace (guessed_id.suggestion ());
 	  error_at_rich_loc (&richloc,
 			     "%qE undeclared here (not in a function);"
 			     " did you mean %qs?",
-			     id, guessed_id);
+			     id, guessed_id.suggestion ());
 	}
       else
 	error_at (loc, "%qE undeclared here (not in a function)", id);
@@ -3493,16 +3496,16 @@ undeclared_variable (location_t loc, tree id)
     {
       if (!objc_diagnose_private_ivar (id))
 	{
-	  const char *guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME);
+	  name_hint guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME, loc);
 	  if (guessed_id)
 	    {
 	      gcc_rich_location richloc (loc);
-	      richloc.add_fixit_replace (guessed_id);
+	      richloc.add_fixit_replace (guessed_id.suggestion ());
 	      error_at_rich_loc
 		(&richloc,
 		 "%qE undeclared (first use in this function);"
 		 " did you mean %qs?",
-		 id, guessed_id);
+		 id, guessed_id.suggestion ());
 	    }
 	  else
 	    error_at (loc, "%qE undeclared (first use in this function)", id);
@@ -4013,8 +4016,8 @@ lookup_name_in_scope (tree name, struct c_scope *scope)
    It also looks for start_typename keywords, to detect "singed" vs "signed"
    typos.  */
 
-const char *
-lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
+name_hint
+lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t)
 {
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
@@ -4104,9 +4107,9 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
 
   tree best = bm.get_best_meaningful_candidate ();
   if (best)
-    return IDENTIFIER_POINTER (best);
+    return name_hint (IDENTIFIER_POINTER (best), NULL);
   else
-    return NULL;
+    return name_hint (NULL, NULL);
 }
 
 \f
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 9398652..550b573 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -1623,13 +1623,14 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 	}
       else
 	{
-	  const char *hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_TYPENAME);
+	  name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_TYPENAME,
+					      here);
 	  if (hint)
 	    {
-	      richloc.add_fixit_replace (hint);
+	      richloc.add_fixit_replace (hint.suggestion ());
 	      error_at_rich_loc (&richloc,
 				 "unknown type name %qE; did you mean %qs?",
-				 name, hint);
+				 name, hint.suggestion ());
 	    }
 	  else
 	    error_at (here, "unknown type name %qE", name);
@@ -3848,15 +3849,16 @@ c_parser_parameter_declaration (c_parser *parser, tree attrs)
       c_parser_set_source_position_from_token (token);
       if (c_parser_next_tokens_start_typename (parser, cla_prefer_type))
 	{
-	  const char *hint = lookup_name_fuzzy (token->value,
-						FUZZY_LOOKUP_TYPENAME);
+	  name_hint hint = lookup_name_fuzzy (token->value,
+					      FUZZY_LOOKUP_TYPENAME,
+					      token->location);
 	  if (hint)
 	    {
 	      gcc_rich_location richloc (token->location);
-	      richloc.add_fixit_replace (hint);
+	      richloc.add_fixit_replace (hint.suggestion ());
 	      error_at_rich_loc (&richloc,
 				 "unknown type name %qE; did you mean %qs?",
-				 token->value, hint);
+				 token->value, hint.suggestion ());
 	    }
 	  else
 	    error_at (token->location, "unknown type name %qE", token->value);
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 0c5df93..de8c267 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -4515,13 +4515,14 @@ suggest_alternatives_for (location_t location, tree name,
     {
       if (suggest_misspellings)
 	{
-	  const char *fuzzy_name = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME);
-	  if (fuzzy_name)
+	  name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
+					      location);
+	  if (hint)
 	    {
 	      gcc_rich_location richloc (location);
-	      richloc.add_fixit_replace (fuzzy_name);
+	      richloc.add_fixit_replace (hint.suggestion ());
 	      inform_at_rich_loc (&richloc, "suggested alternative: %qs",
-				  fuzzy_name);
+				  hint.suggestion ());
 	    }
 	}
       return;
@@ -4953,10 +4954,10 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
 
 /* Search for near-matches for NAME within the current bindings, and within
    macro names, returning the best match as a const char *, or NULL if
-   no reasonable match is found.  */
+   no reasonable match is found. */
 
-const char *
-lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
+name_hint
+lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t)
 {
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
@@ -5010,7 +5011,7 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
       bm.consider (IDENTIFIER_POINTER (resword_identifier));
     }
 
-  return bm.get_best_meaningful_candidate ();
+  return name_hint (bm.get_best_meaningful_candidate (), NULL);
 }
 
 /* Subroutine of outer_binding.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 7197c19..6e9f05c 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -3173,16 +3173,16 @@ cp_parser_diagnose_invalid_type_name (cp_parser *parser, tree id,
   else if (!parser->scope)
     {
       /* Issue an error message.  */
-      const char *suggestion = NULL;
+      name_hint hint;
       if (TREE_CODE (id) == IDENTIFIER_NODE)
-        suggestion = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME);
-      if (suggestion)
+	hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME, location);
+      if (hint)
 	{
 	  gcc_rich_location richloc (location);
-	  richloc.add_fixit_replace (suggestion);
+	  richloc.add_fixit_replace (hint.suggestion ());
 	  error_at_rich_loc (&richloc,
-			     "%qE does not name a type; did you mean %qs?",
-			     id, suggestion);
+			    "%qE does not name a type; did you mean %qs?",
+			    id, hint.suggestion ());
 	}
       else
 	error_at (location, "%qE does not name a type", id);
-- 
1.8.5.3

[PING] Re: [PATCH 1/3] c-family: add name_hint/deferred_diagnostic

[David Malcolm]

I'd like to ping the following patches:

  [PATCH 1/3] c-family: add name_hint/deferred_diagnostic
    https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00439.html

  [PATCH 2/3] C++: provide macro used-before-defined hint (PR c++/72786).
    https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00437.html

  [PATCH 3/3] C: hints for missing stdlib includes for macros and types
    https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00438.html

On Fri, 2017-05-05 at 13:51 -0400, David Malcolm wrote:
> In various places we use lookup_name_fuzzy to provide a hint,
> and can report messages of the form:
>   error: unknown foo named 'bar'
> or:
>   error: unknown foo named 'bar'; did you mean 'SUGGESTION?
> 
> This patch provides a way for lookup_name_fuzzy to provide
> both the suggestion above, and (optionally) additional hints
> that can be printed e.g.
> 
>   note: did you forget to include <SOME_HEADER.h>?
> 
> This patch provides the mechanism and ports existing users
> of lookup_name_fuzzy to the new return type.
> There are no uses of such hints in this patch, but followup
> patches provide various front-end specific uses of this.
> 
> gcc/c-family/ChangeLog:
> 	* c-common.h (class deferred_diagnostic): New class.
> 	(class name_hint): New class.
> 	(lookup_name_fuzzy): Convert return type from const char *
> 	to name_hint.  Add location_t param.
> 
> gcc/c/ChangeLog:
> 	* c-decl.c (implicit_decl_warning): Convert "hint" from
> 	const char * to name_hint.  Pass location to
> 	lookup_name_fuzzy.  Suppress any deferred diagnostic if the
> 	warning was not printed.
> 	(undeclared_variable): Likewise for "guessed_id".
> 	(lookup_name_fuzzy): Convert return type from const char *
> 	to name_hint.  Add location_t param.
> 	* c-parser.c (c_parser_declaration_or_fndef): Convert "hint"
> from
> 	const char * to name_hint.  Pass location to lookup_name_fuzzy.
> 	(c_parser_parameter_declaration): Pass location to
> 	lookup_name_fuzzy.
> 
> gcc/cp/ChangeLog:
> 	* name-lookup.c (suggest_alternatives_for): Convert
> "fuzzy_name" from
> 	const char * to name_hint, and rename to "hint".  Pass location
> to
> 	lookup_name_fuzzy.
> 	(lookup_name_fuzzy): Convert return type from const char *
> 	to name_hint.  Add location_t param.
> 	* parser.c (cp_parser_diagnose_invalid_type_name): Convert
> 	"suggestion" from const char * to name_hint, and rename to
> "hint".
> 	Pass location to lookup_name_fuzzy.
> ---
>  gcc/c-family/c-common.h | 121
> +++++++++++++++++++++++++++++++++++++++++++++++-
>  gcc/c/c-decl.c          |  35 +++++++-------
>  gcc/c/c-parser.c        |  16 ++++---
>  gcc/cp/name-lookup.c    |  17 +++----
>  gcc/cp/parser.c         |  12 ++---
>  5 files changed, 163 insertions(+), 38 deletions(-)
> 
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 138a0a6..83c1a68 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1009,7 +1009,126 @@ enum lookup_name_fuzzy_kind {
>    /* Any name.  */
>    FUZZY_LOOKUP_NAME
>  };
> -extern const char *lookup_name_fuzzy (tree, enum
> lookup_name_fuzzy_kind);
> +
> +/* A deferred_diagnostic is a wrapper around optional extra
> diagnostics
> +   that we may want to bundle into a name_hint.
> +
> +   The emit method is called when no name_hint instances reference
> +   the deferred_diagnostic.  In the simple case this is when the
> name_hint
> +   goes out of scope, but a reference-counting scheme is used to
> allow
> +   name_hint instances to be copied.  */
> +
> +class deferred_diagnostic
> +{
> + public:
> +  virtual ~deferred_diagnostic () {}
> +  virtual void emit () = 0;
> +
> +  void incref () { m_refcnt++; }
> +  void decref ()
> +  {
> +    if (--m_refcnt == 0)
> +      {
> +	if (!m_suppress)
> +	  emit ();
> +	delete this;
> +      }
> +  }
> +
> +  location_t get_location () const { return m_loc; }
> +
> +  /* Call this if the corresponding warning was not emitted,
> +     in which case we should also not emit the deferred_diagnostic. 
>  */
> +  void suppress ()
> +  {
> +    m_suppress = true;
> +  }
> +
> + protected:
> +  deferred_diagnostic (location_t loc)
> +  : m_refcnt (0), m_loc (loc), m_suppress (false) {}
> +
> + private:
> +  int m_refcnt;
> +  location_t m_loc;
> +  bool m_suppress;
> +};
> +
> +/* A name_hint is an optional string suggestion, along with an
> +   optional deferred_diagnostic.
> +   For example:
> +
> +       error: unknown foo named 'bar'
> +
> +   if the SUGGESTION is "baz", then one might print:
> +
> +       error: unknown foo named 'bar'; did you mean 'baz'?
> +
> +   and the deferred_diagnostic allows for additional (optional)
> +   diagnostics e.g.:
> +
> +       note: did you check behind the couch?
> +
> +   The deferred_diagnostic is emitted when no name_hint instances
> reference
> +   the deferred_diagnostic.  In the simple case this is when the
> name_hint
> +   goes out of scope, but a reference-counting scheme is used to
> allow
> +   name_hint instances to be copied.  */
> +
> +class name_hint
> +{
> +public:
> +  name_hint () : m_suggestion (NULL), m_deferred (NULL) {}
> +
> +  name_hint (const char *suggestion, deferred_diagnostic *deferred)
> +  : m_suggestion (suggestion), m_deferred (deferred)
> +  {
> +    if (m_deferred)
> +      m_deferred->incref ();
> +  }
> +
> +  name_hint (const name_hint &other)
> +  : m_suggestion (other.m_suggestion), m_deferred (other.m_deferred)
> +  {
> +    if (m_deferred)
> +      m_deferred->incref ();
> +  }
> +
> +  name_hint& operator= (const name_hint &other)
> +  {
> +    m_suggestion = other.m_suggestion;
> +    if (other.m_deferred)
> +      other.m_deferred->incref ();
> +    if (m_deferred)
> +      m_deferred->decref ();
> +    m_deferred = other.m_deferred;
> +    return *this;
> +  }
> +
> +  ~name_hint ()
> +  {
> +    if (m_deferred)
> +      m_deferred->decref ();
> +  }
> +
> +  const char *suggestion () const { return m_suggestion; }
> +  operator bool () const { return m_suggestion != NULL; }
> +
> +  /* Call this on a name_hint if the corresponding warning was not
> emitted,
> +     in which case we should also not emit the deferred_diagnostic. 
>  */
> +
> +  void suppress ()
> +  {
> +    if (m_deferred)
> +      m_deferred->suppress ();
> +  }
> +
> +private:
> +  const char *m_suggestion;
> +  deferred_diagnostic *m_deferred;
> +};
> +
> +extern name_hint lookup_name_fuzzy (tree, enum
> lookup_name_fuzzy_kind,
> +				    location_t);
>  
>  extern bool vector_targets_convertible_p (const_tree t1, const_tree
> t2);
>  extern bool vector_types_convertible_p (const_tree t1, const_tree
> t2, bool emit_lax_note);
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index 64a1107..64075f9 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -3129,20 +3129,20 @@ implicit_decl_warning (location_t loc, tree
> id, tree olddecl)
>      return;
>  
>    bool warned;
> -  const char *hint = NULL;
> +  name_hint hint;
>    if (!olddecl)
> -    hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_FUNCTION_NAME);
> +    hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_FUNCTION_NAME, loc);
>  
>    if (flag_isoc99)
>      {
>        if (hint)
>  	{
>  	  gcc_rich_location richloc (loc);
> -	  richloc.add_fixit_replace (hint);
> +	  richloc.add_fixit_replace (hint.suggestion ());
>  	  warned = pedwarn_at_rich_loc
>  	    (&richloc, OPT_Wimplicit_function_declaration,
>  	     "implicit declaration of function %qE; did you mean
> %qs?",
> -	     id, hint);
> +	     id, hint.suggestion ());
>  	}
>        else
>  	warned = pedwarn (loc, OPT_Wimplicit_function_declaration,
> @@ -3151,11 +3151,11 @@ implicit_decl_warning (location_t loc, tree
> id, tree olddecl)
>    else if (hint)
>      {
>        gcc_rich_location richloc (loc);
> -      richloc.add_fixit_replace (hint);
> +      richloc.add_fixit_replace (hint.suggestion ());
>        warned = warning_at_rich_loc
>  	(&richloc, OPT_Wimplicit_function_declaration,
>  	 G_("implicit declaration of function %qE; did you mean
> %qs?"),
> -	 id, hint);
> +	 id, hint.suggestion ());
>      }
>    else
>      warned = warning_at (loc, OPT_Wimplicit_function_declaration,
> @@ -3163,6 +3163,9 @@ implicit_decl_warning (location_t loc, tree id,
> tree olddecl)
>  
>    if (olddecl && warned)
>      locate_old_decl (olddecl);
> +
> +  if (!warned)
> +    hint.suppress ();
>  }
>  
>  /* This function represents mapping of a function code FCODE
> @@ -3475,15 +3478,15 @@ undeclared_variable (location_t loc, tree id)
>  
>    if (current_function_decl == 0)
>      {
> -      const char *guessed_id = lookup_name_fuzzy (id,
> FUZZY_LOOKUP_NAME);
> +      name_hint guessed_id = lookup_name_fuzzy (id,
> FUZZY_LOOKUP_NAME, loc);
>        if (guessed_id)
>  	{
>  	  gcc_rich_location richloc (loc);
> -	  richloc.add_fixit_replace (guessed_id);
> +	  richloc.add_fixit_replace (guessed_id.suggestion ());
>  	  error_at_rich_loc (&richloc,
>  			     "%qE undeclared here (not in a
> function);"
>  			     " did you mean %qs?",
> -			     id, guessed_id);
> +			     id, guessed_id.suggestion ());
>  	}
>        else
>  	error_at (loc, "%qE undeclared here (not in a function)",
> id);
> @@ -3493,16 +3496,16 @@ undeclared_variable (location_t loc, tree id)
>      {
>        if (!objc_diagnose_private_ivar (id))
>  	{
> -	  const char *guessed_id = lookup_name_fuzzy (id,
> FUZZY_LOOKUP_NAME);
> +	  name_hint guessed_id = lookup_name_fuzzy (id,
> FUZZY_LOOKUP_NAME, loc);
>  	  if (guessed_id)
>  	    {
>  	      gcc_rich_location richloc (loc);
> -	      richloc.add_fixit_replace (guessed_id);
> +	      richloc.add_fixit_replace (guessed_id.suggestion ());
>  	      error_at_rich_loc
>  		(&richloc,
>  		 "%qE undeclared (first use in this function);"
>  		 " did you mean %qs?",
> -		 id, guessed_id);
> +		 id, guessed_id.suggestion ());
>  	    }
>  	  else
>  	    error_at (loc, "%qE undeclared (first use in this
> function)", id);
> @@ -4013,8 +4016,8 @@ lookup_name_in_scope (tree name, struct c_scope
> *scope)
>     It also looks for start_typename keywords, to detect "singed" vs
> "signed"
>     typos.  */
>  
> -const char *
> -lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
> +name_hint
> +lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind,
> location_t)
>  {
>    gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
>  
> @@ -4104,9 +4107,9 @@ lookup_name_fuzzy (tree name, enum
> lookup_name_fuzzy_kind kind)
>  
>    tree best = bm.get_best_meaningful_candidate ();
>    if (best)
> -    return IDENTIFIER_POINTER (best);
> +    return name_hint (IDENTIFIER_POINTER (best), NULL);
>    else
> -    return NULL;
> +    return name_hint (NULL, NULL);
>  }
>  
>  \f
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index 9398652..550b573 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -1623,13 +1623,14 @@ c_parser_declaration_or_fndef (c_parser
> *parser, bool fndef_ok,
>  	}
>        else
>  	{
> -	  const char *hint = lookup_name_fuzzy (name,
> FUZZY_LOOKUP_TYPENAME);
> +	  name_hint hint = lookup_name_fuzzy (name,
> FUZZY_LOOKUP_TYPENAME,
> +					      here);
>  	  if (hint)
>  	    {
> -	      richloc.add_fixit_replace (hint);
> +	      richloc.add_fixit_replace (hint.suggestion ());
>  	      error_at_rich_loc (&richloc,
>  				 "unknown type name %qE; did you
> mean %qs?",
> -				 name, hint);
> +				 name, hint.suggestion ());
>  	    }
>  	  else
>  	    error_at (here, "unknown type name %qE", name);
> @@ -3848,15 +3849,16 @@ c_parser_parameter_declaration (c_parser
> *parser, tree attrs)
>        c_parser_set_source_position_from_token (token);
>        if (c_parser_next_tokens_start_typename (parser,
> cla_prefer_type))
>  	{
> -	  const char *hint = lookup_name_fuzzy (token->value,
> -						FUZZY_LOOKUP_TYPENAM
> E);
> +	  name_hint hint = lookup_name_fuzzy (token->value,
> +					      FUZZY_LOOKUP_TYPENAME,
> +					      token->location);
>  	  if (hint)
>  	    {
>  	      gcc_rich_location richloc (token->location);
> -	      richloc.add_fixit_replace (hint);
> +	      richloc.add_fixit_replace (hint.suggestion ());
>  	      error_at_rich_loc (&richloc,
>  				 "unknown type name %qE; did you
> mean %qs?",
> -				 token->value, hint);
> +				 token->value, hint.suggestion ());
>  	    }
>  	  else
>  	    error_at (token->location, "unknown type name %qE",
> token->value);
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 0c5df93..de8c267 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -4515,13 +4515,14 @@ suggest_alternatives_for (location_t
> location, tree name,
>      {
>        if (suggest_misspellings)
>  	{
> -	  const char *fuzzy_name = lookup_name_fuzzy (name,
> FUZZY_LOOKUP_NAME);
> -	  if (fuzzy_name)
> +	  name_hint hint = lookup_name_fuzzy (name,
> FUZZY_LOOKUP_NAME,
> +					      location);
> +	  if (hint)
>  	    {
>  	      gcc_rich_location richloc (location);
> -	      richloc.add_fixit_replace (fuzzy_name);
> +	      richloc.add_fixit_replace (hint.suggestion ());
>  	      inform_at_rich_loc (&richloc, "suggested alternative:
> %qs",
> -				  fuzzy_name);
> +				  hint.suggestion ());
>  	    }
>  	}
>        return;
> @@ -4953,10 +4954,10 @@ consider_binding_level (tree name, best_match
> <tree, const char *> &bm,
>  
>  /* Search for near-matches for NAME within the current bindings, and
> within
>     macro names, returning the best match as a const char *, or NULL
> if
> -   no reasonable match is found.  */
> +   no reasonable match is found. */
>  
> -const char *
> -lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
> +name_hint
> +lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind,
> location_t)
>  {
>    gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
>  
> @@ -5010,7 +5011,7 @@ lookup_name_fuzzy (tree name, enum
> lookup_name_fuzzy_kind kind)
>        bm.consider (IDENTIFIER_POINTER (resword_identifier));
>      }
>  
> -  return bm.get_best_meaningful_candidate ();
> +  return name_hint (bm.get_best_meaningful_candidate (), NULL);
>  }
>  
>  /* Subroutine of outer_binding.
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 7197c19..6e9f05c 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -3173,16 +3173,16 @@ cp_parser_diagnose_invalid_type_name
> (cp_parser *parser, tree id,
>    else if (!parser->scope)
>      {
>        /* Issue an error message.  */
> -      const char *suggestion = NULL;
> +      name_hint hint;
>        if (TREE_CODE (id) == IDENTIFIER_NODE)
> -        suggestion = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME);
> -      if (suggestion)
> +	hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME,
> location);
> +      if (hint)
>  	{
>  	  gcc_rich_location richloc (location);
> -	  richloc.add_fixit_replace (suggestion);
> +	  richloc.add_fixit_replace (hint.suggestion ());
>  	  error_at_rich_loc (&richloc,
> -			     "%qE does not name a type; did you mean
> %qs?",
> -			     id, suggestion);
> +			    "%qE does not name a type; did you mean
> %qs?",
> +			    id, hint.suggestion ());
>  	}
>        else
>  	error_at (location, "%qE does not name a type", id);

Re: [PATCH 1/3] c-family: add name_hint/deferred_diagnostic

[Jeff Law]

On 05/05/2017 11:51 AM, David Malcolm wrote:
> In various places we use lookup_name_fuzzy to provide a hint,
> and can report messages of the form:
>   error: unknown foo named 'bar'
> or:
>   error: unknown foo named 'bar'; did you mean 'SUGGESTION?
> 
> This patch provides a way for lookup_name_fuzzy to provide
> both the suggestion above, and (optionally) additional hints
> that can be printed e.g.
> 
>   note: did you forget to include <SOME_HEADER.h>?
> 
> This patch provides the mechanism and ports existing users
> of lookup_name_fuzzy to the new return type.
> There are no uses of such hints in this patch, but followup
> patches provide various front-end specific uses of this.
> 
> gcc/c-family/ChangeLog:
> 	* c-common.h (class deferred_diagnostic): New class.
> 	(class name_hint): New class.
> 	(lookup_name_fuzzy): Convert return type from const char *
> 	to name_hint.  Add location_t param.
> 
> gcc/c/ChangeLog:
> 	* c-decl.c (implicit_decl_warning): Convert "hint" from
> 	const char * to name_hint.  Pass location to
> 	lookup_name_fuzzy.  Suppress any deferred diagnostic if the
> 	warning was not printed.
> 	(undeclared_variable): Likewise for "guessed_id".
> 	(lookup_name_fuzzy): Convert return type from const char *
> 	to name_hint.  Add location_t param.
> 	* c-parser.c (c_parser_declaration_or_fndef): Convert "hint" from
> 	const char * to name_hint.  Pass location to lookup_name_fuzzy.
> 	(c_parser_parameter_declaration): Pass location to
> 	lookup_name_fuzzy.
> 
> gcc/cp/ChangeLog:
> 	* name-lookup.c (suggest_alternatives_for): Convert "fuzzy_name" from
> 	const char * to name_hint, and rename to "hint".  Pass location to
> 	lookup_name_fuzzy.
> 	(lookup_name_fuzzy): Convert return type from const char *
> 	to name_hint.  Add location_t param.
> 	* parser.c (cp_parser_diagnose_invalid_type_name): Convert
> 	"suggestion" from const char * to name_hint, and rename to "hint".
> 	Pass location to lookup_name_fuzzy.

> ---
>  gcc/c-family/c-common.h | 121 +++++++++++++++++++++++++++++++++++++++++++++++-
>  gcc/c/c-decl.c          |  35 +++++++-------
>  gcc/c/c-parser.c        |  16 ++++---
>  gcc/cp/name-lookup.c    |  17 +++----
>  gcc/cp/parser.c         |  12 ++---
>  5 files changed, 163 insertions(+), 38 deletions(-)
> 
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 138a0a6..83c1a68 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1009,7 +1009,126 @@ enum lookup_name_fuzzy_kind {
>    /* Any name.  */
>    FUZZY_LOOKUP_NAME
>  };
> -extern const char *lookup_name_fuzzy (tree, enum lookup_name_fuzzy_kind);
> +
> +/* A deferred_diagnostic is a wrapper around optional extra diagnostics
> +   that we may want to bundle into a name_hint.
> +
> +   The emit method is called when no name_hint instances reference
> +   the deferred_diagnostic.  In the simple case this is when the name_hint
> +   goes out of scope, but a reference-counting scheme is used to allow
> +   name_hint instances to be copied.  */
> +
> +class deferred_diagnostic
> +{
> + public:
> +  virtual ~deferred_diagnostic () {}
> +  virtual void emit () = 0;
> +
> +  void incref () { m_refcnt++; }
> +  void decref ()
> +  {
> +    if (--m_refcnt == 0)
> +      {
> +	if (!m_suppress)
> +	  emit ();
> +	delete this;
> +      }
> +  }
> +
> +  location_t get_location () const { return m_loc; }
> +
> +  /* Call this if the corresponding warning was not emitted,
> +     in which case we should also not emit the deferred_diagnostic.  */
> +  void suppress ()
> +  {
> +    m_suppress = true;
> +  }
> +
> + protected:
> +  deferred_diagnostic (location_t loc)
> +  : m_refcnt (0), m_loc (loc), m_suppress (false) {}
> +
> + private:
> +  int m_refcnt;
> +  location_t m_loc;
> +  bool m_suppress;
> +};
So what stands out here is "delete this" and the need for explicit
reference counting.  Also doesn't that imply that deferred_diagnostic
objects must be allocated on the heap?  Is there another way to get the
behavior you want without resorting to something like this?

Or is your argument that deferred_diagnostic is only used from within
class name_hint and thus the concerns around heap vs stack, explicit
counting, etc are all buried inside the name_hint class?  If so, is
there any reasonable way to restrict the use of deferred_disagnostic to
within the name_hint class?

The rest of the changes seem non-controversial, so I think if we can
sort out the issues with those classes then this will be fine to move
forward.

jeff

Re: [PATCH 2/3] C++: provide macro used-before-defined hint (PR c++/72786).

[Jeff Law]

On 05/05/2017 11:51 AM, David Malcolm wrote:
> This patch uses the name_hint/deferred_diagnostic to provide
> a message in the C++ frontend if a macro is used before it is defined
> e.g.:
> 
> test.c:6:24: error: expected ‘;’ at end of member declaration
>    virtual void clone() const OVERRIDE { }
>                         ^~~~~
>                              ;
> test.c:6:30: error: ‘OVERRIDE’ does not name a type
>    virtual void clone() const OVERRIDE { }
>                               ^~~~~~~~
> test.c:6:30: note: the macro ‘OVERRIDE’ had not yet been defined
> test.c:15:0: note: it was later defined here
>  #define OVERRIDE override
> 
> It's possible to do it from the C++ frontend as tokenization happens
> up-front (and hence the macro already exists when the above is parsed);
> I attempted to do it from the C frontend, but because the C frontend only
> tokenizes on-demand during parsing, the macro isn't known about until
> later.
> 
> gcc/cp/ChangeLog:
> 	PR c++/72786
> 	* name-lookup.c (class macro_use_before_def): New class.
> 	(lookup_name_fuzzy): Detect macro that were used before being
> 	defined, and report them as such.
> 
> gcc/ChangeLog:
> 	PR c++/72786
> 	* spellcheck.h (best_match::blithely_get_best_candidate): New
> 	accessor.
> 
> gcc/testsuite/ChangeLog:
> 	PR c++/72786
> 	* g++.dg/spellcheck-macro-ordering-2.C: New test case.
> 	* g++.dg/spellcheck-macro-ordering.C: Add dg-message directives
> 	for macro used-before-defined.
> 
> libcpp/ChangeLog:
> 	PR c++/72786
> 	* include/cpplib.h (cpp_macro_definition_location): New decl.
> 	* macro.c (cpp_macro_definition): New function.
This is fine once the prereq is approved.

jeff

Re: [PATCH 3/3] C: hints for missing stdlib includes for macros and types

[Jeff Law]

On 05/05/2017 11:51 AM, David Malcolm wrote:
> The C frontend already "knows" about many common functions in
> the C standard library:
> 
>   test.c: In function 'test':
>   test.c:3:3: warning: implicit declaration of function 'printf' [-Wimplicit-function-declaration]
>      printf ("hello world\n");
>      ^~~~~~
>   test.c:3:3: warning: incompatible implicit declaration of built-in function 'printf'
>   test.c:3:3: note: include '<stdio.h>' or provide a declaration of 'printf'
> 
> and which header file they are in.
> 
> However it doesn't know about various types and macros:
> 
> test.c:1:13: error: 'NULL' undeclared here (not in a function)
>  void *ptr = NULL;
>              ^~~~
> 
> This patch uses the name_hint/deferred_diagnostic machinery to
> add hints for missing C standard library headers for some of the
> most common type and macro names.
> 
> For example, the above becomes:
> test.c:1:13: error: 'NULL' undeclared here (not in a function)
>  void *ptr = NULL;
>              ^~~~
> test.c:1:13: note: 'NULL' is defined in header '<stddef.h>'; did you forget to '#include <stddef.h>'?
> 
> If the patch to add fix-it hints for missing #includes is approved:
>   https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00321.html
> then it's trivial to add a fix-it hint to the note.
> 
> gcc/c/ChangeLog:
> 	* c-decl.c (get_c_name_hint): New function.
> 	(class suggest_missing_header): New class.
> 	(lookup_name_fuzzy): Call get_c_name_hint and use it to
> 	suggest missing headers to the user.
> 
> gcc/testsuite/ChangeLog:
> 	* gcc.dg/spellcheck-stdlib.c: New test case.
OK once prereqs are approved.

FWIW, I'm getting a little concerned that we're adding a lot of overhead
to the error paths -- that often doesn't matter.  But sometimes it does
(testcase reduction, errors with machine generated code, etc).
Something to be aware of.


jeff

Re: [PATCH 1/3] c-family: add name_hint/deferred_diagnostic

[David Malcolm]

On Mon, 2017-07-03 at 10:25 -0600, Jeff Law wrote:
> On 05/05/2017 11:51 AM, David Malcolm wrote:
> > In various places we use lookup_name_fuzzy to provide a hint,
> > and can report messages of the form:
> >   error: unknown foo named 'bar'
> > or:
> >   error: unknown foo named 'bar'; did you mean 'SUGGESTION?
> > 
> > This patch provides a way for lookup_name_fuzzy to provide
> > both the suggestion above, and (optionally) additional hints
> > that can be printed e.g.
> > 
> >   note: did you forget to include <SOME_HEADER.h>?
> > 
> > This patch provides the mechanism and ports existing users
> > of lookup_name_fuzzy to the new return type.
> > There are no uses of such hints in this patch, but followup
> > patches provide various front-end specific uses of this.
> > 
> > gcc/c-family/ChangeLog:
> > 	* c-common.h (class deferred_diagnostic): New class.
> > 	(class name_hint): New class.
> > 	(lookup_name_fuzzy): Convert return type from const char *
> > 	to name_hint.  Add location_t param.
> > 
> > gcc/c/ChangeLog:
> > 	* c-decl.c (implicit_decl_warning): Convert "hint" from
> > 	const char * to name_hint.  Pass location to
> > 	lookup_name_fuzzy.  Suppress any deferred diagnostic if the
> > 	warning was not printed.
> > 	(undeclared_variable): Likewise for "guessed_id".
> > 	(lookup_name_fuzzy): Convert return type from const char *
> > 	to name_hint.  Add location_t param.
> > 	* c-parser.c (c_parser_declaration_or_fndef): Convert "hint"
> > from
> > 	const char * to name_hint.  Pass location to lookup_name_fuzzy.
> > 	(c_parser_parameter_declaration): Pass location to
> > 	lookup_name_fuzzy.
> > 
> > gcc/cp/ChangeLog:
> > 	* name-lookup.c (suggest_alternatives_for): Convert
> > "fuzzy_name" from
> > 	const char * to name_hint, and rename to "hint".  Pass location
> > to
> > 	lookup_name_fuzzy.
> > 	(lookup_name_fuzzy): Convert return type from const char *
> > 	to name_hint.  Add location_t param.
> > 	* parser.c (cp_parser_diagnose_invalid_type_name): Convert
> > 	"suggestion" from const char * to name_hint, and rename to
> > "hint".
> > 	Pass location to lookup_name_fuzzy.
> 
> > ---
> >  gcc/c-family/c-common.h | 121
> > +++++++++++++++++++++++++++++++++++++++++++++++-
> >  gcc/c/c-decl.c          |  35 +++++++-------
> >  gcc/c/c-parser.c        |  16 ++++---
> >  gcc/cp/name-lookup.c    |  17 +++----
> >  gcc/cp/parser.c         |  12 ++---
> >  5 files changed, 163 insertions(+), 38 deletions(-)
> > 
> > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> > index 138a0a6..83c1a68 100644
> > --- a/gcc/c-family/c-common.h
> > +++ b/gcc/c-family/c-common.h
> > @@ -1009,7 +1009,126 @@ enum lookup_name_fuzzy_kind {
> >    /* Any name.  */
> >    FUZZY_LOOKUP_NAME
> >  };
> > -extern const char *lookup_name_fuzzy (tree, enum
> > lookup_name_fuzzy_kind);
> > +
> > +/* A deferred_diagnostic is a wrapper around optional extra
> > diagnostics
> > +   that we may want to bundle into a name_hint.
> > +
> > +   The emit method is called when no name_hint instances reference
> > +   the deferred_diagnostic.  In the simple case this is when the
> > name_hint
> > +   goes out of scope, but a reference-counting scheme is used to
> > allow
> > +   name_hint instances to be copied.  */
> > +
> > +class deferred_diagnostic
> > +{
> > + public:
> > +  virtual ~deferred_diagnostic () {}
> > +  virtual void emit () = 0;
> > +
> > +  void incref () { m_refcnt++; }
> > +  void decref ()
> > +  {
> > +    if (--m_refcnt == 0)
> > +      {
> > +	if (!m_suppress)
> > +	  emit ();
> > +	delete this;
> > +      }
> > +  }
> > +
> > +  location_t get_location () const { return m_loc; }
> > +
> > +  /* Call this if the corresponding warning was not emitted,
> > +     in which case we should also not emit the
> > deferred_diagnostic.  */
> > +  void suppress ()
> > +  {
> > +    m_suppress = true;
> > +  }
> > +
> > + protected:
> > +  deferred_diagnostic (location_t loc)
> > +  : m_refcnt (0), m_loc (loc), m_suppress (false) {}
> > +
> > + private:
> > +  int m_refcnt;
> > +  location_t m_loc;
> > +  bool m_suppress;
> > +};
> So what stands out here is "delete this" and the need for explicit
> reference counting.  Also doesn't that imply that deferred_diagnostic
> objects must be allocated on the heap?  Is there another way to get
> the
> behavior you want without resorting to something like this?
> 

Thanks for looking at this.

Yes: deferred_diagnostic instances are heap-allocated.  This is because
it's an abstract base class; each concrete subclass is an
implementation detail within the frontends, for isolating the special
-case logic for each different kind of hint, and thus these concrete
subclasses are hidden within the FE code.

My initial implementation of the above had the name_hint class directly
"own" the deferred_diagnostic ptr, with a:
  delete m_deferred;
within name_hint's dtor.

This worked OK, until I encountered places in the C and C++ frontends
where the natural thing (to avoid complicating the control flow) was to
have a default-constructed name_hint as we enter the error-handling,
and then optionally copy over it with a name_hint instance returned
from a call to lookup_name_fuzzy on some of the paths (specifically,
this was in c/c-decl.c's  implicit_decl_warning and in and
 cp/parser.c's cp_parser_diagnose_invalid_type_name).

AIUI, the idiomatic way to do this assignment of an "owned" ptr in
modern C++ would be to use std::unique_ptr (if I understand things
right, this is move-asssignment), the issue being that when we return a
name_hint:

  name_hint hint;
  if (sometimes)
    hint = lookup_name_fuzzy (...);
  // potentially use hint
  // hint's dtor runs, showing any deferred diagnostic it "owns"

we have two name_hint instances: the return value, and the local
"hint", and the return value's dtor runs immediately after the local is
assigned to, but before we use the deferred diagnostic.

I believe that the above with a std::unique_ptr within class name_hint
requires the "move semantics" of C+11 in order to be guaranteed to work
correctly, which is beyond what we currently require (C++98 iirc).

[C++ gurus, please correct me if I've got this wrong]

Hence I coded up something akin to std::shared_ptr "by hand" here (and
yes, it is perhaps overkill).

> Or is your argument that deferred_diagnostic is only used from within
> class name_hint and thus the concerns around heap vs stack, explicit
> counting, etc are all buried inside the name_hint class?  If so, is
> there any reasonable way to restrict the use of deferred_disagnostic
> to
> within the name_hint class?

I guess I could try moving the scope of the name deferred_diagnostic to
be within class name_hint.

Other approaches to simplify the code:

(a) GC-allocate the deferred_diagnostic, and let it be collected next
time the GC collects (as nothing would ever have a root to it).

(b) convert the deferred_diagnostic subclasses to be singletons, and
then return a ptr to the singleton (and clobber them each time we need
a supposedly "new" one).

Alternatively, we could hide the copy-assignment operator for
name_hint, and uglify/complicate the control flow in the two sites that
need it.

Any other ideas?

Do you have any preferences?

> The rest of the changes seem non-controversial, so I think if we can
> sort out the issues with those classes then this will be fine to move
> forward.

Thanks for the review so far.
Dave

Re: [PATCH 1/3] c-family: add name_hint/deferred_diagnostic

[Trevor Saunders]

On Mon, Jul 03, 2017 at 01:32:46PM -0400, David Malcolm wrote:
> On Mon, 2017-07-03 at 10:25 -0600, Jeff Law wrote:
> > On 05/05/2017 11:51 AM, David Malcolm wrote:
> > > In various places we use lookup_name_fuzzy to provide a hint,
> > > and can report messages of the form:
> > >   error: unknown foo named 'bar'
> > > or:
> > >   error: unknown foo named 'bar'; did you mean 'SUGGESTION?
> > > 
> > > This patch provides a way for lookup_name_fuzzy to provide
> > > both the suggestion above, and (optionally) additional hints
> > > that can be printed e.g.
> > > 
> > >   note: did you forget to include <SOME_HEADER.h>?
> > > 
> > > This patch provides the mechanism and ports existing users
> > > of lookup_name_fuzzy to the new return type.
> > > There are no uses of such hints in this patch, but followup
> > > patches provide various front-end specific uses of this.
> > > 
> > > gcc/c-family/ChangeLog:
> > > 	* c-common.h (class deferred_diagnostic): New class.
> > > 	(class name_hint): New class.
> > > 	(lookup_name_fuzzy): Convert return type from const char *
> > > 	to name_hint.  Add location_t param.
> > > 
> > > gcc/c/ChangeLog:
> > > 	* c-decl.c (implicit_decl_warning): Convert "hint" from
> > > 	const char * to name_hint.  Pass location to
> > > 	lookup_name_fuzzy.  Suppress any deferred diagnostic if the
> > > 	warning was not printed.
> > > 	(undeclared_variable): Likewise for "guessed_id".
> > > 	(lookup_name_fuzzy): Convert return type from const char *
> > > 	to name_hint.  Add location_t param.
> > > 	* c-parser.c (c_parser_declaration_or_fndef): Convert "hint"
> > > from
> > > 	const char * to name_hint.  Pass location to lookup_name_fuzzy.
> > > 	(c_parser_parameter_declaration): Pass location to
> > > 	lookup_name_fuzzy.
> > > 
> > > gcc/cp/ChangeLog:
> > > 	* name-lookup.c (suggest_alternatives_for): Convert
> > > "fuzzy_name" from
> > > 	const char * to name_hint, and rename to "hint".  Pass location
> > > to
> > > 	lookup_name_fuzzy.
> > > 	(lookup_name_fuzzy): Convert return type from const char *
> > > 	to name_hint.  Add location_t param.
> > > 	* parser.c (cp_parser_diagnose_invalid_type_name): Convert
> > > 	"suggestion" from const char * to name_hint, and rename to
> > > "hint".
> > > 	Pass location to lookup_name_fuzzy.
> > 
> > > ---
> > >  gcc/c-family/c-common.h | 121
> > > +++++++++++++++++++++++++++++++++++++++++++++++-
> > >  gcc/c/c-decl.c          |  35 +++++++-------
> > >  gcc/c/c-parser.c        |  16 ++++---
> > >  gcc/cp/name-lookup.c    |  17 +++----
> > >  gcc/cp/parser.c         |  12 ++---
> > >  5 files changed, 163 insertions(+), 38 deletions(-)
> > > 
> > > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> > > index 138a0a6..83c1a68 100644
> > > --- a/gcc/c-family/c-common.h
> > > +++ b/gcc/c-family/c-common.h
> > > @@ -1009,7 +1009,126 @@ enum lookup_name_fuzzy_kind {
> > >    /* Any name.  */
> > >    FUZZY_LOOKUP_NAME
> > >  };
> > > -extern const char *lookup_name_fuzzy (tree, enum
> > > lookup_name_fuzzy_kind);
> > > +
> > > +/* A deferred_diagnostic is a wrapper around optional extra
> > > diagnostics
> > > +   that we may want to bundle into a name_hint.
> > > +
> > > +   The emit method is called when no name_hint instances reference
> > > +   the deferred_diagnostic.  In the simple case this is when the
> > > name_hint
> > > +   goes out of scope, but a reference-counting scheme is used to
> > > allow
> > > +   name_hint instances to be copied.  */
> > > +
> > > +class deferred_diagnostic
> > > +{
> > > + public:
> > > +  virtual ~deferred_diagnostic () {}
> > > +  virtual void emit () = 0;
> > > +
> > > +  void incref () { m_refcnt++; }
> > > +  void decref ()
> > > +  {
> > > +    if (--m_refcnt == 0)
> > > +      {
> > > +	if (!m_suppress)
> > > +	  emit ();
> > > +	delete this;
> > > +      }
> > > +  }
> > > +
> > > +  location_t get_location () const { return m_loc; }
> > > +
> > > +  /* Call this if the corresponding warning was not emitted,
> > > +     in which case we should also not emit the
> > > deferred_diagnostic.  */
> > > +  void suppress ()
> > > +  {
> > > +    m_suppress = true;
> > > +  }
> > > +
> > > + protected:
> > > +  deferred_diagnostic (location_t loc)
> > > +  : m_refcnt (0), m_loc (loc), m_suppress (false) {}
> > > +
> > > + private:
> > > +  int m_refcnt;
> > > +  location_t m_loc;
> > > +  bool m_suppress;
> > > +};
> > So what stands out here is "delete this" and the need for explicit
> > reference counting.  Also doesn't that imply that deferred_diagnostic
> > objects must be allocated on the heap?  Is there another way to get
> > the
> > behavior you want without resorting to something like this?
> > 
> 
> Thanks for looking at this.
> 
> Yes: deferred_diagnostic instances are heap-allocated.  This is because
> it's an abstract base class; each concrete subclass is an
> implementation detail within the frontends, for isolating the special
> -case logic for each different kind of hint, and thus these concrete
> subclasses are hidden within the FE code.
> 
> My initial implementation of the above had the name_hint class directly
> "own" the deferred_diagnostic ptr, with a:
>   delete m_deferred;
> within name_hint's dtor.
> 
> This worked OK, until I encountered places in the C and C++ frontends
> where the natural thing (to avoid complicating the control flow) was to
> have a default-constructed name_hint as we enter the error-handling,
> and then optionally copy over it with a name_hint instance returned
> from a call to lookup_name_fuzzy on some of the paths (specifically,
> this was in c/c-decl.c's  implicit_decl_warning and in and
>  cp/parser.c's cp_parser_diagnose_invalid_type_name).
> 
> AIUI, the idiomatic way to do this assignment of an "owned" ptr in
> modern C++ would be to use std::unique_ptr (if I understand things
> right, this is move-asssignment), the issue being that when we return a
> name_hint:

That would be one way, though you can avoid allocating on the heap by
doing something like this.

std::option<name_hint> hint;
if (something)
  foo (&something);

    void
    foo(std::option<name_hint> *hint)
    {
      hint->emplace (name_hint (5));
      }

>   name_hint hint;
>   if (sometimes)
>     hint = lookup_name_fuzzy (...);
>   // potentially use hint
>   // hint's dtor runs, showing any deferred diagnostic it "owns"
> 
> we have two name_hint instances: the return value, and the local
> "hint", and the return value's dtor runs immediately after the local is
> assigned to, but before we use the deferred diagnostic.
> 
> I believe that the above with a std::unique_ptr within class name_hint
> requires the "move semantics" of C+11 in order to be guaranteed to work
> correctly, which is beyond what we currently require (C++98 iirc).

Well, you can use something like std::auto_ptr that makes this "work"
without move assignment by having a copy constructor that modifies the
original object.

I have patches to add something like that that I need to get to posting,
but have been delayed by moving and such things.

> Hence I coded up something akin to std::shared_ptr "by hand" here (and
> yes, it is perhaps overkill).

fwiw when its necessary to refcount things I do prefer "intrusive"
refcounting like this to the way shared_ptr does it.

> > Or is your argument that deferred_diagnostic is only used from within
> > class name_hint and thus the concerns around heap vs stack, explicit
> > counting, etc are all buried inside the name_hint class?  If so, is
> > there any reasonable way to restrict the use of deferred_disagnostic
> > to
> > within the name_hint class?
> 
> I guess I could try moving the scope of the name deferred_diagnostic to
> be within class name_hint.
> 
> Other approaches to simplify the code:
> 
> (a) GC-allocate the deferred_diagnostic, and let it be collected next
> time the GC collects (as nothing would ever have a root to it).

I'd prefer the current approach to this the refcounting code is ugly,
though perhaps we'll need a generic refcounting framework some day.
However I think its easier to reason about ownership with refcounting
than gc, and hopefully make it easier to switch to something like
unique_ptr when I finish those patches.

> (b) convert the deferred_diagnostic subclasses to be singletons, and
> then return a ptr to the singleton (and clobber them each time we need
> a supposedly "new" one).

This also seems to really complicate ownership which is more of a global
problem than the refcounting?

> Alternatively, we could hide the copy-assignment operator for
> name_hint, and uglify/complicate the control flow in the two sites that
> need it.
> 
> Any other ideas?

I think my highest preference would be the std::option thing, though I'm
not sure we have code for that yet.  Then would be the unique_ptr using
auto_ptr approach I'm working on, and then what you have here.  With the
assumption we'll switch it to one of the first two when possible.

thanks

Trev

Re: [PATCH 1/3] c-family: add name_hint/deferred_diagnostic

[Jeff Law]

On 07/03/2017 11:32 AM, David Malcolm wrote:
> On Mon, 2017-07-03 at 10:25 -0600, Jeff Law wrote:
>> On 05/05/2017 11:51 AM, David Malcolm wrote:
>>> In various places we use lookup_name_fuzzy to provide a hint,
>>> and can report messages of the form:
>>>   error: unknown foo named 'bar'
>>> or:
>>>   error: unknown foo named 'bar'; did you mean 'SUGGESTION?
>>>
>>> This patch provides a way for lookup_name_fuzzy to provide
>>> both the suggestion above, and (optionally) additional hints
>>> that can be printed e.g.
>>>
>>>   note: did you forget to include <SOME_HEADER.h>?
>>>
>>> This patch provides the mechanism and ports existing users
>>> of lookup_name_fuzzy to the new return type.
>>> There are no uses of such hints in this patch, but followup
>>> patches provide various front-end specific uses of this.
>>>
>>> gcc/c-family/ChangeLog:
>>> 	* c-common.h (class deferred_diagnostic): New class.
>>> 	(class name_hint): New class.
>>> 	(lookup_name_fuzzy): Convert return type from const char *
>>> 	to name_hint.  Add location_t param.
>>>
>>> gcc/c/ChangeLog:
>>> 	* c-decl.c (implicit_decl_warning): Convert "hint" from
>>> 	const char * to name_hint.  Pass location to
>>> 	lookup_name_fuzzy.  Suppress any deferred diagnostic if the
>>> 	warning was not printed.
>>> 	(undeclared_variable): Likewise for "guessed_id".
>>> 	(lookup_name_fuzzy): Convert return type from const char *
>>> 	to name_hint.  Add location_t param.
>>> 	* c-parser.c (c_parser_declaration_or_fndef): Convert "hint"
>>> from
>>> 	const char * to name_hint.  Pass location to lookup_name_fuzzy.
>>> 	(c_parser_parameter_declaration): Pass location to
>>> 	lookup_name_fuzzy.
>>>
>>> gcc/cp/ChangeLog:
>>> 	* name-lookup.c (suggest_alternatives_for): Convert
>>> "fuzzy_name" from
>>> 	const char * to name_hint, and rename to "hint".  Pass location
>>> to
>>> 	lookup_name_fuzzy.
>>> 	(lookup_name_fuzzy): Convert return type from const char *
>>> 	to name_hint.  Add location_t param.
>>> 	* parser.c (cp_parser_diagnose_invalid_type_name): Convert
>>> 	"suggestion" from const char * to name_hint, and rename to
>>> "hint".
>>> 	Pass location to lookup_name_fuzzy.
>>
>>> ---
>>>  gcc/c-family/c-common.h | 121
>>> +++++++++++++++++++++++++++++++++++++++++++++++-
>>>  gcc/c/c-decl.c          |  35 +++++++-------
>>>  gcc/c/c-parser.c        |  16 ++++---
>>>  gcc/cp/name-lookup.c    |  17 +++----
>>>  gcc/cp/parser.c         |  12 ++---
>>>  5 files changed, 163 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
>>> index 138a0a6..83c1a68 100644
>>> --- a/gcc/c-family/c-common.h
>>> +++ b/gcc/c-family/c-common.h
>>> @@ -1009,7 +1009,126 @@ enum lookup_name_fuzzy_kind {
>>>    /* Any name.  */
>>>    FUZZY_LOOKUP_NAME
>>>  };
>>> -extern const char *lookup_name_fuzzy (tree, enum
>>> lookup_name_fuzzy_kind);
>>> +
>>> +/* A deferred_diagnostic is a wrapper around optional extra
>>> diagnostics
>>> +   that we may want to bundle into a name_hint.
>>> +
>>> +   The emit method is called when no name_hint instances reference
>>> +   the deferred_diagnostic.  In the simple case this is when the
>>> name_hint
>>> +   goes out of scope, but a reference-counting scheme is used to
>>> allow
>>> +   name_hint instances to be copied.  */
>>> +
>>> +class deferred_diagnostic
>>> +{
>>> + public:
>>> +  virtual ~deferred_diagnostic () {}
>>> +  virtual void emit () = 0;
>>> +
>>> +  void incref () { m_refcnt++; }
>>> +  void decref ()
>>> +  {
>>> +    if (--m_refcnt == 0)
>>> +      {
>>> +	if (!m_suppress)
>>> +	  emit ();
>>> +	delete this;
>>> +      }
>>> +  }
>>> +
>>> +  location_t get_location () const { return m_loc; }
>>> +
>>> +  /* Call this if the corresponding warning was not emitted,
>>> +     in which case we should also not emit the
>>> deferred_diagnostic.  */
>>> +  void suppress ()
>>> +  {
>>> +    m_suppress = true;
>>> +  }
>>> +
>>> + protected:
>>> +  deferred_diagnostic (location_t loc)
>>> +  : m_refcnt (0), m_loc (loc), m_suppress (false) {}
>>> +
>>> + private:
>>> +  int m_refcnt;
>>> +  location_t m_loc;
>>> +  bool m_suppress;
>>> +};
>> So what stands out here is "delete this" and the need for explicit
>> reference counting.  Also doesn't that imply that deferred_diagnostic
>> objects must be allocated on the heap?  Is there another way to get
>> the
>> behavior you want without resorting to something like this?
>>
> 
> Thanks for looking at this.
> 
> Yes: deferred_diagnostic instances are heap-allocated.  This is because
> it's an abstract base class; each concrete subclass is an
> implementation detail within the frontends, for isolating the special
> -case logic for each different kind of hint, and thus these concrete
> subclasses are hidden within the FE code.
> 
> My initial implementation of the above had the name_hint class directly
> "own" the deferred_diagnostic ptr, with a:
>   delete m_deferred;
> within name_hint's dtor.
> 
> This worked OK, until I encountered places in the C and C++ frontends
> where the natural thing (to avoid complicating the control flow) was to
> have a default-constructed name_hint as we enter the error-handling,
> and then optionally copy over it with a name_hint instance returned
> from a call to lookup_name_fuzzy on some of the paths (specifically,
> this was in c/c-decl.c's  implicit_decl_warning and in and
>  cp/parser.c's cp_parser_diagnose_invalid_type_name).
> 
> AIUI, the idiomatic way to do this assignment of an "owned" ptr in
> modern C++ would be to use std::unique_ptr (if I understand things
> right, this is move-asssignment), the issue being that when we return a
> name_hint:
> 
>   name_hint hint;
>   if (sometimes)
>     hint = lookup_name_fuzzy (...);
>   // potentially use hint
>   // hint's dtor runs, showing any deferred diagnostic it "owns"
> 
> we have two name_hint instances: the return value, and the local
> "hint", and the return value's dtor runs immediately after the local is
> assigned to, but before we use the deferred diagnostic.
Right.  And raises the general issue of what to do with "owned" external
resources such as allocated memory.

> 
> I believe that the above with a std::unique_ptr within class name_hint
> requires the "move semantics" of C+11 in order to be guaranteed to work
> correctly, which is beyond what we currently require (C++98 iirc).
> 
> [C++ gurus, please correct me if I've got this wrong]
> 
> Hence I coded up something akin to std::shared_ptr "by hand" here (and
> yes, it is perhaps overkill).
> 
>> Or is your argument that deferred_diagnostic is only used from within
>> class name_hint and thus the concerns around heap vs stack, explicit
>> counting, etc are all buried inside the name_hint class?  If so, is
>> there any reasonable way to restrict the use of deferred_disagnostic
>> to
>> within the name_hint class?
> 
> I guess I could try moving the scope of the name deferred_diagnostic to
> be within class name_hint.
> 
> Other approaches to simplify the code:
> 
> (a) GC-allocate the deferred_diagnostic, and let it be collected next
> time the GC collects (as nothing would ever have a root to it).
> 
> (b) convert the deferred_diagnostic subclasses to be singletons, and
> then return a ptr to the singleton (and clobber them each time we need
> a supposedly "new" one).
> 
> Alternatively, we could hide the copy-assignment operator for
> name_hint, and uglify/complicate the control flow in the two sites that
> need it.
> 
> Any other ideas?
I don't have strong opinions in this space -- primarily I noticed the
reference counting and explicit deletion which seemed unusual and would
likely have implications on where/how the class could be safely used.
So some discussion seemed warranted.

Would Trevor's recent unique_ptr bits simplify things here?


Jeff

[PATCH] c-family: add name_hint/deferred_diagnostic (v2)

[David Malcolm]

Here's an updated version of the patch which drops refcounting in favor
of using gnu::unique_ptr.

One wart with this approach is that the handling of suppressed diagnostics
has to happen in both deferred_diagnostic subclasses, rather
than in the name_hint class.  It would be possible to fix this
by introducing another dynamically-allocated object to manage
this concern, but adding another dynamic allocation seemed like
overkill, so I went with the simpler approach (with the slight wart).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu, with
the followup patches you already approved:

[PATCH 2/3] C++: provide macro used-before-defined hint (PR c++/72786).
  https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00124.html

[PATCH 3/3] C: hints for missing stdlib includes for macros and types
  https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00125.html

(plus I have a further followup under construction that extends this
to address PR c/81404)

OK for trunk?


Blurb from v1:

In various places we use lookup_name_fuzzy to provide a hint,
and can report messages of the form:
  error: unknown foo named 'bar'
or:
  error: unknown foo named 'bar'; did you mean 'SUGGESTION?

This patch provides a way for lookup_name_fuzzy to provide
both the suggestion above, and (optionally) additional hints
that can be printed e.g.

  note: did you forget to include <SOME_HEADER.h>?

This patch provides the mechanism and ports existing users
of lookup_name_fuzzy to the new return type.
There are no uses of such hints in this patch, but followup
patches provide various front-end specific uses of this.

gcc/c-family/ChangeLog:
	* c-common.h (class deferred_diagnostic): New class.
	(class name_hint): New class.
	(lookup_name_fuzzy): Convert return type from const char *
	to name_hint.  Add location_t param.

gcc/c/ChangeLog:
	* c-decl.c (implicit_decl_warning): Convert "hint" from
	const char * to name_hint.  Pass location to
	lookup_name_fuzzy.  Suppress any deferred diagnostic if the
	warning was not printed.
	(undeclared_variable): Likewise for "guessed_id".
	(lookup_name_fuzzy): Convert return type from const char *
	to name_hint.  Add location_t param.
	* c-parser.c (c_parser_declaration_or_fndef): Convert "hint" from
	const char * to name_hint.  Pass location to lookup_name_fuzzy.
	(c_parser_parameter_declaration): Pass location to
	lookup_name_fuzzy.

gcc/cp/ChangeLog:
	* name-lookup.c (suggest_alternatives_for): Convert "fuzzy_name" from
	const char * to name_hint, and rename to "hint".  Pass location to
	lookup_name_fuzzy.
	(lookup_name_fuzzy): Convert return type from const char *
	to name_hint.  Add location_t param.
	* parser.c (cp_parser_diagnose_invalid_type_name): Convert
	"suggestion" from const char * to name_hint, and rename to "hint".
	Pass location to lookup_name_fuzzy.
---
 gcc/c-family/c-common.h | 81 ++++++++++++++++++++++++++++++++++++++++++++++++-
 gcc/c/c-decl.c          | 35 +++++++++++----------
 gcc/c/c-parser.c        | 16 +++++-----
 gcc/cp/name-lookup.c    | 14 +++++----
 gcc/cp/parser.c         | 12 ++++----
 5 files changed, 122 insertions(+), 36 deletions(-)

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 7e1877e..50e60bf 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -25,6 +25,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "alias.h"
 #include "tree.h"
 #include "fold-const.h"
+#include "unique-ptr.h"
 
 /* In order for the format checking to accept the C frontend
    diagnostic framework extensions, you must include this file before
@@ -1000,7 +1001,85 @@ enum lookup_name_fuzzy_kind {
   /* Any name.  */
   FUZZY_LOOKUP_NAME
 };
-extern const char *lookup_name_fuzzy (tree, enum lookup_name_fuzzy_kind);
+
+/* A deferred_diagnostic is a wrapper around optional extra diagnostics
+   that we may want to bundle into a name_hint.
+
+   The diagnostic is emitted by the subclass destructor, which should
+   check that is_suppressed_p () is not true.  */
+
+class deferred_diagnostic
+{
+ public:
+  virtual ~deferred_diagnostic () {}
+
+  location_t get_location () const { return m_loc; }
+
+  /* Call this if the corresponding warning was not emitted,
+     in which case we should also not emit the deferred_diagnostic.  */
+  void suppress ()
+  {
+    m_suppress = true;
+  }
+
+  bool is_suppressed_p () const { return m_suppress; }
+
+ protected:
+  deferred_diagnostic (location_t loc)
+  : m_loc (loc), m_suppress (false) {}
+
+ private:
+  location_t m_loc;
+  bool m_suppress;
+};
+
+/* A name_hint is an optional string suggestion, along with an
+   optional deferred_diagnostic.
+   For example:
+
+       error: unknown foo named 'bar'
+
+   if the SUGGESTION is "baz", then one might print:
+
+       error: unknown foo named 'bar'; did you mean 'baz'?
+
+   and the deferred_diagnostic allows for additional (optional)
+   diagnostics e.g.:
+
+       note: did you check behind the couch?
+
+   The deferred_diagnostic is emitted by its destructor, when the
+   name_hint goes out of scope.  */
+
+class name_hint
+{
+public:
+  name_hint () : m_suggestion (NULL), m_deferred () {}
+
+  name_hint (const char *suggestion, deferred_diagnostic *deferred)
+  : m_suggestion (suggestion), m_deferred (deferred)
+  {
+  }
+
+  const char *suggestion () const { return m_suggestion; }
+  operator bool () const { return m_suggestion != NULL; }
+
+  /* Call this on a name_hint if the corresponding warning was not emitted,
+     in which case we should also not emit the deferred_diagnostic.  */
+
+  void suppress ()
+  {
+    if (m_deferred)
+      m_deferred->suppress ();
+  }
+
+private:
+  const char *m_suggestion;
+  gnu::unique_ptr<deferred_diagnostic> m_deferred;
+};
+
+extern name_hint lookup_name_fuzzy (tree, enum lookup_name_fuzzy_kind,
+				    location_t);
 
 extern bool vector_targets_convertible_p (const_tree t1, const_tree t2);
 extern bool vector_types_convertible_p (const_tree t1, const_tree t2, bool emit_lax_note);
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 26b34ab..e8cea90 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -3106,20 +3106,20 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
     return;
 
   bool warned;
-  const char *hint = NULL;
+  name_hint hint;
   if (!olddecl)
-    hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_FUNCTION_NAME);
+    hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_FUNCTION_NAME, loc);
 
   if (flag_isoc99)
     {
       if (hint)
 	{
 	  gcc_rich_location richloc (loc);
-	  richloc.add_fixit_replace (hint);
+	  richloc.add_fixit_replace (hint.suggestion ());
 	  warned = pedwarn_at_rich_loc
 	    (&richloc, OPT_Wimplicit_function_declaration,
 	     "implicit declaration of function %qE; did you mean %qs?",
-	     id, hint);
+	     id, hint.suggestion ());
 	}
       else
 	warned = pedwarn (loc, OPT_Wimplicit_function_declaration,
@@ -3128,11 +3128,11 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
   else if (hint)
     {
       gcc_rich_location richloc (loc);
-      richloc.add_fixit_replace (hint);
+      richloc.add_fixit_replace (hint.suggestion ());
       warned = warning_at_rich_loc
 	(&richloc, OPT_Wimplicit_function_declaration,
 	 G_("implicit declaration of function %qE; did you mean %qs?"),
-	 id, hint);
+	 id, hint.suggestion ());
     }
   else
     warned = warning_at (loc, OPT_Wimplicit_function_declaration,
@@ -3140,6 +3140,9 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
 
   if (olddecl && warned)
     locate_old_decl (olddecl);
+
+  if (!warned)
+    hint.suppress ();
 }
 
 /* This function represents mapping of a function code FCODE
@@ -3458,15 +3461,15 @@ undeclared_variable (location_t loc, tree id)
 
   if (current_function_decl == NULL_TREE)
     {
-      const char *guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME);
+      name_hint guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME, loc);
       if (guessed_id)
 	{
 	  gcc_rich_location richloc (loc);
-	  richloc.add_fixit_replace (guessed_id);
+	  richloc.add_fixit_replace (guessed_id.suggestion ());
 	  error_at_rich_loc (&richloc,
 			     "%qE undeclared here (not in a function);"
 			     " did you mean %qs?",
-			     id, guessed_id);
+			     id, guessed_id.suggestion ());
 	}
       else
 	error_at (loc, "%qE undeclared here (not in a function)", id);
@@ -3476,16 +3479,16 @@ undeclared_variable (location_t loc, tree id)
     {
       if (!objc_diagnose_private_ivar (id))
 	{
-	  const char *guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME);
+	  name_hint guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME, loc);
 	  if (guessed_id)
 	    {
 	      gcc_rich_location richloc (loc);
-	      richloc.add_fixit_replace (guessed_id);
+	      richloc.add_fixit_replace (guessed_id.suggestion ());
 	      error_at_rich_loc
 		(&richloc,
 		 "%qE undeclared (first use in this function);"
 		 " did you mean %qs?",
-		 id, guessed_id);
+		 id, guessed_id.suggestion ());
 	    }
 	  else
 	    error_at (loc, "%qE undeclared (first use in this function)", id);
@@ -3996,8 +3999,8 @@ lookup_name_in_scope (tree name, struct c_scope *scope)
    It also looks for start_typename keywords, to detect "singed" vs "signed"
    typos.  */
 
-const char *
-lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
+name_hint
+lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t)
 {
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
@@ -4087,9 +4090,9 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
 
   tree best = bm.get_best_meaningful_candidate ();
   if (best)
-    return IDENTIFIER_POINTER (best);
+    return name_hint (IDENTIFIER_POINTER (best), NULL);
   else
-    return NULL;
+    return name_hint (NULL, NULL);
 }
 
 \f
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 6b84324..8bf10ad 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -1808,13 +1808,14 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 	}
       else
 	{
-	  const char *hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_TYPENAME);
+	  name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_TYPENAME,
+					      here);
 	  if (hint)
 	    {
-	      richloc.add_fixit_replace (hint);
+	      richloc.add_fixit_replace (hint.suggestion ());
 	      error_at_rich_loc (&richloc,
 				 "unknown type name %qE; did you mean %qs?",
-				 name, hint);
+				 name, hint.suggestion ());
 	    }
 	  else
 	    error_at (here, "unknown type name %qE", name);
@@ -4041,15 +4042,16 @@ c_parser_parameter_declaration (c_parser *parser, tree attrs)
       c_parser_set_source_position_from_token (token);
       if (c_parser_next_tokens_start_typename (parser, cla_prefer_type))
 	{
-	  const char *hint = lookup_name_fuzzy (token->value,
-						FUZZY_LOOKUP_TYPENAME);
+	  name_hint hint = lookup_name_fuzzy (token->value,
+					      FUZZY_LOOKUP_TYPENAME,
+					      token->location);
 	  if (hint)
 	    {
 	      gcc_rich_location richloc (token->location);
-	      richloc.add_fixit_replace (hint);
+	      richloc.add_fixit_replace (hint.suggestion ());
 	      error_at_rich_loc (&richloc,
 				 "unknown type name %qE; did you mean %qs?",
-				 token->value, hint);
+				 token->value, hint.suggestion ());
 	    }
 	  else
 	    error_at (token->location, "unknown type name %qE", token->value);
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index b1b4ebb..6a736da 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5371,13 +5371,15 @@ suggest_alternatives_for (location_t location, tree name,
     }
   else if (!suggest_misspellings)
     ;
-  else if (const char *fuzzy = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME))
+  else if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
+					       location))
     {
       /* Show a spelling correction.  */
       gcc_rich_location richloc (location);
 
-      richloc.add_fixit_replace (fuzzy);
-      inform_at_rich_loc (&richloc, "suggested alternative: %qs", fuzzy);
+      richloc.add_fixit_replace (hint.suggestion ());
+      inform_at_rich_loc (&richloc, "suggested alternative: %qs",
+			  hint.suggestion ());
     }
 }
 
@@ -5628,8 +5630,8 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
    macro names, returning the best match as a const char *, or NULL if
    no reasonable match is found.  */
 
-const char *
-lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
+name_hint
+lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t)
 {
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
@@ -5683,7 +5685,7 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
       bm.consider (IDENTIFIER_POINTER (resword_identifier));
     }
 
-  return bm.get_best_meaningful_candidate ();
+  return name_hint (bm.get_best_meaningful_candidate (), NULL);
 }
 
 /* Subroutine of outer_binding.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 810e2b7..06946a0 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -3287,16 +3287,16 @@ cp_parser_diagnose_invalid_type_name (cp_parser *parser, tree id,
   else if (!parser->scope)
     {
       /* Issue an error message.  */
-      const char *suggestion = NULL;
+      name_hint hint;
       if (TREE_CODE (id) == IDENTIFIER_NODE)
-        suggestion = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME);
-      if (suggestion)
+	hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME, location);
+      if (hint)
 	{
 	  gcc_rich_location richloc (location);
-	  richloc.add_fixit_replace (suggestion);
+	  richloc.add_fixit_replace (hint.suggestion ());
 	  error_at_rich_loc (&richloc,
-			     "%qE does not name a type; did you mean %qs?",
-			     id, suggestion);
+			    "%qE does not name a type; did you mean %qs?",
+			    id, hint.suggestion ());
 	}
       else
 	error_at (location, "%qE does not name a type", id);
-- 
1.8.5.3

Re: [PATCH] c-family: add name_hint/deferred_diagnostic (v2)

[Joseph Myers]

The C front-end parts of this patch are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

[PATCH] c-family: add name_hint/deferred_diagnostic (v3)

[David Malcolm]

Jeff: You previously had concerns about the refcounting used in v1
of this patch; this avoids that in favor of using gnu::unique_ptr.
Joseph already approved the C frontend parts of v2 of this
patch.  

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?

Changed in v3:
- We can't directly include "unique-ptr.h" due to the fix for
  PR bootstrap/82610; see:
    https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01289.html
  The fix is to define INCLUDE_UNIQUE_PTR before including system.h.
  This version of the patch moves the usage of gnu::unique_ptr from
  c-common.h to a new name-hint.h header, to avoid having to define
  INCLUDE_UNIQUE_PTR everywhere that uses c-common.h.
- Updated for *_at_rich_loc renaming

Changed in v2:
- dropped refcounting in favor of using gnu::unique_ptr.  One
  wart with this is that the handling of suppressed diagnostics
  has to happen in every deferred_diagnostic subclass, rather
  than in the name_hint class.  It would be possible to fix this
  by introducing another dynamically-allocated object to manage
  this concern, but adding another dynamic allocation seemed like
  overkill.

Blurb from v1:

In various places we use lookup_name_fuzzy to provide a hint,
and can report messages of the form:
  error: unknown foo named 'bar'
or:
  error: unknown foo named 'bar'; did you mean 'SUGGESTION?

This patch provides a way for lookup_name_fuzzy to provide
both the suggestion above, and (optionally) additional hints
that can be printed e.g.

  note: did you forget to include <SOME_HEADER.h>?

This patch provides the mechanism and ports existing users
of lookup_name_fuzzy to the new return type.
There are no uses of such hints in this patch, but followup
patches provide various front-end specific uses of this.

gcc/c-family/ChangeLog:
	* c-common.h (enum lookup_name_fuzzy_kind): Move to name-hint.h.
	(lookup_name_fuzzy): Likewise.  Convert return type from
	const char * to name_hint.  Add location_t param.
	* name-hint.h: New header.

gcc/c/ChangeLog:
	* c-decl.c: Define INCLUDE_UNIQUE_PTR before including system.h.
	Include "c-family/name-hint.h"
	(implicit_decl_warning): Convert "hint" from
	const char * to name_hint.  Pass location to
	lookup_name_fuzzy.  Suppress any deferred diagnostic if the
	warning was not printed.
	(undeclared_variable): Likewise for "guessed_id".
	(lookup_name_fuzzy): Convert return type from const char *
	to name_hint.  Add location_t param.
	* c-parser.c: Define INCLUDE_UNIQUE_PTR before including system.h.
	Include "c-family/name-hint.h"
	(c_parser_declaration_or_fndef): Convert "hint" from
	const char * to name_hint.  Pass location to lookup_name_fuzzy.
	(c_parser_parameter_declaration): Likewise.

gcc/cp/ChangeLog:
	* name-lookup.c: Define INCLUDE_UNIQUE_PTR before including system.h.
	Include "c-family/name-hint.h"
	(suggest_alternatives_for): Convert "fuzzy_name" from const char *
	to name_hint, and rename to "hint".  Pass location to
	lookup_name_fuzzy.
	(lookup_name_fuzzy): Convert return type from const char *
	to name_hint.  Add location_t param.
	* parser.c: Define INCLUDE_UNIQUE_PTR before including system.h.
	Include "c-family/name-hint.h"
	(cp_parser_diagnose_invalid_type_name): Convert
	"suggestion" from const char * to name_hint, and rename to "hint".
	Pass location to lookup_name_fuzzy.
---
 gcc/c-family/c-common.h  |  12 -----
 gcc/c-family/name-hint.h | 121 +++++++++++++++++++++++++++++++++++++++++++++++
 gcc/c/c-decl.c           |  37 ++++++++-------
 gcc/c/c-parser.c         |  18 ++++---
 gcc/cp/name-lookup.c     |  15 +++---
 gcc/cp/parser.c          |  12 +++--
 6 files changed, 169 insertions(+), 46 deletions(-)
 create mode 100644 gcc/c-family/name-hint.h

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 7e1877e..0f84de9 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -990,18 +990,6 @@ extern tree lookup_label (tree);
 extern tree lookup_name (tree);
 extern bool lvalue_p (const_tree);
 
-enum lookup_name_fuzzy_kind {
-  /* Names of types.  */
-  FUZZY_LOOKUP_TYPENAME,
-
-  /* Names of function decls.  */
-  FUZZY_LOOKUP_FUNCTION_NAME,
-
-  /* Any name.  */
-  FUZZY_LOOKUP_NAME
-};
-extern const char *lookup_name_fuzzy (tree, enum lookup_name_fuzzy_kind);
-
 extern bool vector_targets_convertible_p (const_tree t1, const_tree t2);
 extern bool vector_types_convertible_p (const_tree t1, const_tree t2, bool emit_lax_note);
 extern tree c_build_vec_perm_expr (location_t, tree, tree, tree, bool = true);
diff --git a/gcc/c-family/name-hint.h b/gcc/c-family/name-hint.h
new file mode 100644
index 0000000..9f342c8
--- /dev/null
+++ b/gcc/c-family/name-hint.h
@@ -0,0 +1,121 @@
+/* Support for offering suggestions for handling unrecognized names.
+   Copyright (C) 2016-2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_NAME_HINT_H
+#define GCC_NAME_HINT_H
+
+/* This header uses gnu::unique_ptr, but unique-ptr.h can't be directly
+   included due to issues with macros.  Hence it must be included from
+   system.h by defining INCLUDE_UNIQUE_PTR in any source file using it.  */
+
+#ifndef GNU_UNIQUE_PTR_H
+# error "You must define INCLUDE_UNIQUE_PTR before including system.h to use name-hint.h"
+#endif
+
+enum lookup_name_fuzzy_kind {
+  /* Names of types.  */
+  FUZZY_LOOKUP_TYPENAME,
+
+  /* Names of function decls.  */
+  FUZZY_LOOKUP_FUNCTION_NAME,
+
+  /* Any name.  */
+  FUZZY_LOOKUP_NAME
+};
+
+/* A deferred_diagnostic is a wrapper around optional extra diagnostics
+   that we may want to bundle into a name_hint.
+
+   The diagnostic is emitted by the subclass destructor, which should
+   check that is_suppressed_p () is not true.  */
+
+class deferred_diagnostic
+{
+ public:
+  virtual ~deferred_diagnostic () {}
+
+  location_t get_location () const { return m_loc; }
+
+  /* Call this if the corresponding warning was not emitted,
+     in which case we should also not emit the deferred_diagnostic.  */
+  void suppress ()
+  {
+    m_suppress = true;
+  }
+
+  bool is_suppressed_p () const { return m_suppress; }
+
+ protected:
+  deferred_diagnostic (location_t loc)
+  : m_loc (loc), m_suppress (false) {}
+
+ private:
+  location_t m_loc;
+  bool m_suppress;
+};
+
+/* A name_hint is an optional string suggestion, along with an
+   optional deferred_diagnostic.
+   For example:
+
+       error: unknown foo named 'bar'
+
+   if the SUGGESTION is "baz", then one might print:
+
+       error: unknown foo named 'bar'; did you mean 'baz'?
+
+   and the deferred_diagnostic allows for additional (optional)
+   diagnostics e.g.:
+
+       note: did you check behind the couch?
+
+   The deferred_diagnostic is emitted by its destructor, when the
+   name_hint goes out of scope.  */
+
+class name_hint
+{
+public:
+  name_hint () : m_suggestion (NULL), m_deferred () {}
+
+  name_hint (const char *suggestion, deferred_diagnostic *deferred)
+  : m_suggestion (suggestion), m_deferred (deferred)
+  {
+  }
+
+  const char *suggestion () const { return m_suggestion; }
+  operator bool () const { return m_suggestion != NULL; }
+
+  /* Call this on a name_hint if the corresponding warning was not emitted,
+     in which case we should also not emit the deferred_diagnostic.  */
+
+  void suppress ()
+  {
+    if (m_deferred)
+      m_deferred->suppress ();
+  }
+
+private:
+  const char *m_suggestion;
+  gnu::unique_ptr<deferred_diagnostic> m_deferred;
+};
+
+extern name_hint lookup_name_fuzzy (tree, enum lookup_name_fuzzy_kind,
+				    location_t);
+
+#endif /* ! GCC_NAME_HINT_H */
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index d95a2b6..7726476 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -25,6 +25,7 @@ along with GCC; see the file COPYING3.  If not see
    line numbers.  For example, the CONST_DECLs for enum values.  */
 
 #include "config.h"
+#define INCLUDE_UNIQUE_PTR
 #include "system.h"
 #include "coretypes.h"
 #include "target.h"
@@ -54,6 +55,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "spellcheck-tree.h"
 #include "gcc-rich-location.h"
 #include "asan.h"
+#include "c-family/name-hint.h"
 
 /* In grokdeclarator, distinguish syntactic contexts of declarators.  */
 enum decl_context
@@ -3109,20 +3111,20 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
     return;
 
   bool warned;
-  const char *hint = NULL;
+  name_hint hint;
   if (!olddecl)
-    hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_FUNCTION_NAME);
+    hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_FUNCTION_NAME, loc);
 
   if (flag_isoc99)
     {
       if (hint)
 	{
 	  gcc_rich_location richloc (loc);
-	  richloc.add_fixit_replace (hint);
+	  richloc.add_fixit_replace (hint.suggestion ());
 	  warned = pedwarn (&richloc, OPT_Wimplicit_function_declaration,
 			    "implicit declaration of function %qE;"
 			    " did you mean %qs?",
-			    id, hint);
+			    id, hint.suggestion ());
 	}
       else
 	warned = pedwarn (loc, OPT_Wimplicit_function_declaration,
@@ -3131,11 +3133,11 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
   else if (hint)
     {
       gcc_rich_location richloc (loc);
-      richloc.add_fixit_replace (hint);
+      richloc.add_fixit_replace (hint.suggestion ());
       warned = warning_at
 	(&richloc, OPT_Wimplicit_function_declaration,
 	 G_("implicit declaration of function %qE; did you mean %qs?"),
-	 id, hint);
+	 id, hint.suggestion ());
     }
   else
     warned = warning_at (loc, OPT_Wimplicit_function_declaration,
@@ -3143,6 +3145,9 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
 
   if (olddecl && warned)
     locate_old_decl (olddecl);
+
+  if (!warned)
+    hint.suppress ();
 }
 
 /* This function represents mapping of a function code FCODE
@@ -3466,15 +3471,15 @@ undeclared_variable (location_t loc, tree id)
 
   if (current_function_decl == NULL_TREE)
     {
-      const char *guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME);
+      name_hint guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME, loc);
       if (guessed_id)
 	{
 	  gcc_rich_location richloc (loc);
-	  richloc.add_fixit_replace (guessed_id);
+	  richloc.add_fixit_replace (guessed_id.suggestion ());
 	  error_at (&richloc,
 		    "%qE undeclared here (not in a function);"
 		    " did you mean %qs?",
-		    id, guessed_id);
+		    id, guessed_id.suggestion ());
 	}
       else
 	error_at (loc, "%qE undeclared here (not in a function)", id);
@@ -3484,15 +3489,15 @@ undeclared_variable (location_t loc, tree id)
     {
       if (!objc_diagnose_private_ivar (id))
 	{
-	  const char *guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME);
+	  name_hint guessed_id = lookup_name_fuzzy (id, FUZZY_LOOKUP_NAME, loc);
 	  if (guessed_id)
 	    {
 	      gcc_rich_location richloc (loc);
-	      richloc.add_fixit_replace (guessed_id);
+	      richloc.add_fixit_replace (guessed_id.suggestion ());
 	      error_at (&richloc,
 			"%qE undeclared (first use in this function);"
 			" did you mean %qs?",
-			id, guessed_id);
+			id, guessed_id.suggestion ());
 	    }
 	  else
 	    error_at (loc, "%qE undeclared (first use in this function)", id);
@@ -4003,8 +4008,8 @@ lookup_name_in_scope (tree name, struct c_scope *scope)
    It also looks for start_typename keywords, to detect "singed" vs "signed"
    typos.  */
 
-const char *
-lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
+name_hint
+lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t)
 {
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
@@ -4094,9 +4099,9 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
 
   tree best = bm.get_best_meaningful_candidate ();
   if (best)
-    return IDENTIFIER_POINTER (best);
+    return name_hint (IDENTIFIER_POINTER (best), NULL);
   else
-    return NULL;
+    return name_hint (NULL, NULL);
 }
 
 \f
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 7bca5f1..dae8297 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -36,6 +36,7 @@ along with GCC; see the file COPYING3.  If not see
    location rather than implicitly using input_location.  */
 
 #include "config.h"
+#define INCLUDE_UNIQUE_PTR
 #include "system.h"
 #include "coretypes.h"
 #include "target.h"
@@ -65,6 +66,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "read-rtl-function.h"
 #include "run-rtl-passes.h"
 #include "intl.h"
+#include "c-family/name-hint.h"
 
 /* We need to walk over decls with incomplete struct/union/enum types
    after parsing the whole translation unit.
@@ -1808,13 +1810,14 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 	}
       else
 	{
-	  const char *hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_TYPENAME);
+	  name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_TYPENAME,
+					      here);
 	  if (hint)
 	    {
-	      richloc.add_fixit_replace (hint);
+	      richloc.add_fixit_replace (hint.suggestion ());
 	      error_at (&richloc,
 			"unknown type name %qE; did you mean %qs?",
-			name, hint);
+			name, hint.suggestion ());
 	    }
 	  else
 	    error_at (here, "unknown type name %qE", name);
@@ -4066,15 +4069,16 @@ c_parser_parameter_declaration (c_parser *parser, tree attrs)
       c_parser_set_source_position_from_token (token);
       if (c_parser_next_tokens_start_typename (parser, cla_prefer_type))
 	{
-	  const char *hint = lookup_name_fuzzy (token->value,
-						FUZZY_LOOKUP_TYPENAME);
+	  name_hint hint = lookup_name_fuzzy (token->value,
+					      FUZZY_LOOKUP_TYPENAME,
+					      token->location);
 	  if (hint)
 	    {
 	      gcc_rich_location richloc (token->location);
-	      richloc.add_fixit_replace (hint);
+	      richloc.add_fixit_replace (hint.suggestion ());
 	      error_at (&richloc,
 			"unknown type name %qE; did you mean %qs?",
-			token->value, hint);
+			token->value, hint.suggestion ());
 	    }
 	  else
 	    error_at (token->location, "unknown type name %qE", token->value);
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index b4976d8..8c7fb97 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_UNIQUE_PTR
 #include "system.h"
 #include "coretypes.h"
 #include "cp-tree.h"
@@ -32,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gcc-rich-location.h"
 #include "spellcheck-tree.h"
 #include "parser.h"
+#include "c-family/name-hint.h"
 
 static cxx_binding *cxx_binding_make (tree value, tree type);
 static cp_binding_level *innermost_nonclass_level (void);
@@ -5371,13 +5373,14 @@ suggest_alternatives_for (location_t location, tree name,
     }
   else if (!suggest_misspellings)
     ;
-  else if (const char *fuzzy = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME))
+  else if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
+					       location))
     {
       /* Show a spelling correction.  */
       gcc_rich_location richloc (location);
 
-      richloc.add_fixit_replace (fuzzy);
-      inform (&richloc, "suggested alternative: %qs", fuzzy);
+      richloc.add_fixit_replace (hint.suggestion ());
+      inform (&richloc, "suggested alternative: %qs", hint.suggestion ());
     }
 }
 
@@ -5628,8 +5631,8 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
    macro names, returning the best match as a const char *, or NULL if
    no reasonable match is found.  */
 
-const char *
-lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
+name_hint
+lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t)
 {
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
@@ -5683,7 +5686,7 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
       bm.consider (IDENTIFIER_POINTER (resword_identifier));
     }
 
-  return bm.get_best_meaningful_candidate ();
+  return name_hint (bm.get_best_meaningful_candidate (), NULL);
 }
 
 /* Subroutine of outer_binding.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 77b9637..8775eeb 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define INCLUDE_UNIQUE_PTR
 #include "system.h"
 #include "coretypes.h"
 #include "cp-tree.h"
@@ -43,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "cp-cilkplus.h"
 #include "gcc-rich-location.h"
 #include "tree-iterator.h"
+#include "c-family/name-hint.h"
 
 \f
 /* The lexer.  */
@@ -3287,16 +3289,16 @@ cp_parser_diagnose_invalid_type_name (cp_parser *parser, tree id,
   else if (!parser->scope)
     {
       /* Issue an error message.  */
-      const char *suggestion = NULL;
+      name_hint hint;
       if (TREE_CODE (id) == IDENTIFIER_NODE)
-        suggestion = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME);
-      if (suggestion)
+	hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME, location);
+      if (hint)
 	{
 	  gcc_rich_location richloc (location);
-	  richloc.add_fixit_replace (suggestion);
+	  richloc.add_fixit_replace (hint.suggestion ());
 	  error_at (&richloc,
 		    "%qE does not name a type; did you mean %qs?",
-		    id, suggestion);
+		    id, hint.suggestion ());
 	}
       else
 	error_at (location, "%qE does not name a type", id);
-- 
1.8.5.3

Re: [PATCH] c-family: add name_hint/deferred_diagnostic (v3)

[Jeff Law]

On 11/02/2017 06:21 PM, David Malcolm wrote:
> Jeff: You previously had concerns about the refcounting used in v1
> of this patch; this avoids that in favor of using gnu::unique_ptr.
> Joseph already approved the C frontend parts of v2 of this
> patch.  
I had to go back and find my original message to remember what I was
concerned about.  It was the "delete this" that caught my eye in
conjunction with reference counting which brought up issues of ensuring
the object was always heap allocated and such.

Otherwise the patch was reasonable.  Given you've changed the concerning
code to use the blessed gnu::unique_ptr those concerns should be fully
addressed now.


> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> OK for trunk?
> 
> Changed in v3:
> - We can't directly include "unique-ptr.h" due to the fix for
>   PR bootstrap/82610; see:
>     https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01289.html
>   The fix is to define INCLUDE_UNIQUE_PTR before including system.h.
>   This version of the patch moves the usage of gnu::unique_ptr from
>   c-common.h to a new name-hint.h header, to avoid having to define
>   INCLUDE_UNIQUE_PTR everywhere that uses c-common.h.
> - Updated for *_at_rich_loc renaming
> 
> Changed in v2:
> - dropped refcounting in favor of using gnu::unique_ptr.  One
>   wart with this is that the handling of suppressed diagnostics
>   has to happen in every deferred_diagnostic subclass, rather
>   than in the name_hint class.  It would be possible to fix this
>   by introducing another dynamically-allocated object to manage
>   this concern, but adding another dynamic allocation seemed like
>   overkill.
> 
> Blurb from v1:
> 
> In various places we use lookup_name_fuzzy to provide a hint,
> and can report messages of the form:
>   error: unknown foo named 'bar'
> or:
>   error: unknown foo named 'bar'; did you mean 'SUGGESTION?
> 
> This patch provides a way for lookup_name_fuzzy to provide
> both the suggestion above, and (optionally) additional hints
> that can be printed e.g.
> 
>   note: did you forget to include <SOME_HEADER.h>?
> 
> This patch provides the mechanism and ports existing users
> of lookup_name_fuzzy to the new return type.
> There are no uses of such hints in this patch, but followup
> patches provide various front-end specific uses of this.
> 
> gcc/c-family/ChangeLog:
> 	* c-common.h (enum lookup_name_fuzzy_kind): Move to name-hint.h.
> 	(lookup_name_fuzzy): Likewise.  Convert return type from
> 	const char * to name_hint.  Add location_t param.
> 	* name-hint.h: New header.
> 
> gcc/c/ChangeLog:
> 	* c-decl.c: Define INCLUDE_UNIQUE_PTR before including system.h.
> 	Include "c-family/name-hint.h"
> 	(implicit_decl_warning): Convert "hint" from
> 	const char * to name_hint.  Pass location to
> 	lookup_name_fuzzy.  Suppress any deferred diagnostic if the
> 	warning was not printed.
> 	(undeclared_variable): Likewise for "guessed_id".
> 	(lookup_name_fuzzy): Convert return type from const char *
> 	to name_hint.  Add location_t param.
> 	* c-parser.c: Define INCLUDE_UNIQUE_PTR before including system.h.
> 	Include "c-family/name-hint.h"
> 	(c_parser_declaration_or_fndef): Convert "hint" from
> 	const char * to name_hint.  Pass location to lookup_name_fuzzy.
> 	(c_parser_parameter_declaration): Likewise.
> 
> gcc/cp/ChangeLog:
> 	* name-lookup.c: Define INCLUDE_UNIQUE_PTR before including system.h.
> 	Include "c-family/name-hint.h"
> 	(suggest_alternatives_for): Convert "fuzzy_name" from const char *
> 	to name_hint, and rename to "hint".  Pass location to
> 	lookup_name_fuzzy.
> 	(lookup_name_fuzzy): Convert return type from const char *
> 	to name_hint.  Add location_t param.
> 	* parser.c: Define INCLUDE_UNIQUE_PTR before including system.h.
> 	Include "c-family/name-hint.h"
> 	(cp_parser_diagnose_invalid_type_name): Convert
> 	"suggestion" from const char * to name_hint, and rename to "hint".
> 	Pass location to lookup_name_fuzzy.
OK.

jeff