Hello,
There are two kinds of data member changes:
1/ changes to the type or offset of a given data member
basically, in this kind of change, the name of the data member
remained the same.
2/ changes where the data member (at a given offset) was replaced by
something else completely.
Today, the comparison engine recognizes these two kinds of changes and
records them in two different data structures. This is useful because
it allows for a finer grain analysis.
But when we report these changes, today, we report them separately and
sometimes that doesn't make sense. For instance, because there is no
change of the 1/ kind, the reporter would say "no data member
changes", and yet go ahead and emit data member changes of the 2/ kind.
This patch groups the reporting of the two kinds of changes so that
when it says "no data member changes", it means there was no data
member changes at all.
* include/abg-comparison.h
(class_or_union_diff::{sorted_changed_data_members,
count_filtered_changed_data_members,
sorted_subtype_changed_data_members,
count_filtered_subtype_changed_data_members}): Declare ...
* src/abg-comparison.cc
(class_or_union_diff::{sorted_changed_data_members,
count_filtered_changed_data_members,
sorted_subtype_changed_data_members,
count_filtered_subtype_changed_data_members}): ... accessors for
existing private data members.
* src/abg-default-reporter.cc (default_reporter::report): In the
class_or_union_diff& overload, group the reporting of the changes
to data member sub-types with the replacement of data members.
These are just data member changes after all. Use the newly
declared accessors for better measure.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Likewise.
* tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt: Adjust.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Likewise.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to master.
---
include/abg-comparison.h | 12 +++++
src/abg-comparison.cc | 30 ++++++++++++
src/abg-default-reporter.cc | 51 ++++++++------------
src/abg-leaf-reporter.cc | 55 ++++++----------------
.../test45-anon-dm-change-report-0.txt | 5 +-
...-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt | 9 ++--
6 files changed, 82 insertions(+), 80 deletions(-)
@@ -1598,6 +1598,18 @@ public:
const string_member_function_sptr_map&
inserted_member_fns() const;
+ const var_diff_sptrs_type&
+ sorted_changed_data_members() const;
+
+ size_t
+ count_filtered_changed_data_members(bool local_only = false) const;
+
+ const var_diff_sptrs_type&
+ sorted_subtype_changed_data_members() const;
+
+ size_t
+ count_filtered_subtype_changed_data_members(bool local_only = false) const;
+
const edit_script&
member_fn_tmpls_changes() const;
@@ -4955,6 +4955,36 @@ const string_member_function_sptr_map&
class_or_union_diff::inserted_member_fns() const
{return get_priv()->inserted_member_functions_;}
+/// Getter of the sorted vector of data members that got replaced by
+/// another data member.
+///
+/// @return sorted vector of changed data member.
+const var_diff_sptrs_type&
+class_or_union_diff::sorted_changed_data_members() const
+{return get_priv()->sorted_changed_dm_;}
+
+/// Count the number of /filtered/ data members that got replaced by
+/// another data member.
+///
+/// @return the number of changed data member that got filtered out.
+size_t
+class_or_union_diff::count_filtered_changed_data_members(bool local) const
+{return get_priv()->count_filtered_changed_dm(local);}
+
+/// Getter of the sorted vector of data members with a (sub-)type change.
+///
+/// @return sorted vector of changed data member.
+const var_diff_sptrs_type&
+class_or_union_diff::sorted_subtype_changed_data_members() const
+{return get_priv()->sorted_subtype_changed_dm_;}
+
+/// Count the number of /filtered/ data members with a sub-type change.
+///
+/// @return the number of changed data member that got filtered out.
+size_t
+class_or_union_diff::count_filtered_subtype_changed_data_members(bool local) const
+{return get_priv()->count_filtered_subtype_changed_dm(local);}
+
/// @return the edit script of the member function templates of the two
/// @ref class_or_union.
const edit_script&
@@ -989,43 +989,32 @@ default_reporter::report(const class_or_union_diff& d,
}
// report change
- size_t numchanges =
- d.class_or_union_diff::get_priv()->sorted_subtype_changed_dm_.size();
- size_t num_filtered =
- d.class_or_union_diff::get_priv()->count_filtered_subtype_changed_dm();
- if (numchanges)
+ size_t num_changes =
+ (d.sorted_subtype_changed_data_members().size()
+ + d.sorted_changed_data_members().size());
+
+ size_t num_changes_filtered =
+ (d.count_filtered_subtype_changed_data_members()
+ + d.count_filtered_changed_data_members());
+
+ if (num_changes)
{
- report_mem_header(out, numchanges, num_filtered,
- subtype_change_kind, "data member", indent);
+ report_mem_header(out, num_changes, num_changes_filtered,
+ change_kind, "data member", indent);
+
for (var_diff_sptrs_type::const_iterator it =
- d.class_or_union_diff::get_priv()->sorted_subtype_changed_dm_.begin();
- it != d.class_or_union_diff::get_priv()->sorted_subtype_changed_dm_.end();
+ d.sorted_changed_data_members().begin();
+ it != d.sorted_changed_data_members().end();
++it)
- {
- if ((*it)->to_be_reported())
- {
- represent(*it, ctxt, out, indent + " ");
- }
- }
- }
+ if ((*it)->to_be_reported())
+ represent(*it, ctxt, out, indent + " ");
- numchanges = d.class_or_union_diff::get_priv()->sorted_changed_dm_.size();
- num_filtered =
- d.class_or_union_diff::get_priv()->count_filtered_changed_dm();
- if (numchanges)
- {
- report_mem_header(out, numchanges, num_filtered,
- change_kind, "data member", indent);
for (var_diff_sptrs_type::const_iterator it =
- d.class_or_union_diff::get_priv()->sorted_changed_dm_.begin();
- it != d.class_or_union_diff::get_priv()->sorted_changed_dm_.end();
+ d.sorted_subtype_changed_data_members().begin();
+ it != d.sorted_subtype_changed_data_members().end();
++it)
- {
- if ((*it)->to_be_reported())
- {
- represent(*it, ctxt, out, indent + " ");
- }
- }
+ if ((*it)->to_be_reported())
+ represent(*it, ctxt, out, indent + " ");
}
}
@@ -603,58 +603,33 @@ leaf_reporter::report(const class_or_union_diff& d,
}
// report changes
- size_t numchanges =
- d.class_or_union_diff::get_priv()->sorted_changed_dm_.size();
+ size_t numchanges = (d.sorted_changed_data_members().size()
+ + d.sorted_subtype_changed_data_members().size());
+
size_t num_filtered =
- d.class_or_union_diff::get_priv()->
- count_filtered_changed_dm(/*local_only =*/ true);
+ (d.count_filtered_changed_data_members(/*local_only=*/true)
+ + d.count_filtered_subtype_changed_data_members(/*local_only=*/true));
+
ABG_ASSERT(numchanges >= num_filtered);
size_t net_numchanges = numchanges - num_filtered;
- bool emitted_data_members_changes = false;
if (net_numchanges)
{
report_mem_header(out, change_kind, "data member", indent);
+
for (var_diff_sptrs_type::const_iterator it =
- d.class_or_union_diff::get_priv()->
- sorted_changed_dm_.begin();
- it != d.class_or_union_diff::get_priv()->
- sorted_changed_dm_.end();
+ d.sorted_changed_data_members().begin();
+ it != d.sorted_changed_data_members().end();
++it)
- {
- if (diff_to_be_reported((*it).get()))
- {
- represent(*it, ctxt, out, indent + " ",
- /*local_only=*/true);
- emitted_data_members_changes = true;
- }
- }
- }
+ if (diff_to_be_reported((*it).get()))
+ represent(*it, ctxt, out, indent + " ", /*local_only=*/true);
- numchanges =
- d.class_or_union_diff::get_priv()->sorted_subtype_changed_dm_.size();
- num_filtered =
- d.class_or_union_diff::get_priv()->
- count_filtered_subtype_changed_dm(/*local_only =*/ true);
- ABG_ASSERT(numchanges >= num_filtered);
- net_numchanges = numchanges - num_filtered;
-
- if (net_numchanges)
- {
- if (!emitted_data_members_changes)
- report_mem_header(out, subtype_change_kind, "data member", indent);
for (var_diff_sptrs_type::const_iterator it =
- d.class_or_union_diff::get_priv()->sorted_subtype_changed_dm_.begin();
- it != d.class_or_union_diff::get_priv()->sorted_subtype_changed_dm_.end();
+ d.sorted_subtype_changed_data_members().begin();
+ it != d.sorted_subtype_changed_data_members().end();
++it)
- {
- if (diff_to_be_reported((*it).get()))
- {
- represent(*it, ctxt, out, indent + " ",
- /*local_only=*/true);
- emitted_data_members_changes = true;
- }
- }
+ if (diff_to_be_reported((*it).get()))
+ represent(*it, ctxt, out, indent + " ", /*local_only=*/true);
}
}
}
@@ -9,11 +9,10 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
type size changed from 96 to 64 (in bits)
1 data member insertion:
'char S1::m01', at offset 32 (in bits)
- 1 data member change:
- 'char S1::m1' offset changed from 64 to 40 (in bits) (by -24 bits)
- 1 data member change:
+ 2 data member changes:
anonymous data member struct {int m0; char m01;} at offset 0 (in bits) became data member 'int S1::m0'
and size changed from 64 to 32 (in bits) (by -32 bits)
+ 'char S1::m1' offset changed from 64 to 40 (in bits) (by -24 bits)
[C] 'function void foo(S0&)' has some indirect sub-type changes:
parameter 1 of type 'S0&' has sub-type changes:
@@ -159,11 +159,11 @@
in pointed to type 'typedef SpiceServer' at spice-server.h:38:1:
underlying type 'struct RedsState' at reds-private.h:127:1 changed:
type size hasn't changed
- 2 data member changes (1 filtered):
+ 2 data member changes (2 filtered):
type of 'VDIPortState RedsState::agent_state' changed:
underlying type 'struct VDIPortState' at reds-private.h:46:1 changed:
type size hasn't changed
- 1 data member change:
+ 1 data member changes (3 filtered):
type of 'SpiceCharDeviceState* VDIPortState::base' changed:
in pointed to type 'typedef SpiceCharDeviceState' at spice-char.h:34:1:
underlying type 'struct SpiceCharDeviceState' at char_device.c:47:1 changed:
@@ -199,7 +199,6 @@
pointed to type 'typedef SpiceCharDeviceInstance' changed at spice.h:399:1, as reported earlier
and offset changed from 1024 to 1088 (in bits) (by +64 bits)
'void* SpiceCharDeviceState::opaque' offset changed from 1472 to 1536 (in bits) (by +64 bits)
- no data member changes (3 filtered);
type of 'MainChannel* RedsState::main_channel' changed:
in pointed to type 'typedef MainChannel' at main_channel.h:48:1:
underlying type 'struct MainChannel' at main_channel.h:36:1 changed:
@@ -404,7 +403,6 @@
in pointed to type 'function type void (RedChannelClient*)':
parameter 1 of type 'RedChannelClient*' has sub-type changes:
pointed to type 'typedef RedChannelClient' changed at red_channel.h:136:1, as reported earlier
- no data member change (1 filtered);
[C] 'function int spice_server_add_interface(SpiceServer*, SpiceBaseInstance*)' at reds.c:3159:1 has some indirect sub-type changes:
parameter 1 of type 'SpiceServer*' has sub-type changes:
@@ -519,7 +517,7 @@
in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
underlying type 'struct SndChannel' at snd_worker.c:89:1 changed:
type size hasn't changed
- 6 data member changes:
+ 6 data member changes (1 filtered):
type of 'RedsStream* SndChannel::stream' changed:
pointed to type 'typedef RedsStream' changed at red_channel.h:134:1, as reported earlier
type of 'SndWorker* SndChannel::worker' changed:
@@ -545,7 +543,6 @@
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, 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, as being reported