]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
diagnostic_path: avoid printing redundant data
authorDavid Malcolm <dmalcolm@redhat.com>
Tue, 24 Dec 2019 01:02:54 +0000 (20:02 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Wed, 15 Jan 2020 02:10:45 +0000 (21:10 -0500)
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.

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.

18 files changed:
gcc/diagnostic-format-json.cc
gcc/diagnostic-show-locus.c
gcc/diagnostic.c
gcc/diagnostic.h
gcc/doc/invoke.texi
gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-default.c
gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-1.c
gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-2.c
gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-inline-events-3.c
gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2.c
gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-2a.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
gcc/testsuite/gcc.dg/plugin/plugin.exp
gcc/tree-diagnostic-path.cc
gcc/tree-diagnostic.c
gcc/tree-diagnostic.h
libcpp/include/line-map.h

index 7bda5c4ba8344360ff02f56874a5f45ff1e6a6a0..e3dc7f1a566651f32c0c0e20e93f931fe5557272 100644 (file)
@@ -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;
index 4618b4edb7d19846e2ce74ad07fc26212d66e761..4c4af80a8d2882a5eb980620f3dde7d6de998893 100644 (file)
@@ -2565,6 +2565,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++)
index 72afd7c6adf08fcce462e851daafce69b935f17f..5b817ec7eca99b73890a8014399b4b2d297f6512 100644 (file)
@@ -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;
@@ -286,6 +288,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;
@@ -672,8 +677,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
@@ -693,6 +698,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)
index 307dbcfb34a6f8257ce7d6b51903e75bff0bf131..afb02658fbc3e96cdf0ee23c36818a0d5501e98d 100644 (file)
@@ -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;
index d8b94d837a968954b985350da396ea6d4a477e25..13742042de298fea871943c22c4767cef79853b9 100644 (file)
@@ -4209,27 +4209,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,
@@ -4283,13 +4280,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.
index 5712dbd647250850bf6aa81d916b6370ed81961f..d0ad8d6b987d75a865a972375fb2360e53c4332c 100644 (file)
@@ -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
     |
     | {
index 430d81737718b070280684c10faddbd437f55016..5fc005a767b3ca11de65a033cd56d51bc783909b 100644 (file)
@@ -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
     |
     | {
index c2bfabec9101216669d7e893eb444f2c9e5bfde6..7072308185cdc241b9816f25018658c661ebe11d 100644 (file)
@@ -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
     |
     | {
index 386cac91c41575fa708538fce8de6d5be1acf299..7e619da6ec77a20820bb0e06e5bd6ff748a961e8 100644 (file)
@@ -17,8 +17,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 | {
index 946a234dd236cbfd1be1aa843b235c20d9835b92..29f0a848f52ecd3457ef3a5ee19bfc217b3b1465 100644 (file)
@@ -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 (file)
index 0000000..e8dc5bf
--- /dev/null
@@ -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 "" } */
+}
index 847b6d423e40f032449e8ccbb91fe4deb09fb8e5..2b5c395940992045f09e37c9e910870aac054728 100644 (file)
@@ -30,8 +30,6 @@ void test (void)
 }
 
 /* { dg-begin-multiline-output "" }
-   NN |   fprintf(stderr, "LOG: %s", msg);
-      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   'test': events 1-2
     |
     |   NN | {
index cf05ca3a5d325e6bbf15f678a7f24478b8f963d1..fa075057d504e98b072868ff1d65194bf4b183f1 100644 (file)
@@ -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;
index c02b008271528262600bff74dbd08d3db74dfb08..99e3c3d531e31f7d2b89b07e76e2d37be6fa36eb 100644 (file)
@@ -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 \
index 381a49cb0b4746e79e3b5395fd3788708e3ea455..381ec77bc346cd72714e4f65a8b5f0dff9e9aa92 100644 (file)
@@ -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.  */
index 8422714aecbc2de2dafad3f8776d4bfc8ccb26e3..d1768367d4668f5165e701ed06188cdcfc72338f 100644 (file)
@@ -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;
 }
index 40dc9fa0e8319600feb0e7e833ecd688b5d28574..9f3175ea423cd561c7fdc6d70df8c836fcbf7ae0 100644 (file)
@@ -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 *);
 
index dbbc13762e39dbcc9206eda5fe9dc27f18cf7145..8691cbd13024e8903c70e5a1c5360f3d063d9907 100644 (file)
@@ -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 ();