From patchwork Fri Apr 3 21:53:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39076 From: gprocida@google.com (Giuliano Procida) Date: Fri, 3 Apr 2020 22:53:54 +0100 Subject: [PATCH 1/2] abidiff: More compact references to prior diffs. Message-ID: <20200403215356.186742-1-gprocida@google.com> 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. * 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 Reviewed-by: Matthias Maennich --- src/abg-reporter-priv.cc | 19 ++++++++----- 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, 45 insertions(+), 42 deletions(-) diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc index 883c815e..323de503 100644 --- a/src/abg-reporter-priv.cc +++ b/src/abg-reporter-priv.cc @@ -507,11 +507,11 @@ represent(const var_diff_sptr &diff, 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 { @@ -527,13 +527,20 @@ 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; 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 >::_List_impl' to 'std::__cxx11::_List_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 >::_List_impl::_M_node' changed: - details were reported earlier + type of 'std::__detail::_List_node_base std::_List_base >::_List_impl::_M_node' changed, as reported earlier and name of 'std::_List_base >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base >::_List_impl::_M_node' and name of 'std::_List_base >::_M_impl' changed to 'std::__cxx11::_List_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: