From 9ef3c9b5d84e0d6678c7fae1d0ca75d195803163 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 18 Sep 2015 13:18:06 -0400
Subject: [PATCH 29/42] PR/62314: add ability to add fixit-hints

This patch adds the ability to add "fix-it hints" to a rich_location,
which will be displayed when the corresponding diagnostic is printed.

It does not actually add any fix-it hints (that comes in a later
patch), but it adds test coverage of the machinery and printing,
by using the existing diagnostic_plugin_test_show_locus to inject
some meaningless fixit hints, and to verify the output.

gcc/ChangeLog:
	PR/62314
	* diagnostic-show-locus.c (colorizer::set_fixit_hint): New.
	(layout::move_to_column): New method.
	(layout::print_line): Print any fixit hints affecting the line.
	For now, add nasty linker kludge for the sake of
	diagnostic_plugin_test_show_locus.

gcc/testsuite/ChangeLog:
	PR/62314
	* gcc.dg/plugin/diagnostic-test-show-locus-ascii-bw.c
	(test_fixit_insert): New.
	(test_fixit_remove): New.
	(test_fixit_replace): New.
	* gcc.dg/plugin/diagnostic-test-show-locus-ascii-color.c
	(test_fixit_insert): New.
	(test_fixit_remove): New.
	(test_fixit_replace): New.
	* gcc.dg/plugin/diagnostic-test-show-locus-utf-8-bw.c
	(test_fixit_insert): New.
	(test_fixit_remove): New.
	(test_fixit_replace): New.
	* gcc.dg/plugin/diagnostic-test-show-locus-utf-8-color.c
	(test_fixit_insert): New.
	(test_fixit_remove): New.
	(test_fixit_replace): New.
	* gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
	(test_show_locus): Add tests of rendering fixit hints.

libcpp/ChangeLog:
	PR/62314
	* include/line-map.h (source_range::intersects_line_p): New
	method.
	(rich_location::add_fixit_insert): New method.
	(rich_location::add_fixit_remove): New method.
	(rich_location::add_fixit_replace): New method.
	(rich_location::get_num_fixit_hints): New accessor.
	(rich_location::get_fixit_hint): New accessor.
	(rich_location::MAX_FIXIT_HINTS): New constant.
	(rich_location::m_num_fixit_hints): New field.
	(rich_location::m_fixit_hints): New field.
	(class fixit_hint): New class.
	(class fixit_insert): New class.
	(class fixit_remove): New class.
	(class fixit_replace): New class.
	* line-map.c (source_range::intersects_line_p): New method.
	(rich_location::rich_location): Add initialization of
	m_num_fixit_hints to both ctors.
	(rich_location::~rich_location): Delete the fixit hints.
	(rich_location::add_fixit_insert): New method.
	(rich_location::add_fixit_remove): New method.
	(rich_location::add_fixit_replace): New method.
	(fixit_insert::fixit_insert): New.
	(fixit_insert::~fixit_insert): New.
	(fixit_insert::affects_line_p): New.
	(fixit_remove::fixit_remove): New.
	(fixit_remove::affects_line_p): New.
	(fixit_replace::fixit_replace): New.
	(fixit_replace::~fixit_replace): New.
	(fixit_replace::affects_line_p): New.

Conflicts:
	gcc/diagnostic-show-locus.c
	gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-ascii-color.c
	gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c
	gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-utf-8-bw.c
	gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-utf-8-color.c
	libcpp/include/line-map.h
	libcpp/line-map.c
---
 gcc/diagnostic-show-locus.c                        | 111 ++++++++++++++++-
 .../gcc.dg/plugin/diagnostic-test-show-locus-bw.c  |  43 +++++++
 .../plugin/diagnostic-test-show-locus-color.c      |  43 +++++++
 .../plugin/diagnostic_plugin_test_show_locus.c     |  35 ++++++
 libcpp/include/line-map.h                          |  96 +++++++++++++++
 libcpp/line-map.c                                  | 136 ++++++++++++++++++++-
 6 files changed, 461 insertions(+), 3 deletions(-)

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 06d2086..7dd0ff2 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -273,6 +273,7 @@ class colorizer
 
   void set_range (int range_idx) { set_state (range_idx); }
   void set_normal_text () { set_state (STATE_NORMAL_TEXT); }
+  void set_fixit_hint () { set_state (0); }
 
  private:
   void set_state (int state);
@@ -342,6 +343,9 @@ class layout
   per_range_info *
   get_any_range (int line);
 
+  void
+  move_to_column (int *column, int dest_column);
+
  private:
   diagnostic_context *m_context;
   pretty_printer *m_pp;
@@ -605,7 +609,10 @@ layout::layout (diagnostic_context * context,
    describing the ranges.
 
    Both lines (1) and (2) may contain a right-most margin containing a
-   vertical bar and a caption, describing a range.  */
+   vertical bar and a caption, describing a range.
+
+   In addition, if there are fixit hints affecting this source line,
+   there will be one or more further lines printed, showing them.  */
 
 void
 layout::print_line (int row)
@@ -691,6 +698,87 @@ layout::print_line (int row)
 	}
     }
   pp_newline (m_pp);
+
+  /* Step 3: if there are any fixit hints on this source line, print them.
+     They are printed in order, attempting to combine them onto lines, but
+     starting new lines if necessary.  */
+  column = 0;
+
+  for (unsigned int i = 0; i < m_richloc->get_num_fixit_hints (); i++)
+    {
+      fixit_hint *hint = m_richloc->get_fixit_hint (i);
+      if (hint->affects_line_p (m_exploc.file, row))
+	{
+	  /* For now we assume each fixit hint can only touch one line.  */
+	  switch (hint->get_kind ())
+	    {
+	    case fixit_hint::INSERT:
+	      {
+		fixit_insert *insert = static_cast <fixit_insert *> (hint);
+		/* This assumes the insertion just affects one line.  */
+		int start_column
+		  = LOCATION_COLUMN (insert->get_location ());
+		move_to_column (&column, start_column);
+		m_colorizer.set_fixit_hint ();
+		pp_string (m_pp, insert->get_string ());
+		m_colorizer.set_normal_text ();
+		column += insert->get_length ();
+	      }
+	      break;
+
+	    case fixit_hint::REMOVE:
+	      {
+		fixit_remove *remove = static_cast <fixit_remove *> (hint);
+		/* This assumes the removal just affects one line.  */
+		source_range src_range = remove->get_range ();
+		int start_column = LOCATION_COLUMN (src_range.m_start);
+		int finish_column = LOCATION_COLUMN (src_range.m_finish);
+		move_to_column (&column, start_column);
+		for (int column = start_column; column <= finish_column; column++)
+		  {
+		    m_colorizer.set_fixit_hint ();
+		    pp_character (m_pp, '-');
+		    m_colorizer.set_normal_text ();
+		  }
+	      }
+	      break;
+
+	    case fixit_hint::REPLACE:
+	      {
+		fixit_replace *replace = static_cast <fixit_replace *> (hint);
+		int start_column
+		  = LOCATION_COLUMN (replace->get_range ().m_start);
+		move_to_column (&column, start_column);
+		m_colorizer.set_fixit_hint ();
+		pp_string (m_pp, replace->get_string ());
+		m_colorizer.set_normal_text ();
+		column += replace->get_length ();
+	      }
+	      break;
+
+	    default:
+	      gcc_unreachable ();
+	    }
+	}
+    }
+
+  /* Nasty workaround to convince the linker to add
+      rich_location::add_fixit_insert
+      rich_location::add_fixit_remove
+      rich_location::add_fixit_replace
+     to cc1 for use by diagnostic_plugin_test_show_locus,
+     before anything in cc1 is using them.
+
+     This conditional should never hold, but hopefully the compiler can't
+     figure that out.  */
+  if ('.' == global_dc->caret_chars[0])
+    {
+      m_richloc->add_fixit_insert (UNKNOWN_LOCATION, "");
+      m_richloc->add_fixit_remove
+	(source_range::from_location (UNKNOWN_LOCATION));
+      m_richloc->add_fixit_replace
+	(source_range::from_location (UNKNOWN_LOCATION), "");
+    }
 }
 
 /* Given a line of source code, get the per_range_info for the first range
@@ -707,6 +795,27 @@ layout::get_any_range (int line)
   return NULL;
 }
 
+/* Given *COLUMN as an x-coordinate, print spaces to position
+   successive output at DEST_COLUMN, printing a newline if necessary,
+   and updating *COLUMN.  */
+
+void
+layout::move_to_column (int *column, int dest_column)
+{
+  /* Start a new line if we need to.  */
+  if (*column > dest_column)
+    {
+      pp_newline (m_pp);
+      *column = 0;
+    }
+
+  while (*column < dest_column)
+    {
+      pp_space (m_pp);
+      (*column)++;
+    }
+}
+
 } /* End of anonymous namespace.  */
 
 /* For debugging layout issues in diagnostic_show_locus and friends,
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c
index b3fe9d8..cd038b3 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c
@@ -122,3 +122,46 @@ void test_very_wide_line (void)
    { dg-end-multiline-output "" } */
 #endif
 }
+
+/* Unit test for rendering of insertion fixit hints
+   (example taken from PR 62316).  */
+
+void test_fixit_insert (void)
+{
+#if 0
+   int a[2][2] = { 0, 1 , 2, 3 }; /* { dg-warning "insertion hints" } */
+/* { dg-begin-multiline-output "" }
+    int a[2][2] = { 0, 1 , 2, 3 };
+                    ^~~~
+                    {   }
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+/* Unit test for rendering of "remove" fixit hints.  */
+
+void test_fixit_remove (void)
+{
+#if 0
+  int a;; /* { dg-warning "example of a removal hint" } */
+/* { dg-begin-multiline-output "" }
+   int a;;
+         ^
+         -
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+/* Unit test for rendering of "replace" fixit hints.  */
+
+void test_fixit_replace (void)
+{
+#if 0
+  gtk_widget_showall (dlg); /* { dg-warning "example of a replacement hint" } */
+/* { dg-begin-multiline-output "" }
+   gtk_widget_showall (dlg);
+   ^~~~~~~~~~~~~~~~~~
+   gtk_widget_show_all
+   { dg-end-multiline-output "" } */
+#endif
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c
index 68fc1b5..112d276 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c
@@ -129,3 +129,46 @@ void test_very_wide_line (void)
    { dg-end-multiline-output "" } */
 #endif
 }
+
+/* Unit test for rendering of insertion fixit hints
+   (example taken from PR 62316).  */
+
+void test_fixit_insert (void)
+{
+#if 0
+   int a[2][2] = { 0, 1 , 2, 3 }; /* { dg-warning "insertion hints" } */
+/* { dg-begin-multiline-output "" }
+    int a[2][2] = { [01;35m[K0, 1[m[K , 2, 3 };
+                    [01;35m[K^~~~
+                    {[m[K   [01;35m[K}[m[K
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+/* Unit test for rendering of "remove" fixit hints.  */
+
+void test_fixit_remove (void)
+{
+#if 0
+  int a;; /* { dg-warning "example of a removal hint" } */
+/* { dg-begin-multiline-output "" }
+   int a;[01;35m[K;[m[K
+         [01;35m[K^
+         -[m[K
+   { dg-end-multiline-output "" } */
+#endif
+}
+
+/* Unit test for rendering of "replace" fixit hints.  */
+
+void test_fixit_replace (void)
+{
+#if 0
+  gtk_widget_showall (dlg); /* { dg-warning "example of a replacement hint" } */
+/* { dg-begin-multiline-output "" }
+   [01;35m[Kgtk_widget_showall[m[K (dlg);
+   [01;35m[K^~~~~~~~~~~~~~~~~~
+   gtk_widget_show_all[m[K
+   { dg-end-multiline-output "" } */
+#endif
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
index 8147f1d..7827ce5 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
@@ -241,6 +241,41 @@ test_show_locus (function *fun)
       richloc.set_range (0, src_range, true, false);
       warning_at_rich_loc (&richloc, 0, "test");
     }
+
+  /* Tests of rendering fixit hints.  */
+  if (0 == strcmp (fnname, "test_fixit_insert"))
+    {
+      const int line = fnstart_line + 2;
+      source_range src_range;
+      src_range.m_start = get_loc (line, 19);
+      src_range.m_finish = get_loc (line, 22);
+      gcc_rich_location richloc (src_range);
+      richloc.add_fixit_insert (src_range.m_start, "{");
+      richloc.add_fixit_insert (src_range.m_finish + 1, "}");
+      warning_at_rich_loc (&richloc, 0, "example of insertion hints");
+    }
+
+  if (0 == strcmp (fnname, "test_fixit_remove"))
+    {
+      const int line = fnstart_line + 2;
+      source_range src_range;
+      src_range.m_start = get_loc (line, 8);
+      src_range.m_finish = get_loc (line, 8);
+      gcc_rich_location richloc (src_range);
+      richloc.add_fixit_remove (src_range);
+      warning_at_rich_loc (&richloc, 0, "example of a removal hint");
+    }
+
+  if (0 == strcmp (fnname, "test_fixit_replace"))
+    {
+      const int line = fnstart_line + 2;
+      source_range src_range;
+      src_range.m_start = get_loc (line, 2);
+      src_range.m_finish = get_loc (line, 19);
+      gcc_rich_location richloc (src_range);
+      richloc.add_fixit_replace (src_range, "gtk_widget_show_all");
+      warning_at_rich_loc (&richloc, 0, "example of a replacement hint");
+    }
 }
 
 unsigned int
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index b2ee29a..19f1b32 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -145,6 +145,9 @@ struct GTY(()) source_range
     result.m_finish = loc;
     return result;
   }
+
+  /* Is there any part of this range on the given line?  */
+  bool intersects_line_p (const char *file, int line) const;
 };
 
 /* Memory allocation function typedef.  Works like xrealloc.  */
@@ -1094,6 +1097,11 @@ struct location_range
   bool contains_point (int row, int column) const;
 };
 
+class fixit_hint;
+  class fixit_insert;
+  class fixit_remove;
+  class fixit_replace;
+
 /* A "rich" source code location, for use when printing diagnostics.
    A rich_location has one or more ranges, each optionally with
    a caret.   Typically the zeroth range has a caret; other ranges
@@ -1179,6 +1187,9 @@ class rich_location
   /* Constructing from a source_range.  */
   rich_location (source_range src_range);
 
+  /* Destructor.  */
+  ~rich_location ();
+
   /* Accessors.  */
   source_location get_loc () const { return m_loc; }
 
@@ -1231,8 +1242,24 @@ class rich_location
   void
   override_column (int column);
 
+  /* Fix-it hints.  */
+  void
+  add_fixit_insert (source_location where,
+		    const char *new_content);
+
+  void
+  add_fixit_remove (source_range src_range);
+
+  void
+  add_fixit_replace (source_range src_range,
+		     const char *new_content);
+
+  unsigned int get_num_fixit_hints () const { return m_num_fixit_hints; }
+  fixit_hint *get_fixit_hint (int idx) const { return m_fixit_hints[idx]; }
+
 public:
   static const int MAX_RANGES = 3;
+  static const int MAX_FIXIT_HINTS = 2;
 
 protected:
   friend class range_iter;
@@ -1244,8 +1271,77 @@ protected:
 
   bool m_have_expanded_location;
   expanded_location m_expanded_location;
+
+  unsigned int m_num_fixit_hints;
+  fixit_hint *m_fixit_hints[MAX_FIXIT_HINTS];
+};
+
+class fixit_hint
+{
+public:
+  enum kind {INSERT, REMOVE, REPLACE};
+
+  virtual ~fixit_hint () {}
+
+  virtual enum kind get_kind () const = 0;
+  virtual bool affects_line_p (const char *file, int line) = 0;
+};
+
+class fixit_insert : public fixit_hint
+{
+ public:
+  fixit_insert (source_location where,
+		const char *new_content);
+  ~fixit_insert ();
+  enum kind get_kind () const { return INSERT; }
+  bool affects_line_p (const char *file, int line);
+
+  source_location get_location () const { return m_where; }
+  const char *get_string () const { return m_bytes; }
+  size_t get_length () const { return m_len; }
+
+ private:
+  source_location m_where;
+  char *m_bytes;
+  size_t m_len;
+};
+
+class fixit_remove : public fixit_hint
+{
+ public:
+  fixit_remove (source_range src_range);
+  ~fixit_remove () {}
+
+  enum kind get_kind () const { return REMOVE; }
+  bool affects_line_p (const char *file, int line);
+
+  source_range get_range () const { return m_src_range; }
+
+ private:
+  source_range m_src_range;
 };
 
+class fixit_replace : public fixit_hint
+{
+ public:
+  fixit_replace (source_range src_range,
+                 const char *new_content);
+  ~fixit_replace ();
+
+  enum kind get_kind () const { return REPLACE; }
+  bool affects_line_p (const char *file, int line);
+
+  source_range get_range () const { return m_src_range; }
+  const char *get_string () const { return m_bytes; }
+  size_t get_length () const { return m_len; }
+
+ private:
+  source_range m_src_range;
+  char *m_bytes;
+  size_t m_len;
+};
+
+
 inline
 rich_location::range_iter::range_iter (rich_location *richloc) :
   m_richloc (richloc),
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 3ef57d1..97a71e5 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -1761,6 +1761,28 @@ line_table_dump (FILE *stream, struct line_maps *set, unsigned int num_ordinary,
     }
 }
 
+/* struct source_range.  */
+
+/* Is there any part of this range on the given line?  */
+
+bool
+source_range::intersects_line_p (const char *file, int line) const
+{
+  expanded_location exploc_start
+    = linemap_client_expand_location_to_spelling_point (m_start);
+  if (file != exploc_start.file)
+    return false;
+  if (line < exploc_start.line)
+      return false;
+  expanded_location exploc_finish
+    = linemap_client_expand_location_to_spelling_point (m_finish);
+  if (file != exploc_finish.file)
+    return false;
+  if (line > exploc_finish.line)
+      return false;
+  return true;
+}
+
 /* class rich_location.  */
 
 /* Construct a rich_location with location LOC as its initial range.  */
@@ -1769,7 +1791,8 @@ rich_location::rich_location (line_maps *line_table,
 			      source_location loc) :
   m_loc (loc),
   m_num_ranges (0),
-  m_have_expanded_location (false)
+  m_have_expanded_location (false),
+  m_num_fixit_hints (0)
 {
   /* Set up the 0th range: */
   source_range src_range;
@@ -1787,12 +1810,21 @@ rich_location::rich_location (line_maps *line_table,
 rich_location::rich_location (source_range src_range)
 : m_loc (src_range.m_start),
   m_num_ranges (0),
-  m_have_expanded_location (false)
+  m_have_expanded_location (false),
+  m_num_fixit_hints (0)
 {
   /* Set up the 0th range: */
   add_range (src_range, true);
 }
 
+/* The destructor for class rich_location.  */
+
+rich_location::~rich_location ()
+{
+  for (unsigned int i = 0; i < m_num_fixit_hints; i++)
+    delete m_fixit_hints[i];
+}
+
 /* Get the first line of the rich_location, either that of
    the primary location, or of one of the ranges.  */
 
@@ -1930,3 +1962,103 @@ rich_location::set_range (unsigned int idx, source_range src_range,
       m_have_expanded_location = false;
     }
 }
+
+/* Add a fixit-hint, suggesting insertion of NEW_CONTENT
+   at WHERE.  */
+
+void
+rich_location::add_fixit_insert (source_location where,
+				 const char *new_content)
+{
+  linemap_assert (m_num_fixit_hints < MAX_FIXIT_HINTS);
+  m_fixit_hints[m_num_fixit_hints++]
+    = new fixit_insert (where, new_content);
+}
+
+/* Add a fixit-hint, suggesting removal of the content at
+   SRC_RANGE.  */
+
+void
+rich_location::add_fixit_remove (source_range src_range)
+{
+  linemap_assert (m_num_fixit_hints < MAX_FIXIT_HINTS);
+  m_fixit_hints[m_num_fixit_hints++] = new fixit_remove (src_range);
+}
+
+/* Add a fixit-hint, suggesting replacement of the content at
+   SRC_RANGE with NEW_CONTENT.  */
+
+void
+rich_location::add_fixit_replace (source_range src_range,
+				  const char *new_content)
+{
+  linemap_assert (m_num_fixit_hints < MAX_FIXIT_HINTS);
+  m_fixit_hints[m_num_fixit_hints++]
+    = new fixit_replace (src_range, new_content);
+}
+
+/* class fixit_insert.  */
+
+fixit_insert::fixit_insert (source_location where,
+			    const char *new_content)
+: m_where (where),
+  m_bytes (xstrdup (new_content)),
+  m_len (strlen (new_content))
+{
+}
+
+fixit_insert::~fixit_insert ()
+{
+  free (m_bytes);
+}
+
+/* Implementation of fixit_hint::affects_line_p for fixit_insert.  */
+
+bool
+fixit_insert::affects_line_p (const char *file, int line)
+{
+  expanded_location exploc
+    = linemap_client_expand_location_to_spelling_point (m_where);
+  if (file == exploc.file)
+    if (line == exploc.line)
+      return true;
+  return false;
+}
+
+/* class fixit_remove.  */
+
+fixit_remove::fixit_remove (source_range src_range)
+: m_src_range (src_range)
+{
+}
+
+/* Implementation of fixit_hint::affects_line_p for fixit_remove.  */
+
+bool
+fixit_remove::affects_line_p (const char *file, int line)
+{
+  return m_src_range.intersects_line_p (file, line);
+}
+
+/* class fixit_replace.  */
+
+fixit_replace::fixit_replace (source_range src_range,
+			      const char *new_content)
+: m_src_range (src_range),
+  m_bytes (xstrdup (new_content)),
+  m_len (strlen (new_content))
+{
+}
+
+fixit_replace::~fixit_replace ()
+{
+  free (m_bytes);
+}
+
+/* Implementation of fixit_hint::affects_line_p for fixit_replace.  */
+
+bool
+fixit_replace::affects_line_p (const char *file, int line)
+{
+  return m_src_range.intersects_line_p (file, line);
+}
-- 
1.8.5.3

