diff mbox series

[v2,1/2] abidiff: More compact references to prior diffs.

Message ID 20200407213314.248613-1-gprocida@google.com
State Committed
Headers
Series [v2,1/2] abidiff: More compact references to prior diffs. |

Commit Message

Giuliano Procida April 7, 2020, 9:33 p.m. UTC 
  In both the default and leaf reporting modes, when there is a repeated
or circular reference to a type difference of a member variable, some
placeholder text is emitted instead.

In the leaf reporter:

  type 'struct S' of 'foo::bar' changed as reported earlier

In the default reporter, this spans two lines:

  type of 'S foo::bar' changed:
    details were reported earlier

This patch changes the latter to the more compact:

  type of 'S foo::bar' changed, as reported earlier

More generally, this patch makes the punctuation of such placeholder
text more consistent with a comma separating the phrases in all cases.

It doesn't attempt to reconcile the different formatting of member
variable declarations between the two modes.

	* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
	overload, use consistent punctuation and keep to a single line
	of output when referring back to an existing type diff report.
	Remove unnecessary braces around single line conditional
	blocks.
	* src/abg-reporter-priv.h: In the macro
	RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER, use
	consistent punctuation when referring back to an existing type
	diff report.
	* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust
	formatting of back references to existing type diff reports.
	* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
	Ditto.
	* tests/data/test-diff-filter/test16-report-2.txt: Ditto.
	* tests/data/test-diff-filter/test17-1-report.txt: Ditto.
	* tests/data/test-diff-filter/test25-cyclic-type-report-1.txt:
	Ditto.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
	Ditto.
	* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-reporter-priv.cc                      | 25 ++++++++---------
 src/abg-reporter-priv.h                       |  8 +++---
 .../test-abidiff/test-PR18791-report0.txt     |  3 +--
 .../PR25058-liblttng-ctl-report-1.txt         | 22 +++++++--------
 .../data/test-diff-filter/test16-report-2.txt |  2 +-
 .../data/test-diff-filter/test17-1-report.txt |  2 +-
 .../test25-cyclic-type-report-1.txt           |  2 +-
 ...l7.x86_64-0.12.8-1.el7.x86_64-report-2.txt | 27 +++++++++----------
 .../test-diff-suppr/test36-leaf-report-0.txt  |  2 +-
 9 files changed, 43 insertions(+), 50 deletions(-)

Comments

Dodji Seketeli April 14, 2020, 10:42 a.m. UTC | #1 
Hello Giuliano,

Giuliano Procida <gprocida@google.com> a ?crit:

[...]

>
> 	* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
> 	overload, use consistent punctuation and keep to a single line
> 	of output when referring back to an existing type diff report.
> 	Remove unnecessary braces around single line conditional
> 	blocks.
> 	* src/abg-reporter-priv.h: In the macro
> 	RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER, use
> 	consistent punctuation when referring back to an existing type
> 	diff report.
> 	* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust
> 	formatting of back references to existing type diff reports.
> 	* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
> 	Ditto.
> 	* tests/data/test-diff-filter/test16-report-2.txt: Ditto.
> 	* tests/data/test-diff-filter/test17-1-report.txt: Ditto.
> 	* tests/data/test-diff-filter/test25-cyclic-type-report-1.txt:
> 	Ditto.
> 	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
> 	Ditto.
> 	* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Ditto.

Applied to master, thanks!

Cheers,
diff mbox series

Patch

diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
index 883c815e..eb7df341 100644
--- a/src/abg-reporter-priv.cc
+++ b/src/abg-reporter-priv.cc
@@ -506,13 +506,9 @@  represent(const var_diff_sptr	&diff,
 	      << "' changed";
 
 	  if (d->currently_reporting())
-	    {
-	      out << " as being reported\n";
-	    }
+	    out << ", as being reported\n";
 	  else if (d->reported_once())
-	    {
-	      out << " as reported earlier\n";
-	    }
+	    out << ", as reported earlier\n";
 	  else
 	    {
 	      out << ":\n";
@@ -527,13 +523,16 @@  represent(const var_diff_sptr	&diff,
 	  if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
 	    {
 	      out << indent
-		  << "type of '" << pretty_representation << "' changed:\n";
+		  << "type of '" << pretty_representation << "' changed";
 	      if (d->currently_reporting())
-		out << indent << "  details are being reported\n";
+		out << ", as being reported\n";
 	      else if (d->reported_once())
-		out << indent << "  details were reported earlier\n";
+		out << ", as reported earlier\n";
 	      else
-		d->report(out, indent + "  ");
+		{
+		  out << ":\n";
+		  d->report(out, indent + "  ");
+		}
 
 	      begin_with_and = true;
 	      emitted = true;
@@ -715,10 +714,8 @@  represent(const var_diff_sptr	&diff,
     out << indent << "'" << pretty_representation
 	<< "' has *some* difference - please report as a bug";
   else
-    {
-      // do nothing
-      ;
-    }
+    // do nothing
+    ;
   emitted = true;
 
   if (!begin_with_and)
diff --git a/src/abg-reporter-priv.h b/src/abg-reporter-priv.h
index b6c56fdb..b221fdc3 100644
--- a/src/abg-reporter-priv.h
+++ b/src/abg-reporter-priv.h
@@ -68,14 +68,14 @@ 
 	{								\
 	  string _name_ = _diff_->first_subject()->get_pretty_representation(); \
 	  if (_diff_->currently_reporting())				\
-	    out << indent << INTRO_TEXT << " '" << _name_ << "' changed; " \
-	      "details are being reported\n";				\
+	    out << indent << INTRO_TEXT << " '" << _name_		\
+		<< "' changed, as being reported\n";			\
 	  else								\
 	    {								\
 	      out << indent << INTRO_TEXT << " '"			\
-	      << _name_ << "' changed";				\
+	      << _name_ << "' changed";					\
 	      report_loc_info(D->first_subject(), *d.context(), out);	\
-	      out << ", as reported earlier\n";			\
+	      out << ", as reported earlier\n";				\
 	    }								\
 	  return ;							\
 	}								\
diff --git a/tests/data/test-abidiff/test-PR18791-report0.txt b/tests/data/test-abidiff/test-PR18791-report0.txt
index 93faf599..5bc2d08b 100644
--- a/tests/data/test-abidiff/test-PR18791-report0.txt
+++ b/tests/data/test-abidiff/test-PR18791-report0.txt
@@ -123,8 +123,7 @@  Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
                     type name changed from 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl' to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl'
                     type size changed from 128 to 192 (in bits)
                     1 data member change:
-                      type of 'std::__detail::_List_node_base std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed:
-                        details were reported earlier
+                      type of 'std::__detail::_List_node_base std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed, as reported earlier
                       and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node'
                   and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl'
 
diff --git a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
index dfb63684..44ddc6d6 100644
--- a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
+++ b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
@@ -179,8 +179,7 @@  Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
         in pointed to type 'struct lttng_event_field':
           type size hasn't changed
           1 data member change:
-            type of 'lttng_event lttng_event_field::event' changed:
-              details were reported earlier
+            type of 'lttng_event lttng_event_field::event' changed, as reported earlier
 
   [C] 'function int lttng_list_tracepoints(lttng_handle*, lttng_event**)' has some indirect sub-type changes:
     parameter 2 of type 'lttng_event**' has sub-type changes:
@@ -232,7 +231,7 @@  Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
                   type size changed from 576 to 640 (in bits)
                   2 data member changes:
                     type of 'filter_node* filter_node::parent' changed:
-                      pointed to type 'struct filter_node' changed; details are being reported
+                      pointed to type 'struct filter_node' changed, as being reported
                     type of 'union {struct {} unknown; struct {filter_node* child;} root; struct {__anonymous_enum__ type; ast_link_type post_op; ast_link_type pre_op; union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;} u; filter_node* prev; filter_node* next;} expression; struct {op_type type; filter_node* lchild; filter_node* rchild;} op; struct {unary_op_type type; filter_node* child;} unary_op;} filter_node::u' changed:
                       type size changed from 320 to 384 (in bits)
                       4 data member changes:
@@ -245,21 +244,20 @@  Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
                               type size hasn't changed
                               1 enumerator insertion:
                                 'ast_link_type::AST_LINK_BRACKET' value '3'
-                            type of 'ast_link_type pre_op' changed:
-                              details were reported earlier
+                            type of 'ast_link_type pre_op' changed, as reported earlier
                             type of 'union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;} u' changed:
                               type size hasn't changed
                               1 data member change:
                                 type of 'filter_node* child' changed:
-                                  pointed to type 'struct filter_node' changed; details are being reported
+                                  pointed to type 'struct filter_node' changed, as being reported
                               type changed from:
                                 union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;}
                               to:
                                 union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;}
                             type of 'filter_node* prev' changed:
-                              pointed to type 'struct filter_node' changed; details are being reported
+                              pointed to type 'struct filter_node' changed, as being reported
                             type of 'filter_node* next' changed:
-                              pointed to type 'struct filter_node' changed; details are being reported
+                              pointed to type 'struct filter_node' changed, as being reported
                         type of 'struct {op_type type; filter_node* lchild; filter_node* rchild;} op' changed:
                           type size hasn't changed
                           3 data member changes:
@@ -278,14 +276,14 @@  Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
                                 'op_type::AST_OP_BIT_OR' value '11'
                                 'op_type::AST_OP_BIT_XOR' value '12'
                             type of 'filter_node* lchild' changed:
-                              pointed to type 'struct filter_node' changed; details are being reported
+                              pointed to type 'struct filter_node' changed, as being reported
                             type of 'filter_node* rchild' changed:
-                              pointed to type 'struct filter_node' changed; details are being reported
+                              pointed to type 'struct filter_node' changed, as being reported
                         type of 'struct {filter_node* child;} root' changed:
                           type size hasn't changed
                           1 data member change:
                             type of 'filter_node* child' changed:
-                              pointed to type 'struct filter_node' changed; details are being reported
+                              pointed to type 'struct filter_node' changed, as being reported
                         type of 'struct {unary_op_type type; filter_node* child;} unary_op' changed:
                           type size hasn't changed
                           2 data member changes:
@@ -296,7 +294,7 @@  Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
                               1 enumerator insertion:
                                 'unary_op_type::AST_UNARY_BIT_NOT' value '4'
                             type of 'filter_node* child' changed:
-                              pointed to type 'struct filter_node' changed; details are being reported
+                              pointed to type 'struct filter_node' changed, as being reported
                 'cds_list_head filter_ast::allocated_nodes' offset changed from 576 to 640 (in bits) (by +64 bits)
 
   [C] 'function YYSTYPE* lttng_yyget_lval(yyscan_t)' has some indirect sub-type changes:
diff --git a/tests/data/test-diff-filter/test16-report-2.txt b/tests/data/test-diff-filter/test16-report-2.txt
index a90e85ad..f69eabdc 100644
--- a/tests/data/test-diff-filter/test16-report-2.txt
+++ b/tests/data/test-diff-filter/test16-report-2.txt
@@ -11,6 +11,6 @@  Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
           'int S::m0', at offset 0 (in bits)
         1 data member change:
           type of 'S* S::m2' changed:
-            pointed to type 'struct S' changed; details are being reported
+            pointed to type 'struct S' changed, as being reported
           and offset changed from 0 to 64 (in bits) (by +64 bits)
 
diff --git a/tests/data/test-diff-filter/test17-1-report.txt b/tests/data/test-diff-filter/test17-1-report.txt
index 7c51152a..0b39c5d4 100644
--- a/tests/data/test-diff-filter/test17-1-report.txt
+++ b/tests/data/test-diff-filter/test17-1-report.txt
@@ -11,7 +11,7 @@  Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
           'int S::m0', at offset 0 (in bits)
         1 data member change:
           type of 'S* S::m2' changed:
-            pointed to type 'struct S' changed; details are being reported
+            pointed to type 'struct S' changed, as being reported
           and offset changed from 0 to 64 (in bits) (by +64 bits)
 
   [C] 'function void foo(S&)' has some indirect sub-type changes:
diff --git a/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt b/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
index 6a6beeef..0215d892 100644
--- a/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
+++ b/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
@@ -11,5 +11,5 @@  Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
           'char S::m1', at offset 32 (in bits)
         1 data member change:
           type of 'S* S::m2' changed:
-            pointed to type 'struct S' changed; details are being reported
+            pointed to type 'struct S' changed, as being reported
 
diff --git a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
index 6d9beb85..58c94b7a 100644
--- a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
+++ b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
@@ -37,7 +37,7 @@ 
                             13 data member changes:
                               type of 'QXLInstance* RedDispatcher::qxl' changed:
                                 in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1:
-                                  underlying type 'struct QXLInstance' changed; details are being reported
+                                  underlying type 'struct QXLInstance' changed, as being reported
                               type of 'Dispatcher RedDispatcher::dispatcher' changed:
                                 underlying type 'struct Dispatcher' at dispatcher.h:22:1 changed:
                                   type size changed from 960 to 1024 (in bits)
@@ -51,7 +51,7 @@ 
                               'int RedDispatcher::use_hardware_cursor' offset changed from 2240 to 2304 (in bits) (by +64 bits)
                               type of 'RedDispatcher* RedDispatcher::next' changed:
                                 in pointed to type 'typedef RedDispatcher' at red_worker.h:87:1:
-                                  underlying type 'struct RedDispatcher' changed; details are being reported
+                                  underlying type 'struct RedDispatcher' changed, as being reported
                               and offset changed from 2304 to 2368 (in bits) (by +64 bits)
                               'Ring RedDispatcher::async_commands' offset changed from 2368 to 2432 (in bits) (by +64 bits)
                               'pthread_mutex_t RedDispatcher::async_lock' offset changed from 2496 to 2560 (in bits) (by +64 bits)
@@ -185,7 +185,7 @@ 
                                   1 data member change:
                                     type of 'SpiceCharDeviceState* SpiceCharDeviceInstance::st' changed:
                                       in pointed to type 'typedef SpiceCharDeviceState' at spice-char.h:34:1:
-                                        underlying type 'struct SpiceCharDeviceState' changed; details are being reported
+                                        underlying type 'struct SpiceCharDeviceState' changed, as being reported
                             and offset changed from 896 to 960 (in bits) (by +64 bits)
                             'int SpiceCharDeviceState::during_read_from_device' offset changed from 960 to 1024 (in bits) (by +64 bits)
                             'int SpiceCharDeviceState::during_write_to_device' offset changed from 992 to 1056 (in bits) (by +64 bits)
@@ -223,7 +223,7 @@ 
                                               2 data member changes (2 filtered):
                                                 type of 'RedChannel* RedChannelClient::channel' changed:
                                                   in pointed to type 'typedef RedChannel' at red_channel.h:130:1:
-                                                    underlying type 'struct RedChannel' changed; details are being reported
+                                                    underlying type 'struct RedChannel' changed, as being reported
                                                 type of 'RedsStream* RedChannelClient::stream' changed:
                                                   in pointed to type 'typedef RedsStream' at reds_stream.h:31:1:
                                                     underlying type 'struct RedsStream' at reds.h:68:1 changed:
@@ -391,7 +391,7 @@ 
                                       in pointed to type 'function type void (RedChannel*, RedClient*, RedsStream*, int, int, uint32_t*, int, uint32_t*)':
                                         parameter 1 of type 'RedChannel*' has sub-type changes:
                                           in pointed to type 'typedef RedChannel' at red_channel.h:130:1:
-                                            underlying type 'struct RedChannel' changed; details are being reported
+                                            underlying type 'struct RedChannel' changed, as being reported
                                         parameter 3 of type 'RedsStream*' has sub-type changes:
                                           pointed to type 'typedef RedsStream' changed at red_channel.h:134:1, as reported earlier
                                   type of 'channel_client_disconnect_proc disconnect' changed:
@@ -524,7 +524,7 @@ 
                                     pointed to type 'typedef RedsStream' changed at red_channel.h:134:1, as reported earlier
                                   type of 'SndWorker* SndChannel::worker' changed:
                                     in pointed to type 'typedef SndWorker' at snd_worker.c:80:1:
-                                      underlying type 'struct SndWorker' changed; details are being reported
+                                      underlying type 'struct SndWorker' changed, as being reported
                                   type of 'RedChannelClient* SndChannel::channel_client' changed:
                                     pointed to type 'typedef RedChannelClient' changed at red_channel.h:136:1, as reported earlier
                                   type of 'snd_channel_handle_message_proc SndChannel::handle_message' changed:
@@ -532,26 +532,26 @@ 
                                       in pointed to type 'function type int (SndChannel*, typedef size_t, typedef uint32_t, void*)':
                                         parameter 1 of type 'SndChannel*' has sub-type changes:
                                           in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
-                                            underlying type 'struct SndChannel' changed; details are being reported
+                                            underlying type 'struct SndChannel' changed, as being reported
                                   type of 'snd_channel_on_message_done_proc SndChannel::on_message_done' changed:
                                     underlying type 'void (SndChannel*)*' changed:
                                       in pointed to type 'function type void (SndChannel*)':
                                         parameter 1 of type 'SndChannel*' has sub-type changes:
                                           in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
-                                            underlying type 'struct SndChannel' changed; details are being reported
+                                            underlying type 'struct SndChannel' changed, as being reported
                                   type of 'snd_channel_cleanup_channel_proc SndChannel::cleanup' changed:
                                     underlying type 'void (SndChannel*)*' changed:
                                       in pointed to type 'function type void (SndChannel*)':
                                         parameter 1 of type 'SndChannel*' has sub-type changes:
                                           in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
-                                            underlying type 'struct SndChannel' changed; details are being reported
+                                            underlying type 'struct SndChannel' changed, as being reported
                                 no data member change (1 filtered);
                           type of 'SndWorker* SndWorker::next' changed:
                             in pointed to type 'typedef SndWorker' at snd_worker.c:80:1:
-                              underlying type 'struct SndWorker' changed; details are being reported
+                              underlying type 'struct SndWorker' changed, as being reported
                       type of 'SpicePlaybackInstance* SpicePlaybackState::sin' changed:
                         in pointed to type 'typedef SpicePlaybackInstance' at spice-audio.h:33:1:
-                          underlying type 'struct SpicePlaybackInstance' changed; details are being reported
+                          underlying type 'struct SpicePlaybackInstance' changed, as being reported
 
     [C] 'function void spice_server_playback_put_samples(SpicePlaybackInstance*, uint32_t*)' at snd_worker.c:1100:1 has some indirect sub-type changes:
       parameter 1 of type 'SpicePlaybackInstance*' has sub-type changes:
@@ -590,11 +590,10 @@ 
                     1 data member insertion:
                       'uint32_t SpiceRecordState::frequency', at offset 512 (in bits) at snd_worker.c:166:1
                     2 data member changes:
-                      type of 'SndWorker SpiceRecordState::worker' changed:
-                        details were reported earlier
+                      type of 'SndWorker SpiceRecordState::worker' changed, as reported earlier
                       type of 'SpiceRecordInstance* SpiceRecordState::sin' changed:
                         in pointed to type 'typedef SpiceRecordInstance' at spice-audio.h:67:1:
-                          underlying type 'struct SpiceRecordInstance' changed; details are being reported
+                          underlying type 'struct SpiceRecordInstance' changed, as being reported
 
     [C] 'function void spice_server_record_set_mute(SpiceRecordInstance*, uint8_t)' at snd_worker.c:1279:1 has some indirect sub-type changes:
       parameter 1 of type 'SpiceRecordInstance*' has sub-type changes:
diff --git a/tests/data/test-diff-suppr/test36-leaf-report-0.txt b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
index d270740d..9caa9428 100644
--- a/tests/data/test-diff-suppr/test36-leaf-report-0.txt
+++ b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
@@ -15,7 +15,7 @@  Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
 'struct leaf2 at test36-leaf-v0.cc:9:1' changed:
   type size changed from 64 to 96 (in bits)
   there are data member changes:
-    type 'struct leaf1' of 'leaf2::member0' changed as reported earlier
+    type 'struct leaf1' of 'leaf2::member0' changed, as reported earlier
     and size changed from 32 to 64 (in bits) (by +32 bits)
     'char leaf2::member1' offset changed from 32 to 64 (in bits) (by +32 bits)
   3 impacted interfaces: