diff mbox

[committed] rich_location: add convenience overloads for adding fix-it hints

Message ID 1472567287-11927-1-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Aug. 30, 2016, 2:28 p.m. UTC
Adding a fix-it hint to a diagnostic usually follows one of these
patterns:
(a) an insertion fix-its, with the insertion at the primary caret location
(b) a removals/replacements, affecting the range of the primary location

(other cases are possible, e.g. multiple fix-its, and affecting other
locations, but these are the common ones)

Given these common cases, this patch adds overloads of the rich_location
methods for adding fix-it hints, so that the location information can
be omitted if it matches that of the primary location within the
rich_location.

Similarly when adding "remove" and "replace" fix-it hints to a diagnostic,
it's tedious to have to extract the source_range from a location_t
(aka source_location).  To make this more convenient, this patch
adds overload of the rich_location::add_fixit_remove/replace methods,
accepting a source_location directly.

The patch updates the various in-tree users of fix-it hints to use
the new simpler API where appropriate.  I didn't touch the case where
there are multiple fix-its in one rich_location, as it seems better to
be more explicit about locations for this case (adding a pair of parens
in warn_logical_not_parentheses).

The above makes the gcc_rich_location::add_fixit_misspelled_id overload
taking a const char * rather redundant, so I eliminated it.

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

Committed to trunk as r239861.

gcc/c/ChangeLog:
	* c-decl.c (implicit_decl_warning): Use add_fixit_replace
	rather than add_fixit_misspelled_id.
	(undeclared_variable): Likewise.
	* c-parser.c (c_parser_declaration_or_fndef): Likewise.  Remove
	now-redundant "here" params from add_fixit_insert method calls.
	(c_parser_parameter_declaration): Likewise.
	* c-typeck.c (build_component_ref): Remove now-redundant range
	param from add_fixit_replace method calls.

gcc/cp/ChangeLog:
	* name-lookup.c (suggest_alternatives_for): Use add_fixit_replace
	rather than add_fixit_misspelled_id.
	* parser.c (cp_parser_diagnose_invalid_type_name): Likewise.

gcc/ChangeLog:
	* diagnostic-show-locus.c (test_one_liner_fixit_insert): Remove
	redundant location param.
	(test_one_liner_fixit_remove): Likewise.
	(test_one_liner_fixit_replace): Likewise.
	(test_one_liner_fixit_replace_equal_secondary_range): Likewise.
	* gcc-rich-location.c
	(gcc_rich_location::add_fixit_misspelled_id): Eliminate call to
	get_range_from_loc.  Drop overload taking a const char *.
	* gcc-rich-location.h
	(gcc_rich_location::add_fixit_misspelled_id): Drop overload taking
	a const char *.

libcpp/ChangeLog:
	* include/line-map.h (rich_location::add_fixit_insert): Add
	comments.  Add overload omitting the source_location param.
	(rich_location::add_fixit_remove): Add comments.  Add overloads
	omitting the range, and accepting a source_location.
	(rich_location::add_fixit_replace): Likewise.
	* line-map.c (rich_location::add_fixit_insert): Add comments.  Add
	overload omitting the source_location param.
	(rich_location::add_fixit_remove): Add comments.  Add overloads
	omitting the range, and accepting a source_location.
	(rich_location::add_fixit_replace): Likewise.
---
 gcc/c/c-decl.c              |  8 +++----
 gcc/c/c-parser.c            | 10 ++++-----
 gcc/c/c-typeck.c            |  2 +-
 gcc/cp/name-lookup.c        |  2 +-
 gcc/cp/parser.c             |  2 +-
 gcc/diagnostic-show-locus.c | 17 ++++----------
 gcc/gcc-rich-location.c     | 17 +-------------
 gcc/gcc-rich-location.h     |  2 --
 libcpp/include/line-map.h   | 34 ++++++++++++++++++++++++++++
 libcpp/line-map.c           | 54 +++++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 105 insertions(+), 43 deletions(-)
diff mbox

Patch

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 0c52a64..8f49c35 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -3096,7 +3096,7 @@  implicit_decl_warning (location_t loc, tree id, tree olddecl)
 	if (hint)
 	  {
 	    gcc_rich_location richloc (loc);
-	    richloc.add_fixit_misspelled_id (loc, hint);
+	    richloc.add_fixit_replace (hint);
 	    warned = pedwarn_at_rich_loc
 	      (&richloc, OPT_Wimplicit_function_declaration,
 	       "implicit declaration of function %qE; did you mean %qs?",
@@ -3109,7 +3109,7 @@  implicit_decl_warning (location_t loc, tree id, tree olddecl)
 	if (hint)
 	  {
 	    gcc_rich_location richloc (loc);
-	    richloc.add_fixit_misspelled_id (loc, hint);
+	    richloc.add_fixit_replace (hint);
 	    warned = warning_at_rich_loc
 	      (&richloc, OPT_Wimplicit_function_declaration,
 	       G_("implicit declaration of function %qE;did you mean %qs?"),
@@ -3437,7 +3437,7 @@  undeclared_variable (location_t loc, tree id)
       if (guessed_id)
 	{
 	  gcc_rich_location richloc (loc);
-	  richloc.add_fixit_misspelled_id (loc, guessed_id);
+	  richloc.add_fixit_replace (guessed_id);
 	  error_at_rich_loc (&richloc,
 			     "%qE undeclared here (not in a function);"
 			     " did you mean %qs?",
@@ -3455,7 +3455,7 @@  undeclared_variable (location_t loc, tree id)
 	  if (guessed_id)
 	    {
 	      gcc_rich_location richloc (loc);
-	      richloc.add_fixit_misspelled_id (loc, guessed_id);
+	      richloc.add_fixit_replace (guessed_id);
 	      error_at_rich_loc
 		(&richloc,
 		 "%qE undeclared (first use in this function);"
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index fe0c95f..c245e70 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -1685,7 +1685,7 @@  c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
       if (tag_exists_p (RECORD_TYPE, name))
 	{
 	  /* This is not C++ with its implicit typedef.  */
-	  richloc.add_fixit_insert (here, "struct");
+	  richloc.add_fixit_insert ("struct");
 	  error_at_rich_loc (&richloc,
 			     "unknown type name %qE;"
 			     " use %<struct%> keyword to refer to the type",
@@ -1693,7 +1693,7 @@  c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 	}
       else if (tag_exists_p (UNION_TYPE, name))
 	{
-	  richloc.add_fixit_insert (here, "union");
+	  richloc.add_fixit_insert ("union");
 	  error_at_rich_loc (&richloc,
 			     "unknown type name %qE;"
 			     " use %<union%> keyword to refer to the type",
@@ -1701,7 +1701,7 @@  c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 	}
       else if (tag_exists_p (ENUMERAL_TYPE, name))
 	{
-	  richloc.add_fixit_insert (here, "enum");
+	  richloc.add_fixit_insert ("enum");
 	  error_at_rich_loc (&richloc,
 			     "unknown type name %qE;"
 			     " use %<enum%> keyword to refer to the type",
@@ -1712,7 +1712,7 @@  c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 	  const char *hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_TYPENAME);
 	  if (hint)
 	    {
-	      richloc.add_fixit_misspelled_id (here, hint);
+	      richloc.add_fixit_replace (hint);
 	      error_at_rich_loc (&richloc,
 				 "unknown type name %qE; did you mean %qs?",
 				 name, hint);
@@ -3864,7 +3864,7 @@  c_parser_parameter_declaration (c_parser *parser, tree attrs)
 	  if (hint)
 	    {
 	      gcc_rich_location richloc (token->location);
-	      richloc.add_fixit_misspelled_id (token->location, hint);
+	      richloc.add_fixit_replace (hint);
 	      error_at_rich_loc (&richloc,
 				 "unknown type name %qE; did you mean %qs?",
 				 token->value, hint);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 2f8d611..5194027 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -2474,7 +2474,7 @@  build_component_ref (location_t loc, tree datum, tree component,
 	 where the user has confused "." vs "->".  */
       rich_location richloc (line_table, loc);
       /* "loc" should be the "." token.  */
-      richloc.add_fixit_replace (source_range::from_location (loc), "->");
+      richloc.add_fixit_replace ("->");
       error_at_rich_loc (&richloc,
 			 "%qE is a pointer; did you mean to use %<->%>?",
 			 datum);
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index f6ba4d2..022ab6a 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -4446,7 +4446,7 @@  suggest_alternatives_for (location_t location, tree name)
       if (fuzzy_name)
 	{
 	  gcc_rich_location richloc (location);
-	  richloc.add_fixit_misspelled_id (location, fuzzy_name);
+	  richloc.add_fixit_replace (fuzzy_name);
 	  inform_at_rich_loc (&richloc, "suggested alternative: %qs",
 			      fuzzy_name);
 	}
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d54cf8a..48dbca1 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -3171,7 +3171,7 @@  cp_parser_diagnose_invalid_type_name (cp_parser *parser, tree id,
       if (suggestion)
 	{
 	  gcc_rich_location richloc (location);
-	  richloc.add_fixit_misspelled_id (location, suggestion);
+	  richloc.add_fixit_replace (suggestion);
 	  error_at_rich_loc (&richloc,
 			     "%qE does not name a type; did you mean %qs?",
 			     id, suggestion);
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index ba52f24..60f6820 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -1487,7 +1487,7 @@  test_one_liner_fixit_insert ()
   test_diagnostic_context dc;
   location_t caret = linemap_position_for_column (line_table, 7);
   rich_location richloc (line_table, caret);
-  richloc.add_fixit_insert (caret, "&");
+  richloc.add_fixit_insert ("&");
   diagnostic_show_locus (&dc, &richloc, DK_ERROR);
   ASSERT_STREQ ("\n"
 		" foo = bar.field;\n"
@@ -1506,10 +1506,7 @@  test_one_liner_fixit_remove ()
   location_t finish = linemap_position_for_column (line_table, 15);
   location_t dot = make_location (start, start, finish);
   rich_location richloc (line_table, dot);
-  source_range range;
-  range.m_start = start;
-  range.m_finish = finish;
-  richloc.add_fixit_remove (range);
+  richloc.add_fixit_remove ();
   diagnostic_show_locus (&dc, &richloc, DK_ERROR);
   ASSERT_STREQ ("\n"
 		" foo = bar.field;\n"
@@ -1528,10 +1525,7 @@  test_one_liner_fixit_replace ()
   location_t finish = linemap_position_for_column (line_table, 15);
   location_t field = make_location (start, start, finish);
   rich_location richloc (line_table, field);
-  source_range range;
-  range.m_start = start;
-  range.m_finish = finish;
-  richloc.add_fixit_replace (range, "m_field");
+  richloc.add_fixit_replace ("m_field");
   diagnostic_show_locus (&dc, &richloc, DK_ERROR);
   ASSERT_STREQ ("\n"
 		" foo = bar.field;\n"
@@ -1580,10 +1574,7 @@  test_one_liner_fixit_replace_equal_secondary_range ()
   rich_location richloc (line_table, equals);
   location_t field = make_location (start, start, finish);
   richloc.add_range (field, false);
-  source_range range;
-  range.m_start = start;
-  range.m_finish = finish;
-  richloc.add_fixit_replace (range, "m_field");
+  richloc.add_fixit_replace (field, "m_field");
   diagnostic_show_locus (&dc, &richloc, DK_ERROR);
   /* The replacement range is indicated in the annotation line,
      so it shouldn't be indicated via an additional underline.  */
diff --git a/gcc/gcc-rich-location.c b/gcc/gcc-rich-location.c
index 09475ff..b8787b4 100644
--- a/gcc/gcc-rich-location.c
+++ b/gcc/gcc-rich-location.c
@@ -70,20 +70,5 @@  gcc_rich_location::add_fixit_misspelled_id (location_t misspelled_token_loc,
 {
   gcc_assert (TREE_CODE (hint_id) == IDENTIFIER_NODE);
 
-  source_range misspelled_token_range
-    = get_range_from_loc (line_table, misspelled_token_loc);
-  add_fixit_replace (misspelled_token_range, IDENTIFIER_POINTER (hint_id));
-}
-
-/* As above, but with a const char * for the suggested replacement.  */
-
-void
-gcc_rich_location::add_fixit_misspelled_id (location_t misspelled_token_loc,
-					    const char *hint)
-{
-  gcc_assert (hint);
-
-  source_range misspelled_token_range
-    = get_range_from_loc (line_table, misspelled_token_loc);
-  add_fixit_replace (misspelled_token_range, hint);
+  add_fixit_replace (misspelled_token_loc, IDENTIFIER_POINTER (hint_id));
 }
diff --git a/gcc/gcc-rich-location.h b/gcc/gcc-rich-location.h
index cc5987f..0599904 100644
--- a/gcc/gcc-rich-location.h
+++ b/gcc/gcc-rich-location.h
@@ -40,8 +40,6 @@  class gcc_rich_location : public rich_location
 
   void add_fixit_misspelled_id (location_t misspelled_token_loc,
 				tree hint_id);
-  void add_fixit_misspelled_id (location_t misspelled_token_loc,
-				const char *hint);
 };
 
 #endif /* GCC_RICH_LOCATION_H */
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index d9c31de..122e474 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -1401,13 +1401,47 @@  class rich_location
   override_column (int column);
 
   /* Fix-it hints.  */
+
+  /* Methods for adding insertion fix-it hints.  */
+
+  /* Suggest inserting NEW_CONTENT at the primary range's caret.  */
+  void
+  add_fixit_insert (const char *new_content);
+
+  /* Suggest inserting NEW_CONTENT at WHERE.  */
   void
   add_fixit_insert (source_location where,
 		    const char *new_content);
 
+  /* Methods for adding removal fix-it hints.  */
+
+  /* Suggest removing the content covered by range 0.  */
+  void
+  add_fixit_remove ();
+
+  /* Suggest removing the content covered between the start and finish
+     of WHERE.  */
+  void
+  add_fixit_remove (source_location where);
+
+  /* Suggest removing the content covered by SRC_RANGE.  */
   void
   add_fixit_remove (source_range src_range);
 
+  /* Methods for adding "replace" fix-it hints.  */
+
+  /* Suggest replacing the content covered by range 0 with NEW_CONTENT.  */
+  void
+  add_fixit_replace (const char *new_content);
+
+  /* Suggest replacing the content between the start and finish of
+     WHERE with NEW_CONTENT.  */
+  void
+  add_fixit_replace (source_location where,
+		     const char *new_content);
+
+  /* Suggest replacing the content covered by SRC_RANGE with
+     NEW_CONTENT.  */
   void
   add_fixit_replace (source_range src_range,
 		     const char *new_content);
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index f5b1586..3189326 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -2092,6 +2092,17 @@  rich_location::set_range (line_maps * /*set*/, unsigned int idx,
     m_have_expanded_location = false;
 }
 
+/* Methods for adding insertion fix-it hints.  */
+
+/* Add a fixit-hint, suggesting insertion of NEW_CONTENT
+   at the primary range's caret location.  */
+
+void
+rich_location::add_fixit_insert (const char *new_content)
+{
+  add_fixit_insert (get_loc (), new_content);
+}
+
 /* Add a fixit-hint, suggesting insertion of NEW_CONTENT
    at WHERE.  */
 
@@ -2109,6 +2120,27 @@  rich_location::add_fixit_insert (source_location where,
     = new fixit_insert (where, new_content);
 }
 
+/* Methods for adding removal fix-it hints.  */
+
+/* Add a fixit-hint, suggesting removal of the content covered
+   by range 0.  */
+
+void
+rich_location::add_fixit_remove ()
+{
+  add_fixit_remove (get_loc ());
+}
+
+/* Add a fixit-hint, suggesting removal of the content between
+   the start and finish of WHERE.  */
+
+void
+rich_location::add_fixit_remove (source_location where)
+{
+  source_range range = get_range_from_loc (m_line_table, where);
+  add_fixit_remove (range);
+}
+
 /* Add a fixit-hint, suggesting removal of the content at
    SRC_RANGE.  */
 
@@ -2156,6 +2188,28 @@  column_before_p (line_maps *set, source_location a, source_location b)
   return column_b == column_a + 1;
 }
 
+/* Add a fixit-hint, suggesting replacement of the content covered
+   by range 0 with NEW_CONTENT.  */
+
+void
+rich_location::add_fixit_replace (const char *new_content)
+{
+  add_fixit_replace (get_loc (), new_content);
+}
+
+/* Methods for adding "replace" fix-it hints.  */
+
+/* Add a fixit-hint, suggesting replacement of the content between
+   the start and finish of WHERE with NEW_CONTENT.  */
+
+void
+rich_location::add_fixit_replace (source_location where,
+				  const char *new_content)
+{
+  source_range range = get_range_from_loc (m_line_table, where);
+  add_fixit_replace (range, new_content);
+}
+
 /* Add a fixit-hint, suggesting replacement of the content at
    SRC_RANGE with NEW_CONTENT.  */