diff mbox series

c-family: add name_hint/deferred_diagnostic (v2)

Message ID 1508187941-32824-1-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series c-family: add name_hint/deferred_diagnostic (v2) | expand

Commit Message

David Malcolm Oct. 16, 2017, 9:05 p.m. UTC
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(-)

Comments

Joseph Myers Oct. 17, 2017, 8 p.m. UTC | #1
The C front-end parts of this patch are OK.
diff mbox series

Patch

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);
 }
 
 
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);