[1/2] analyzer: diagnostic_path: avoid printing redundant data

Message ID 20200102224444.2835-1-dmalcolm@redhat.com
State New
Headers show
Series
  • [1/2] analyzer: diagnostic_path: avoid printing redundant data
Related show

Commit Message

David Malcolm Jan. 2, 2020, 10:44 p.m.
This patch tweaks the default implementation of diagnostic_path
printing (-fdiagnostics-path-format=inline-events) to be less verbose
for various common cases.

Consider this synthetic diagnostic from the test plugin:

test.c: In function 'test':
test.c:29:5: error: passing NULL as argument 1 to 'PyList_Append' which
  requires a non-NULL parameter
   29 |     PyList_Append(list, item);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
  'test': events 1-3
    |
    |   25 |   list = PyList_New(0);
    |      |          ^~~~~~~~~~~~~
    |      |          |
    |      |          (1) when 'PyList_New' fails, returning NULL
    |   26 |
    |   27 |   for (i = 0; i < count; i++) {
    |      |   ~~~
    |      |   |
    |      |   (2) when 'i < count'
    |   28 |     item = PyLong_FromLong(random());
    |   29 |     PyList_Append(list, item);
    |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |     |
    |      |     (3) when calling 'PyList_Append', passing NULL from (1) as argument 1
    |

The diagnostic's primary location_t is contained within the path, so
printing the source code for it at the top before the path is redundant.

Also, this path is purely intraprocedural, so there's no point in
printing the "interprocedural margin" on the left-hand side, and there's
a single event run, so there's no point in printing the header for the
run.

This patch simplifies the output for the above to:

test.c: In function 'test':
test.c:29:5: error: passing NULL as argument 1 to 'PyList_Append' which
  requires a non-NULL parameter
   25 |   list = PyList_New(0);
      |          ^~~~~~~~~~~~~
      |          |
      |          (1) when 'PyList_New' fails, returning NULL
   26 |
   27 |   for (i = 0; i < count; i++) {
      |   ~~~
      |   |
      |   (2) when 'i < count'
   28 |     item = PyLong_FromLong(random());
   29 |     PyList_Append(list, item);
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
      |     |
      |     (3) when calling 'PyList_Append', passing NULL from (1) as argument 1

The patch replaces the diagnostic_context's print_path callback with an
object with two vfuncs, allowing for eliding diagnostic_show_locus for
the rich_location in favor of just printing the path for the common case
of a single location_t that's within the path, no fix-it hints or
labels, and -fdiagnostics-path-format=inline-events.

This went through several iterations; in one iteration I attempted to
use a small class hierarchy of per enum diagnostics_output_format
path_printer subclasses; this didn't work because the option-handling
is done in opts.o:common_handle_option, which is in
OBJS-libcommon-target, which isn't "tree-ish".  So this version has
a single implementation subclass in tree-diagnostic-path.cc.

This patch contains the non-analyzer changes; a follow-up updates the
expected output for the analyzer testsuite.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to dmalcolm/analyzer on the GCC git mirror.

gcc/ChangeLog:
	* diagnostic-format-json.cc (diagnostic_output_format_init): For
	the JSON case, update for conversion from a print_path callback to
	a m_path_printer object, deleting any such object.
	* diagnostic-show-locus.c (diagnostic_show_locus): Call any
	path_printer's maybe_print_path_rather_than_richloc and
	potentially bail out early.
	* diagnostic.c (diagnostic_initialize): Initializer m_path_printer
	and make_json_for_path.
	(diagnostic_finish): Delete any m_path_printer.
	(diagnostic_show_any_path): Update to call any m_path_printer's
	print_path vfunc.
	(rich_location::path_makes_location_redundant_p): New function.
	* diagnostic.h (class path_printer): New class.
	(diagnostic_context::print_path): Replace this callback with...
	(diagnostic_context::m_path_printer): ...this object pointer.
	(diagnostic_context::make_json_for_path): Fix overlong line.
	* doc/invoke.texi (-fdiagnostics-path-format=inline-events):
	Update intraprocedural example to reflect removal of event run
	header and interprocedural margin.  Add sentence describing
	how source code for location can be elided.
	(-fdiagnostics-show-path-depths): Fix reference to pertinent
	option.
	* tree-diagnostic-path.cc: (path_summary::m_path): New field.
	(path_summary::path_summary): Initialize it.
	(print_fndecl): Likewise.
	(path_summary::print): Add param "need_event_header".  Determine
	if the path is interprocedural vs purely intraprocedural.  Only
	print headers for event-runs if we need to, or if there's more
	than one event-run.  Only print the "interprocedural border" for
	interprocedural paths.  Only print the function name in event-run
	headers for interprocedural paths.
	(default_tree_diagnostic_path_printer): Replace with...
	(class impl_path_printer): ...this class.
	(path_printer::make_tree_default): New function.
	(selftest::test_intraprocedural_path): Update expected result for
	above changes; do with and without requiring event headers.
	* tree-diagnostic.c (tree_diagnostics_defaults): Replace
	initialization of print_path with that for m_path_printer.
	* tree-diagnostic.h (default_tree_diagnostic_path_printer): Delete decl.

gcc/testsuite/ChangeLog:
	* gcc.dg/plugin/diagnostic-path-format-default.c: Update expected
	output to remove source code from diagnostic locus, made redundant
	by path.
	* gcc.dg/plugin/diagnostic-path-format-inline-events-1.c:
	Likewise.
	* gcc.dg/plugin/diagnostic-path-format-inline-events-2.c:
	Likewise.
	* gcc.dg/plugin/diagnostic-path-format-inline-events-3.c:
	Likewise.
	* gcc.dg/plugin/diagnostic-test-paths-2.c: Likewise; also
	remove redundant event run header and interprocedual margin.
	* gcc.dg/plugin/diagnostic-test-paths-2a.c: New test.
	* gcc.dg/plugin/diagnostic-test-paths-4.c: Update expected output
	to remove source code from diagnostic locus, made redundant by
	path.
	* gcc.dg/plugin/diagnostic_plugin_test_paths.c
	(class pass_test_show_path): Add an m_dummy_label field and
	initialize it in the ctor.
	(example_1): Add a "dummy_label" param.  Use it to optionally add
	a labelled to the rich_location.
	(pass_test_show_path::execute): Pass m_dummy_label to the call to
	example_1.
	(make_pass_test_show_path): Delete.
	(plugin_init): Look for a "dummy_label" plugin argument and use
	it to initialize the pass's m_dummy_label field.  Call new directly
	to avoid passing around such params.
	* gcc.dg/plugin/plugin.exp (plugin_test_list): Add new test.

libcpp/ChangeLog:
	* include/line-map.h
	(rich_location::path_makes_location_redundant_p): New decl.
---
 gcc/diagnostic-format-json.cc                 |   4 +-
 gcc/diagnostic-show-locus.c                   |   7 +
 gcc/diagnostic.c                              |  43 ++-
 gcc/diagnostic.h                              |  38 ++-
 gcc/doc/invoke.texi                           |  39 +--
 .../plugin/diagnostic-path-format-default.c   |   2 -
 .../diagnostic-path-format-inline-events-1.c  |   2 -
 .../diagnostic-path-format-inline-events-2.c  |   2 -
 .../diagnostic-path-format-inline-events-3.c  |   2 -
 .../gcc.dg/plugin/diagnostic-test-paths-2.c   |  31 +-
 .../gcc.dg/plugin/diagnostic-test-paths-2a.c  |  58 ++++
 .../gcc.dg/plugin/diagnostic-test-paths-4.c   |   2 -
 .../plugin/diagnostic_plugin_test_paths.c     |  33 +-
 gcc/testsuite/gcc.dg/plugin/plugin.exp        |   1 +
 gcc/tree-diagnostic-path.cc                   | 299 ++++++++++++------
 gcc/tree-diagnostic.c                         |   2 +-
 gcc/tree-diagnostic.h                         |   2 -
 libcpp/include/line-map.h                     |   2 +
 18 files changed, 401 insertions(+), 168 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2a.c

-- 
2.21.0

Patch

diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc
index 60363348a92e..181ba8a65afc 100644
--- a/gcc/diagnostic-format-json.cc
+++ b/gcc/diagnostic-format-json.cc
@@ -295,7 +295,9 @@  diagnostic_output_format_init (diagnostic_context *context,
 	context->begin_group_cb = json_begin_group;
 	context->end_group_cb =  json_end_group;
 	context->final_cb =  json_final_cb;
-	context->print_path = NULL; /* handled in json_end_diagnostic.  */
+	/* Handled in json_end_diagnostic:  */
+	delete context->m_path_printer;
+	context->m_path_printer = NULL;
 
 	/* The metadata is handled in JSON format, rather than as text.  */
 	context->show_cwe = false;
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index 3208d99c3451..055a264ae01c 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -2570,6 +2570,13 @@  diagnostic_show_locus (diagnostic_context * context,
 
   context->last_location = loc;
 
+  /* If we have a path that which when printed would make printing
+     RICHLOC redundant, then print that now instead.  */
+  if (context->m_path_printer)
+    if (context->m_path_printer->maybe_print_path_rather_than_richloc
+	  (context, *richloc))
+      return;
+
   layout layout (context, richloc, diagnostic_kind);
   for (int line_span_idx = 0; line_span_idx < layout.get_num_line_spans ();
        line_span_idx++)
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 52d5ffbfb4a0..fe540d9a133d 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -210,6 +210,8 @@  diagnostic_initialize (diagnostic_context *context, int n_opts)
   context->get_option_url = NULL;
   context->last_location = UNKNOWN_LOCATION;
   context->last_module = 0;
+  context->m_path_printer = NULL;
+  context->make_json_for_path = NULL;
   context->x_data = NULL;
   context->lock = 0;
   context->inhibit_notes_p = false;
@@ -287,6 +289,9 @@  diagnostic_finish (diagnostic_context *context)
   XDELETE (context->printer);
   context->printer = NULL;
 
+  delete context->m_path_printer;
+  context->m_path_printer = NULL;
+
   if (context->edit_context_ptr)
     {
       delete context->edit_context_ptr;
@@ -673,8 +678,8 @@  diagnostic_show_any_path (diagnostic_context *context,
   if (!path)
     return;
 
-  if (context->print_path)
-    context->print_path (context, path);
+  if (context->m_path_printer)
+    context->m_path_printer->print_path (context, *diagnostic->richloc);
 }
 
 /* Return true if the events in this path involve more than one
@@ -694,6 +699,40 @@  diagnostic_path::interprocedural_p () const
   return false;
 }
 
+/* Return true if it would be redundant to print this rich_location with
+   diagnostic_show_locus if we were to also then print the path of this
+   rich_location.
+
+   Specifically return true if this rich_location has a single location,
+   and a path, no fix-it hints or labels, and the single location is
+   within the path.
+
+   Implemented here as libcpp has only a forward decl of diagnostic_path.  */
+
+bool
+rich_location::path_makes_location_redundant_p () const
+{
+  if (m_path == NULL)
+    return false;
+  if (get_num_locations () != 1)
+    return false;
+
+  /* Must be no fix-it hints or labels. */
+  if (get_num_fixit_hints () > 0)
+    return false;
+  const location_range *lr = get_range (0);
+  if (lr->m_label)
+    return false;
+
+  /* Check if primary location is within path.  */
+  location_t loc = get_loc ();
+  const unsigned num = m_path->num_events ();
+  for (unsigned i = 0; i < num; i++)
+    if (m_path->get_event (i).get_location () == loc)
+      return true;
+  return false;
+}
+
 void
 default_diagnostic_starter (diagnostic_context *context,
 			    diagnostic_info *diagnostic)
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 2e42d95752fc..7691a8b1d114 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -99,6 +99,38 @@  typedef void (*diagnostic_finalizer_fn) (diagnostic_context *,
 class edit_context;
 namespace json { class value; }
 
+/* Abstract base class for printing diagnostic_paths, to isolate
+   the path-printing code (which uses tree and thus is in OBJS) from the
+   core diagnostic machinery (which is in OBJS-libcommon).
+
+   Concrete implementation is in tree-diagnostic-path.cc.
+
+   Implemented as a pair of calls, and thus as a pair of vfuncs, rather than
+   a pair of callbacks.
+
+   This is done to allow -fdiagnostics-path-format=inline-events to
+   consolidate and print less for, the common case where the path contains the
+   rich_location, and hence the rich_location can be elided when it is
+   redundant.  */
+
+class path_printer
+{
+public:
+  static path_printer *make_tree_default ();
+  virtual ~path_printer () {}
+
+  /* Vfunc called by diagnostic_show_locus: potentially print the path
+     as inline events if the rest of the rich_location is redundant,
+     returning true if this is the case.  */
+  virtual bool maybe_print_path_rather_than_richloc (diagnostic_context *,
+						     const rich_location &) = 0;
+
+  /* Vfunc called after the call to diagnostic_show_locus: print the path, if
+     it wasn't printed already.  */
+  virtual void print_path (diagnostic_context *,
+			   const rich_location &) = 0;
+};
+
 /* This data structure bundles altogether any information relevant to
    the context of a diagnostic message.  */
 struct diagnostic_context
@@ -232,8 +264,10 @@  struct diagnostic_context
      particular option.  */
   char *(*get_option_url) (diagnostic_context *, int);
 
-  void (*print_path) (diagnostic_context *, const diagnostic_path *);
-  json::value *(*make_json_for_path) (diagnostic_context *, const diagnostic_path *);
+  /* Client hooks for working with diagnostic_paths.  */
+  path_printer *m_path_printer;
+  json::value *(*make_json_for_path) (diagnostic_context *,
+				      const diagnostic_path *);
 
   /* Auxiliary data for client.  */
   void *x_data;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index dc6d5a14ed41..c471a602d23a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4177,27 +4177,24 @@  test.c:29:5: note: (3) when calling 'PyList_Append', passing NULL from (1) as ar
 @samp{inline-events} means to print the events ``inline'' within the source
 code.  This view attempts to consolidate the events into runs of
 sufficiently-close events, printing them as labelled ranges within the source.
-
 For example, the same events as above might be printed as:
 
 @smallexample
-  'test': events 1-3
-    |
-    |   25 |   list = PyList_New(0);
-    |      |          ^~~~~~~~~~~~~
-    |      |          |
-    |      |          (1) when 'PyList_New' fails, returning NULL
-    |   26 |
-    |   27 |   for (i = 0; i < count; i++) @{
-    |      |   ~~~
-    |      |   |
-    |      |   (2) when 'i < count'
-    |   28 |     item = PyLong_FromLong(random());
-    |   29 |     PyList_Append(list, item);
-    |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
-    |      |     |
-    |      |     (3) when calling 'PyList_Append', passing NULL from (1) as argument 1
-    |
+test.c:29:5: error: passing NULL as argument 1 to 'PyList_Append' which requires a non-NULL parameter
+   25 |   list = PyList_New(0);
+      |          ^~~~~~~~~~~~~
+      |          |
+      |          (1) when 'PyList_New' fails, returning NULL
+   26 |
+   27 |   for (i = 0; i < count; i++) @{
+      |   ~~~
+      |   |
+      |   (2) when 'i < count'
+   28 |     item = PyLong_FromLong(random());
+   29 |     PyList_Append(list, item);
+      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
+      |     |
+      |     (3) when calling 'PyList_Append', passing NULL from (1) as argument 1
 @end smallexample
 
 Interprocedural control flow is shown by grouping the events by stack frame,
@@ -4251,13 +4248,17 @@  For example:
 (etc)
 @end smallexample
 
+If the location of the diagnostic is contained within the path and there are
+no fix-it hints or other complications, the source code for the location
+is elided and just the path is printed.
+
 @item -fdiagnostics-show-path-depths
 @opindex fdiagnostics-show-path-depths
 This option provides additional information when printing control-flow paths
 associated with a diagnostic.
 
 If this is option is provided then the stack depth will be printed for
-each run of events within @option{-fdiagnostics-path-format=separate-events}.
+each run of events within @option{-fdiagnostics-path-format=inline-events}.
 
 This is intended for use by GCC developers and plugin developers when
 debugging diagnostics that report interprocedural control flow.
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-default.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-default.c
index 5712dbd64725..d0ad8d6b987d 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-default.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-default.c
@@ -12,8 +12,6 @@  void wrapped_free (void *ptr)
 {
   free (ptr); /* { dg-warning "double-free of 'ptr' \\\[CWE-415\\]" } */
   /* { dg-begin-multiline-output "" }
-   free (ptr);
-   ^~~~~~~~~~
   'test': events 1-2
     |
     | {
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-1.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-1.c
index 430d81737718..5fc005a767b3 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-1.c
@@ -12,8 +12,6 @@  void wrapped_free (void *ptr)
 {
   free (ptr); /* { dg-warning "double-free of 'ptr' \\\[CWE-415\\]" } */
   /* { dg-begin-multiline-output "" }
-   free (ptr);
-   ^~~~~~~~~~
   'test': events 1-2
     |
     | {
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-2.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-2.c
index c2bfabec9101..7072308185cd 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-2.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-2.c
@@ -17,8 +17,6 @@  void wrapped_free (void *ptr)
 {
   free (ptr); /* { dg-warning "double-free of 'ptr' \\\[CWE-415\\]" } */
   /* { dg-begin-multiline-output "" }
-   free (ptr);
-   ^~~~~~~~~~
   'test': events 1-2
     |
     | {
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-3.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-3.c
index 5e7b099f7273..fa346628b14c 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-3.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-3.c
@@ -16,8 +16,6 @@  void wrapped_free (void *ptr)
 {
   free (ptr); /* { dg-warning "double-free of 'ptr' \\\[CWE-415\\]" } */
   /* { dg-begin-multiline-output "" }
-   NN |   free (ptr);
-      |   ^~~~~~~~~~
   'test': events 1-2
     |
     |   NN | {
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2.c
index 391aeb9ec30f..d436b0e5d6cf 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2.c
@@ -33,24 +33,19 @@  make_a_list_of_random_ints_badly(PyObject *self,
 
   /* { dg-error "passing NULL as argument 1 to 'PyList_Append' which requires a non-NULL parameter" "" { target *-*-* } PyList_Append } */
   /* { dg-begin-multiline-output "" }
+   25 |   list = PyList_New(0);
+      |          ^~~~~~~~~~~~~
+      |          |
+      |          (1) when 'PyList_New' fails, returning NULL
+   26 | 
+   27 |   for (i = 0; i < count; i++) {
+      |   ~~~     
+      |   |
+      |   (2) when 'i < count'
+   28 |     item = PyLong_FromLong(random());
    29 |     PyList_Append(list, item);
-      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
-  'make_a_list_of_random_ints_badly': events 1-3
-    |
-    |   25 |   list = PyList_New(0);
-    |      |          ^~~~~~~~~~~~~
-    |      |          |
-    |      |          (1) when 'PyList_New' fails, returning NULL
-    |   26 | 
-    |   27 |   for (i = 0; i < count; i++) {
-    |      |   ~~~     
-    |      |   |
-    |      |   (2) when 'i < count'
-    |   28 |     item = PyLong_FromLong(random());
-    |   29 |     PyList_Append(list, item);
-    |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
-    |      |     |
-    |      |     (3) when calling 'PyList_Append', passing NULL from (1) as argument 1
-    |
+      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
+      |     |
+      |     (3) when calling 'PyList_Append', passing NULL from (1) as argument 1
      { dg-end-multiline-output "" } */
 }
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2a.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2a.c
new file mode 100644
index 000000000000..e8dc5bf82155
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2a.c
@@ -0,0 +1,58 @@ 
+/* Example of a path that can't elide its richloc, due to a label.  */
+
+/* { dg-do compile } */
+/* { dg-options "-fdiagnostics-show-caret -fdiagnostics-show-line-numbers -fplugin-arg-diagnostic_plugin_test_paths-dummy_label" } */
+
+#include <stddef.h>
+#include <stdlib.h>
+
+/* Minimal reimplementation of cpython API.  */
+typedef struct PyObject {} PyObject;
+extern int PyArg_ParseTuple (PyObject *args, const char *fmt, ...);
+extern PyObject *PyList_New (int);
+extern PyObject *PyLong_FromLong(long);
+extern void PyList_Append(PyObject *list, PyObject *item);
+
+PyObject *
+make_a_list_of_random_ints_badly (PyObject *self,
+				  PyObject *args)
+{
+  PyObject *list, *item;
+  long count, i;
+
+  if (!PyArg_ParseTuple(args, "i", &count)) {
+    return NULL;
+  }
+
+  list = PyList_New(0); /* { dg-line PyList_New } */
+	
+  for (i = 0; i < count; i++) {
+    item = PyLong_FromLong(random());
+    PyList_Append(list, item); /* { dg-line PyList_Append } */
+  }
+  
+  return list;
+
+  /* { dg-error "passing NULL as argument 1 to 'PyList_Append' which requires a non-NULL parameter" "" { target *-*-* } PyList_Append } */
+  /* { dg-begin-multiline-output "" }
+   31 |     PyList_Append(list, item);
+      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
+      |     |
+      |     dummy label
+  events 1-3
+   27 |   list = PyList_New(0);
+      |          ^~~~~~~~~~~~~
+      |          |
+      |          (1) when 'PyList_New' fails, returning NULL
+   28 | 
+   29 |   for (i = 0; i < count; i++) {
+      |   ~~~     
+      |   |
+      |   (2) when 'i < count'
+   30 |     item = PyLong_FromLong(random());
+   31 |     PyList_Append(list, item);
+      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
+      |     |
+      |     (3) when calling 'PyList_Append', passing NULL from (1) as argument 1
+     { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
index 41cbaaaaa976..5f1648b0fbce 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
@@ -29,8 +29,6 @@  void test (void)
 }
 
 /* { dg-begin-multiline-output "" }
-   NN |   fprintf(stderr, "LOG: %s", msg);
-      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   'test': events 1-2
     |
     |   NN | {
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
index cf05ca3a5d32..fa075057d504 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
@@ -57,7 +57,7 @@  const pass_data pass_data_test_show_path =
 class pass_test_show_path : public ipa_opt_pass_d
 {
 public:
-  pass_test_show_path(gcc::context *ctxt)
+  pass_test_show_path(gcc::context *ctxt, bool dummy_label)
     : ipa_opt_pass_d (pass_data_test_show_path, ctxt,
 		      NULL, /* generate_summary */
 		      NULL, /* write_summary */
@@ -67,13 +67,16 @@  public:
 		      NULL, /* stmt_fixup */
 		      0, /* function_transform_todo_flags_start */
 		      NULL, /* function_transform */
-		      NULL) /* variable_transform */
+		      NULL), /* variable_transform */
+    m_dummy_label (dummy_label)
   {}
 
   /* opt_pass methods: */
   bool gate (function *) { return true; }
   virtual unsigned int execute (function *);
 
+  bool m_dummy_label;
+
 }; // class pass_test_show_path
 
 /* Determine if STMT is a call with NUM_ARGS arguments to a function
@@ -110,7 +113,7 @@  check_for_named_call (gimple *stmt,
 /* Example 1: a purely intraprocedural path.  */
 
 static void
-example_1 ()
+example_1 (bool dummy_label)
 {
   gimple_stmt_iterator gsi;
   basic_block bb;
@@ -146,6 +149,13 @@  example_1 ()
     {
       auto_diagnostic_group d;
       gcc_rich_location richloc (gimple_location (call_to_PyList_Append));
+
+      text_range_label label ("dummy label");
+      if (dummy_label)
+	richloc.add_range (gimple_location (call_to_PyList_Append),
+			   SHOW_RANGE_WITHOUT_CARET,
+			   &label);
+
       simple_diagnostic_path path (global_dc->printer);
       diagnostic_event_id_t alloc_event_id
 	= path.add_event (gimple_location (call_to_PyList_New),
@@ -424,19 +434,13 @@  example_3 ()
 unsigned int
 pass_test_show_path::execute (function *)
 {
-  example_1 ();
+  example_1 (m_dummy_label);
   example_2 ();
   example_3 ();
 
   return 0;
 }
 
-static opt_pass *
-make_pass_test_show_path (gcc::context *ctxt)
-{
-  return new pass_test_show_path (ctxt);
-}
-
 int
 plugin_init (struct plugin_name_args *plugin_info,
 	     struct plugin_gcc_version *version)
@@ -449,7 +453,14 @@  plugin_init (struct plugin_name_args *plugin_info,
   if (!plugin_default_version_check (version, &gcc_version))
     return 1;
 
-  pass_info.pass = make_pass_test_show_path (g);
+  bool dummy_label = false;
+  for (int i = 0; i < argc; i++)
+    {
+      if (strcmp (argv[i].key, "dummy_label") == 0)
+	dummy_label = true;
+    }
+
+  pass_info.pass = new pass_test_show_path (g, dummy_label);
   pass_info.reference_pass_name = "whole-program";
   pass_info.ref_pass_instance_number = 1;
   pass_info.pos_op = PASS_POS_INSERT_BEFORE;
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index c380ac942bd9..65d34625a1ce 100644
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -98,6 +98,7 @@  set plugin_test_list [list \
     { diagnostic_plugin_test_paths.c \
 	  diagnostic-test-paths-1.c \
 	  diagnostic-test-paths-2.c \
+	  diagnostic-test-paths-2a.c \
 	  diagnostic-test-paths-3.c \
 	  diagnostic-test-paths-4.c \
 	  diagnostic-path-format-default.c \
diff --git a/gcc/tree-diagnostic-path.cc b/gcc/tree-diagnostic-path.cc
index 32dc054519b7..da7d4893a2d5 100644
--- a/gcc/tree-diagnostic-path.cc
+++ b/gcc/tree-diagnostic-path.cc
@@ -211,11 +211,13 @@  class path_summary
  public:
   path_summary (const diagnostic_path &path, bool check_rich_locations);
 
-  void print (diagnostic_context *dc, bool show_depths) const;
+  void print (diagnostic_context *dc, bool show_depths,
+	      bool need_event_header = true) const;
 
   unsigned get_num_ranges () const { return m_ranges.length (); }
 
  private:
+  const diagnostic_path &m_path;
   auto_delete_vec <event_range> m_ranges;
 };
 
@@ -223,6 +225,7 @@  class path_summary
 
 path_summary::path_summary (const diagnostic_path &path,
 			    bool check_rich_locations)
+: m_path (path)
 {
   const unsigned num_events = path.num_events ();
 
@@ -272,34 +275,42 @@  print_fndecl (pretty_printer *pp, tree fndecl, bool quoted)
    descriptions within calls to diagnostic_show_locus, using labels to
    show the events:
 
-   'foo' (events 1-2)
+   'foo': events 1-2
      | NN |
      |    |
-     +--> 'bar' (events 3-4)
+     +--> 'bar': events 3-4
             | NN |
             |    |
-            +--> 'baz' (events 5-6)
+            +--> 'baz': events 5-6
                    | NN |
                    |    |
      <------------ +
      |
-   'foo' (events 7-8)
+   'foo': events 7-8
      | NN |
      |    |
-     +--> 'bar' (events 9-10)
+     +--> 'bar': events 9-10
             | NN |
             |    |
-            +--> 'baz' (events 11-12)
+            +--> 'baz': events 11-12
                    | NN |
                    |    |
 
    If SHOW_DEPTHS is true, append " (depth N)" to the header of each run
    of events.
 
-   For events with UNKNOWN_LOCATION, print a summary of each the event.  */
+   For events with UNKNOWN_LOCATION, print a summary of each the event.
+
+   If the path is purely intraprocedural, don't print the function name
+   or the left-hand stack-depth lines.
+
+   If NEED_EVENT_HEADER is true, then a header is printed for every run
+   of events.  Otherwise, headers are only printed if there is more than
+   one run of events.  */
 
 void
-path_summary::print (diagnostic_context *dc, bool show_depths) const
+path_summary::print (diagnostic_context *dc,  bool show_depths,
+		     bool need_event_header) const
 {
   pretty_printer *pp = dc->printer;
 
@@ -317,6 +328,8 @@  path_summary::print (diagnostic_context *dc, bool show_depths) const
   typedef int_hash <int, EMPTY, DELETED> vbar_hash;
   hash_map <vbar_hash, int> vbar_column_for_depth;
 
+  const bool interprocedural = m_path.interprocedural_p ();
+
   /* Print the ranges.  */
   const int base_indent = 2;
   int cur_indent = base_indent;
@@ -324,67 +337,77 @@  path_summary::print (diagnostic_context *dc, bool show_depths) const
   event_range *range;
   FOR_EACH_VEC_ELT (m_ranges, i, range)
     {
-      write_indent (pp, cur_indent);
-      if (i > 0)
+      /* Write the header line for a run of events.  */
+      if (m_ranges.length () > 1 || need_event_header)
 	{
-	  const path_summary::event_range *prev_range
-	    = m_ranges[i - 1];
-	  if (range->m_stack_depth > prev_range->m_stack_depth)
+	  write_indent (pp, cur_indent);
+	  if (i > 0)
 	    {
-	      /* Show pushed stack frame(s).  */
-	      const char *push_prefix = "+--> ";
-	      pp_string (pp, start_line_color);
-	      pp_string (pp, push_prefix);
-	      pp_string (pp, end_line_color);
-	      cur_indent += strlen (push_prefix);
+	      const path_summary::event_range *prev_range
+		= m_ranges[i - 1];
+	      if (range->m_stack_depth > prev_range->m_stack_depth)
+		{
+		  /* Show pushed stack frame(s).  */
+		  const char *push_prefix = "+--> ";
+		  pp_string (pp, start_line_color);
+		  pp_string (pp, push_prefix);
+		  pp_string (pp, end_line_color);
+		  cur_indent += strlen (push_prefix);
+		}
 	    }
+	  if (interprocedural && range->m_fndecl)
+	    {
+	      print_fndecl (pp, range->m_fndecl, true);
+	      pp_string (pp, ": ");
+	    }
+	  if (range->m_start_idx == range->m_end_idx)
+	    pp_printf (pp, "event %i",
+		       range->m_start_idx + 1);
+	  else
+	    pp_printf (pp, "events %i-%i",
+		       range->m_start_idx + 1, range->m_end_idx + 1);
+	  if (show_depths)
+	    pp_printf (pp, " (depth %i)", range->m_stack_depth);
+	  pp_newline (pp);
 	}
-      if (range->m_fndecl)
-	{
-	  print_fndecl (pp, range->m_fndecl, true);
-	  pp_string (pp, ": ");
-	}
-      if (range->m_start_idx == range->m_end_idx)
-	pp_printf (pp, "event %i",
-		   range->m_start_idx + 1);
-      else
-	pp_printf (pp, "events %i-%i",
-		   range->m_start_idx + 1, range->m_end_idx + 1);
-      if (show_depths)
-	pp_printf (pp, " (depth %i)", range->m_stack_depth);
-      pp_newline (pp);
 
       /* Print a run of events.  */
       {
-	write_indent (pp, cur_indent + per_frame_indent);
-	pp_string (pp, start_line_color);
-	pp_string (pp, "|");
-	pp_string (pp, end_line_color);
-	pp_newline (pp);
+	if (interprocedural)
+	  {
+	    write_indent (pp, cur_indent + per_frame_indent);
+	    pp_string (pp, start_line_color);
+	    pp_string (pp, "|");
+	    pp_string (pp, end_line_color);
+	    pp_newline (pp);
+	  }
 
 	char *saved_prefix = pp_take_prefix (pp);
 	char *prefix;
-	{
-	  pretty_printer tmp_pp;
-	  write_indent (&tmp_pp, cur_indent + per_frame_indent);
-	  pp_string (&tmp_pp, start_line_color);
-	  pp_string (&tmp_pp, "|");
-	  pp_string (&tmp_pp, end_line_color);
-	  prefix = xstrdup (pp_formatted_text (&tmp_pp));
-	}
-	pp_set_prefix (pp, prefix);
-	pp_prefixing_rule (pp) = DIAGNOSTICS_SHOW_PREFIX_EVERY_LINE;
+	if (interprocedural)
+	  {
+	    pretty_printer tmp_pp;
+	    write_indent (&tmp_pp, cur_indent + per_frame_indent);
+	    pp_string (&tmp_pp, start_line_color);
+	    pp_string (&tmp_pp, "|");
+	    pp_string (&tmp_pp, end_line_color);
+	    prefix = xstrdup (pp_formatted_text (&tmp_pp));
+	    pp_set_prefix (pp, prefix);
+	    pp_prefixing_rule (pp) = DIAGNOSTICS_SHOW_PREFIX_EVERY_LINE;
+	  }
 	range->print (dc);
-	pp_set_prefix (pp, saved_prefix);
-
-	write_indent (pp, cur_indent + per_frame_indent);
-	pp_string (pp, start_line_color);
-	pp_string (pp, "|");
-	pp_string (pp, end_line_color);
-	pp_newline (pp);
+	if (interprocedural)
+	  {
+	    pp_set_prefix (pp, saved_prefix);
+	    write_indent (pp, cur_indent + per_frame_indent);
+	    pp_string (pp, start_line_color);
+	    pp_string (pp, "|");
+	    pp_string (pp, end_line_color);
+	    pp_newline (pp);
+	  }
       }
 
-      if (i < m_ranges.length () - 1)
+      if (interprocedural && i < m_ranges.length () - 1)
 	{
 	  const path_summary::event_range *next_range
 	    = m_ranges[i + 1];
@@ -441,51 +464,116 @@  path_summary::print (diagnostic_context *dc, bool show_depths) const
     }
 }
 
-} /* end of anonymous namespace for path-printing code.  */
+/* Concrete implementation of path_printer.
 
-/* Print PATH to CONTEXT, according to CONTEXT's path_format.  */
+   Putting this here isolates the path-printing code (which uses tree and
+   thus is in OBJS) from the core diagnostic machinery (which is in
+   OBJS-libcommon).  */
 
-void
-default_tree_diagnostic_path_printer (diagnostic_context *context,
-				      const diagnostic_path *path)
+class impl_path_printer : public path_printer
 {
-  gcc_assert (path);
-
-  const unsigned num_events = path->num_events ();
-
-  switch (context->path_format)
-    {
-    case DPF_NONE:
-      /* Do nothing.  */
-      return;
-
-    case DPF_SEPARATE_EVENTS:
+public:
+  bool maybe_print_path_rather_than_richloc (diagnostic_context *context,
+					     const rich_location &richloc)
+    FINAL OVERRIDE
+  {
+    switch (context->path_format)
       {
-	/* A note per event.  */
-	for (unsigned i = 0; i < num_events; i++)
-	  {
-	    const diagnostic_event &event = path->get_event (i);
-	    label_text event_text (event.get_desc (false));
-	    gcc_assert (event_text.m_buffer);
-	    diagnostic_event_id_t event_id (i);
-	    inform (event.get_location (),
-		    "%@ %s", &event_id, event_text.m_buffer);
-	    event_text.maybe_free ();
-	  }
+      default:
+	gcc_unreachable ();
+      case DPF_NONE:
+      case DPF_SEPARATE_EVENTS:
+	return false;
+
+      case DPF_INLINE_EVENTS:
+	{
+	  /* If we can, then print the path here, rather than RICHLOC. */
+	  if (richloc.path_makes_location_redundant_p ())
+	    {
+	      impl_print_path (context, richloc, false);
+	      return true;
+	    }
+	  return false;
+	}
       }
-      break;
+  }
 
-    case DPF_INLINE_EVENTS:
+  void print_path (diagnostic_context *context,
+		   const rich_location &richloc) FINAL OVERRIDE
+  {
+    switch (context->path_format)
       {
-	/* Consolidate related events.  */
-	path_summary summary (*path, true);
-	char *saved_prefix = pp_take_prefix (context->printer);
-	pp_set_prefix (context->printer, NULL);
-	summary.print (context, context->show_path_depths);
-	pp_flush (context->printer);
-	pp_set_prefix (context->printer, saved_prefix);
+      default:
+	gcc_unreachable ();
+      case DPF_NONE:
+	break;
+      case DPF_SEPARATE_EVENTS:
+	{
+	  const diagnostic_path *path = richloc.get_path ();
+	  gcc_assert (path);
+
+	  const unsigned num_events = path->num_events ();
+
+	  /* A note per event.  */
+	  for (unsigned i = 0; i < num_events; i++)
+	    {
+	      const diagnostic_event &event = path->get_event (i);
+	      label_text event_text (event.get_desc (false));
+	      gcc_assert (event_text.m_buffer);
+	      diagnostic_event_id_t event_id (i);
+	      inform (event.get_location (),
+		      "%@ %s", &event_id, event_text.m_buffer);
+	      event_text.maybe_free ();
+	    }
+	}
+	break;
+
+      case DPF_INLINE_EVENTS:
+	{
+	  if (richloc.path_makes_location_redundant_p ())
+	    {
+	      /* Then we already printed the path during diagnostic_show_locus
+		 on RICHLOC; nothing left to do.  */
+	      return;
+	    }
+	  /* Otherwise, print the path.  We have also printed RICHLOC
+	     via diagnostic_show_locus, so we must print event headers, to
+	     stop the path following on directly from the diagnostic_show_locus
+	     call.  */
+	  impl_print_path (context, richloc, true);
+	}
       }
-    }
+  }
+
+  void impl_print_path (diagnostic_context *context,
+			const rich_location &richloc,
+			bool need_event_header)
+  {
+    const diagnostic_path *path = richloc.get_path ();
+    gcc_assert (path);
+
+    /* Consolidate related events.  */
+    path_summary summary (*path, true);
+    char *saved_prefix = pp_take_prefix (context->printer);
+    pp_set_prefix (context->printer, NULL);
+    summary.print (context, context->show_path_depths,
+		   need_event_header);
+    pp_flush (context->printer);
+    pp_set_prefix (context->printer, saved_prefix);
+  }
+};
+
+} /* end of anonymous namespace for path-printing code.  */
+
+/* class path_printer.  */
+
+/* Make a concrete path_printer instance.
+   The caller is reponsible for deleting it.  */
+
+path_printer *
+path_printer::make_tree_default ()
+{
+  return new impl_path_printer ();
 }
 
 /* This has to be here, rather than diagnostic-format-json.cc,
@@ -591,14 +679,21 @@  test_intraprocedural_path (pretty_printer *event_pp)
   path_summary summary (path, false);
   ASSERT_EQ (summary.get_num_ranges (), 1);
 
-  test_diagnostic_context dc;
-  summary.print (&dc, true);
-  ASSERT_STREQ ("  `foo': events 1-2 (depth 0)\n"
-		"    |\n"
-		"    | (1): first `free'\n"
-		"    | (2): double `free'\n"
-		"    |\n",
-		pp_formatted_text (dc.printer));
+  {
+    test_diagnostic_context dc;
+    summary.print (&dc, true, false);
+    ASSERT_STREQ (" (1): first `free'\n"
+		  " (2): double `free'\n",
+		  pp_formatted_text (dc.printer));
+  }
+  {
+    test_diagnostic_context dc;
+    summary.print (&dc, true, true);
+    ASSERT_STREQ ("  events 1-2 (depth 0)\n"
+		  " (1): first `free'\n"
+		  " (2): double `free'\n",
+		  pp_formatted_text (dc.printer));
+  }
 }
 
 /* Verify that print_path_summary works on an interprocedural path.  */
diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c
index d5f2f9e7e2d7..b8b78f5ea279 100644
--- a/gcc/tree-diagnostic.c
+++ b/gcc/tree-diagnostic.c
@@ -312,6 +312,6 @@  tree_diagnostics_defaults (diagnostic_context *context)
   diagnostic_starter (context) = default_tree_diagnostic_starter;
   diagnostic_finalizer (context) = default_diagnostic_finalizer;
   diagnostic_format_decoder (context) = default_tree_printer;
-  context->print_path = default_tree_diagnostic_path_printer;
+  context->m_path_printer = path_printer::make_tree_default ();
   context->make_json_for_path = default_tree_make_json_for_path;
 }
diff --git a/gcc/tree-diagnostic.h b/gcc/tree-diagnostic.h
index d97a97380245..e9a119273a0c 100644
--- a/gcc/tree-diagnostic.h
+++ b/gcc/tree-diagnostic.h
@@ -57,8 +57,6 @@  void tree_diagnostics_defaults (diagnostic_context *context);
 bool default_tree_printer (pretty_printer *, text_info *, const char *,
 			   int, bool, bool, bool, bool *, const char **);
 
-extern void default_tree_diagnostic_path_printer (diagnostic_context *,
-						  const diagnostic_path *);
 extern json::value *default_tree_make_json_for_path (diagnostic_context *,
 						     const diagnostic_path *);
 
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index eaddcb64bc67..229806fd4984 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -1732,6 +1732,8 @@  class rich_location
   const diagnostic_path *get_path () const { return m_path; }
   void set_path (const diagnostic_path *path) { m_path = path; }
 
+  bool path_makes_location_redundant_p () const;
+
 private:
   bool reject_impossible_fixit (location_t where);
   void stop_supporting_fixits ();