[4/7] abidiff: Remove member function diff blank lines.

Message ID 20200327140017.1917-5-gprocida@google.com
State Committed
Headers
Series Mostly whitespace changes and fixes. |

Commit Message

Giuliano Procida March 27, 2020, 2 p.m. UTC
  Currently, lists of member function deletions, additions and changes
are each followed by a blank line in abidiff output.

While this formatting works reasonably well in leaf-changes-only mode
for top-level changes to types, it is inconsistent with everthing
else. In particular, when reporting member function diffs in default
mode, or more deeply nested in leaf mode, the blank lines are
jarring.

There was also no test coverage for leaf-changes-only mode.

This change removes these blank lines and associated state variables
and logic. It adds a test case for leaf-changes-only mode.

Note that the patch also modifies default mode reporting of member
types, template member functions and template member classes
analogously, but I wasn't able to exercise those code paths.

	* src/abg-default-reporter.cc (report): In the
	class_or_union_diff overload, don't emit a new line after each
	list of member function, member type, template member
	function and template member class changes.
	* src/abg-leaf-reporter.cc (report): : In the
	class_or_union_diff overload, don't emit a new line after each
	list of member function changes.
	* tests/data/Makefile.am: Add new test case files.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.cc:
	New test case for --leaf-changes-only member function diffs.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.o:
	Ditto.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc:
	Ditto. Also add a TODO regarding a potentially bad diff.
	* tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.o:
	Ditto.
	* tests/data/test-abidiff/test-struct1-report.txt: Remove
	blank lines after member function changes lists.
	* tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt:
	Ditto.
	* tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt:
	Ditto.
	* tests/test-abidiff-exit.cc: Add new test case.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-default-reporter.cc                   |  63 +++---------------
 src/abg-leaf-reporter.cc                      |  21 ------
 tests/data/Makefile.am                        |   5 ++
 .../test-leaf-cxx-members-report.txt          |  42 ++++++++++++
 .../test-leaf-cxx-members-v0.cc               |  12 ++++
 .../test-leaf-cxx-members-v0.o                | Bin 0 -> 7328 bytes
 .../test-leaf-cxx-members-v1.cc               |  14 ++++
 .../test-leaf-cxx-members-v1.o                | Bin 0 -> 7360 bytes
 .../data/test-abidiff/test-struct1-report.txt |   1 -
 .../test41-PR20476-hidden-report-0.txt        |   2 -
 ...bb-4.3-3.20141204.fc23.x86_64-report-0.txt |   2 -
 tests/test-abidiff-exit.cc                    |  10 +++
 12 files changed, 91 insertions(+), 81 deletions(-)
 create mode 100644 tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
 create mode 100644 tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.cc
 create mode 100644 tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.o
 create mode 100644 tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc
 create mode 100644 tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.o
  

Comments

Dodji Seketeli March 30, 2020, 2:13 p.m. UTC | #1
Giuliano Procida <gprocida@google.com> a ?crit:

> Currently, lists of member function deletions, additions and changes
> are each followed by a blank line in abidiff output.
>
> While this formatting works reasonably well in leaf-changes-only mode
> for top-level changes to types, it is inconsistent with everthing
> else. In particular, when reporting member function diffs in default
> mode, or more deeply nested in leaf mode, the blank lines are
> jarring.
>
> There was also no test coverage for leaf-changes-only mode.
>
> This change removes these blank lines and associated state variables
> and logic. It adds a test case for leaf-changes-only mode.
>
> Note that the patch also modifies default mode reporting of member
> types, template member functions and template member classes
> analogously, but I wasn't able to exercise those code paths.

The template handling code can be ignored for now.  I initially started
to work on it but it needed huge adaptations on the compiler side to
have it emit debug info for non-instantiated templates.  I eventually
gave up on doing that because of more pressing matters.  So until we
have a compiler that emits debug information for non instantiated
templates, I don't think that code will be exercised.

[...]


> diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc
> new file mode 100644
> index 00000000..0437584e
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc
> @@ -0,0 +1,14 @@
> +struct ops {
> +  // TODO: type, name and size are differnent from deleted_var, but this is
> +  // still reported as a change rather than a deletion and an addition.
> +  long added_var;
> +  virtual long added_fn() { return 0; }

This is by design.  If a member variable S::foo_member at a given
offset/ is replaced by another variable S::bar_member (foo_member and
bar_member have the same offset), we chose to detect that foo_member was
renamed into bar_member.

So I think we can remove that TODO above.

Would you agree?

Cheers,
  
Giuliano Procida March 30, 2020, 2:50 p.m. UTC | #2
Hi.

On Mon, 30 Mar 2020 at 15:13, Dodji Seketeli <dodji@seketeli.org> wrote:
>
> Giuliano Procida <gprocida@google.com> a ?crit:
>
> > Currently, lists of member function deletions, additions and changes
> > are each followed by a blank line in abidiff output.
> >
> > While this formatting works reasonably well in leaf-changes-only mode
> > for top-level changes to types, it is inconsistent with everthing
> > else. In particular, when reporting member function diffs in default
> > mode, or more deeply nested in leaf mode, the blank lines are
> > jarring.
> >
> > There was also no test coverage for leaf-changes-only mode.
> >
> > This change removes these blank lines and associated state variables
> > and logic. It adds a test case for leaf-changes-only mode.
> >
> > Note that the patch also modifies default mode reporting of member
> > types, template member functions and template member classes
> > analogously, but I wasn't able to exercise those code paths.
>
> The template handling code can be ignored for now.  I initially started
> to work on it but it needed huge adaptations on the compiler side to
> have it emit debug info for non-instantiated templates.  I eventually
> gave up on doing that because of more pressing matters.  So until we
> have a compiler that emits debug information for non instantiated
> templates, I don't think that code will be exercised.

Good to know. It's worth adding some comments to the code to this
effect, so I'll do this.

> [...]
>
>
> > diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc
> > new file mode 100644
> > index 00000000..0437584e
> > --- /dev/null
> > +++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc
> > @@ -0,0 +1,14 @@
> > +struct ops {
> > +  // TODO: type, name and size are differnent from deleted_var, but this is
> > +  // still reported as a change rather than a deletion and an addition.
> > +  long added_var;
> > +  virtual long added_fn() { return 0; }
>
> This is by design.  If a member variable S::foo_member at a given
> offset/ is replaced by another variable S::bar_member (foo_member and
> bar_member have the same offset), we chose to detect that foo_member was
> renamed into bar_member.
>
> So I think we can remove that TODO above.
>
> Would you agree?

I think the TODO has served its purpose which was to attract attention
(now or in the future). I'll remove it.

As to whether I agree... It was a little surprising. I think in most
large structs, if there are changes, the thing I'd mostly to discount
is offset, or perhaps name, next size (for arrays and perhaps integer
types on little endian architectures - but that is very risky) and
finally types. This doesn't really answer the question.

I think my mental model of comparing structs would be: list all the
names in offset order, compute a minimal edit based on names. Pair up
names that have been both removed and added and reclassify them as
moves (note that there may be more than one solution A,B -> B,A could
be A moving or B moving). Finally, perhaps, look at all (remaining)
possible matching removed/added pairs at the same position and see if
the type (including size) remains the same and reclassify these as
renames (this is also ambiguous and is more likely to be misleading
(bool flag1, bool flag2, -> bool flag3)).

If your structs correspond to network (wire) or hardware (DMA) data
structures, you're going to care about offsets an awful lot more.

Here are a couple more worth thinking about:

struct {
  char padding1[16];
  long foo;
  char padding2[8];
};

struct {
  char padding1[8];
  // did foo move and get renamed?
  long bar;
  char padding2[16];
};

and

struct {
  bool flag1;
  bool flag2;
};

struct {
  // flag1 deleted or flag1 renamed and flag2 deleted?
  bool flag2;
};

I'm happy for the code to be working as intended.

Regards.
Giuliano.

> Cheers,
>
> --
>                 Dodji
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
  
Dodji Seketeli March 30, 2020, 3:22 p.m. UTC | #3
Giuliano Procida <gprocida@google.com> a ?crit:

[...]


>> > +struct ops {
>> > +  // TODO: type, name and size are differnent from deleted_var, but this is
>> > +  // still reported as a change rather than a deletion and an addition.
>> > +  long added_var;
>> > +  virtual long added_fn() { return 0; }
>>
>> This is by design.  If a member variable S::foo_member at a given
>> offset/ is replaced by another variable S::bar_member (foo_member and
>> bar_member have the same offset), we chose to detect that foo_member was
>> renamed into bar_member.
>>
>> So I think we can remove that TODO above.
>>
>> Would you agree?

[...]

> I think the TODO has served its purpose which was to attract attention
> (now or in the future). I'll remove it.

Thanks.  I am doing it locally here, so you don't need to send another
patch just for that.

[...]

> Here are a couple more worth thinking about:
>
> struct {
>   char padding1[16];
>   long foo;
>   char padding2[8];
> };
>
> struct {
>   char padding1[8];
>   // did foo move and get renamed?
>   long bar;
>   char padding2[16];
> };

Here is what abidiff shows for this one (I named the struct 'S'):

$ ~/git/libabigail/master/build/tools/abidiff -l test-v0.o test-v1.o 
Leaf changes summary: 1 artifact changed
Changed leaf types summary: 1 leaf type changed
Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable

'struct S at test-v0.c:1:1' changed:
  type size hasn't changed
  1 data member deletion:
    'long int S::foo', at offset 128 (in bits) at test-v0.c:3:1
  1 data member insertion:
    'long int S::bar', at offset 64 (in bits) at test-v1.c:4:1
  there are data member changes:
    type 'char[16]' of 'S::padding1' changed:
      type name changed from 'char[16]' to 'char[8]'
      array type size changed from 128 to 64
      array type subrange 1 changed length from 16 to 8
    and size changed from 128 to 64 (in bits) (by -64 bits)
    type 'char[8]' of 'S::padding2' changed:
      type name changed from 'char[8]' to 'char[16]'
      array type size changed from 64 to 128
      array type subrange 1 changed length from 8 to 16
    and offset changed from 192 to 128 (in bits) (by -64 bits), size changed from 64 to 128 (in bits) (by +64 bits)

$ 

> and
>
> struct {
>   bool flag1;
>   bool flag2;
> };
>
> struct {
>   // flag1 deleted or flag1 renamed and flag2 deleted?
>   bool flag2;
> };

And for this one, here is what abidiff shows (I renamed the struct as S
and did s/bool/char/):

$ ~/git/libabigail/master/build/tools/abidiff -l test-v0.o test-v1.o 
Leaf changes summary: 1 artifact changed
Changed leaf types summary: 1 leaf type changed
Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable

'struct S at test-v0.c:1:1' changed:
  type size changed from 16 to 8 (in bits)
  1 data member deletion:
    'bool S::flag1', at offset 0 (in bits) at test-v0.c:2:1
  there are data member changes:
    'bool S::flag2' offset changed from 8 to 0 (in bits) (by -8 bits)

$ 

Hopefully these change reports should address your (valid!) concerns.

Thanks for looking into this!

Cheers,
  

Patch

diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index 1c39b3b2..2754da4b 100644
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -894,7 +894,6 @@  default_reporter::report(const class_or_union_diff& d,
       if (numdels)
 	report_mem_header(out, numdels, num_filtered, del_kind,
 			  "member function", indent);
-      bool emitted = false;
       for (string_member_function_sptr_map::const_iterator i =
 	     d.get_priv()->deleted_member_functions_.begin();
 	   i != d.get_priv()->deleted_member_functions_.end();
@@ -905,16 +904,10 @@  default_reporter::report(const class_or_union_diff& d,
 	      && !get_member_function_is_virtual(i->second))
 	    continue;
 
-	  if (emitted
-	      && i != d.get_priv()->deleted_member_functions_.begin())
-	    out << "\n";
 	  method_decl_sptr mem_fun = i->second;
 	  out << indent << "  ";
 	  represent(*ctxt, mem_fun, out);
-	  emitted = true;
 	}
-      if (emitted)
-	out << "\n";
 
       // report insertions;
       int numins = d.get_priv()->inserted_member_functions_.size();
@@ -922,7 +915,6 @@  default_reporter::report(const class_or_union_diff& d,
       if (numins)
 	report_mem_header(out, numins, num_filtered, ins_kind,
 			  "member function", indent);
-      emitted = false;
       for (string_member_function_sptr_map::const_iterator i =
 	     d.get_priv()->inserted_member_functions_.begin();
 	   i != d.get_priv()->inserted_member_functions_.end();
@@ -933,16 +925,10 @@  default_reporter::report(const class_or_union_diff& d,
 	      && !get_member_function_is_virtual(i->second))
 	    continue;
 
-	  if (emitted
-	      && i != d.get_priv()->inserted_member_functions_.begin())
-	    out << "\n";
 	  method_decl_sptr mem_fun = i->second;
 	  out << indent << "  ";
 	  represent(*ctxt, mem_fun, out);
-	  emitted = true;
 	}
-      if (emitted)
-	out << "\n";
 
       // report member function with sub-types changes
       int numchanges = d.get_priv()->sorted_changed_member_functions_.size();
@@ -950,7 +936,6 @@  default_reporter::report(const class_or_union_diff& d,
       if (numchanges)
 	report_mem_header(out, numchanges, num_filtered, change_kind,
 			  "member function", indent);
-      emitted = false;
       for (function_decl_diff_sptrs_type::const_iterator i =
 	     d.get_priv()->sorted_changed_member_functions_.begin();
 	   i != d.get_priv()->sorted_changed_member_functions_.end();
@@ -970,15 +955,9 @@  default_reporter::report(const class_or_union_diff& d,
 
 	  string repr =
 	    (*i)->first_function_decl()->get_pretty_representation();
-	  if (emitted
-	      && i != d.get_priv()->sorted_changed_member_functions_.begin())
-	    out << "\n";
 	  out << indent << "  '" << repr << "' has some sub-type changes:\n";
 	  diff->report(out, indent + "    ");
-	  emitted = true;
 	}
-      if (emitted)
-	out << "\n";
     }
 
   // data members
@@ -1090,14 +1069,12 @@  default_reporter::report(const class_or_union_diff& d,
 	       i != d.class_or_union_diff::get_priv()->deleted_member_types_.end();
 	       ++i)
 	    {
-	      if (i != d.class_or_union_diff::get_priv()->deleted_member_types_.begin())
-		out << "\n";
 	      decl_base_sptr mem_type = i->second;
 	      out << indent << "  '"
 		  << mem_type->get_pretty_representation()
-		  << "'";
+		  << "'\n";
 	    }
-	  out << "\n\n";
+	  out << "\n";
 	}
       // report changes
       if (numchanges)
@@ -1135,7 +1112,6 @@  default_reporter::report(const class_or_union_diff& d,
 	  report_mem_header(out, numins, 0, ins_kind,
 			    "member type", indent);
 
-	  bool emitted = false;
 	  for (vector<insertion>::const_iterator i = e.insertions().begin();
 	       i != e.insertions().end();
 	       ++i)
@@ -1146,8 +1122,6 @@  default_reporter::report(const class_or_union_diff& d,
 		   j != i->inserted_indexes().end();
 		   ++j)
 		{
-		  if (emitted)
-		    out << "\n";
 		  mem_type = second->get_member_types()[*j];
 		  if (!d.class_or_union_diff::get_priv()->
 		      member_type_has_changed(get_type_declaration(mem_type)))
@@ -1155,12 +1129,11 @@  default_reporter::report(const class_or_union_diff& d,
 		      out << indent << "  '"
 			  << get_type_declaration(mem_type)->
 			get_pretty_representation()
-			  << "'";
-		      emitted = true;
+			  << "'\n";
 		    }
 		}
 	    }
-	  out << "\n\n";
+	  out << "\n";
 	}
     }
 
@@ -1176,23 +1149,18 @@  default_reporter::report(const class_or_union_diff& d,
 	   i != e.deletions().end();
 	   ++i)
 	{
-	  if (i != e.deletions().begin())
-	    out << "\n";
 	  member_function_template_sptr mem_fn_tmpl =
 	    first->get_member_function_templates()[i->index()];
 	  out << indent << "  '"
 	      << mem_fn_tmpl->as_function_tdecl()->get_pretty_representation()
-	      << "'";
+	      << "'\n";
 	}
-      if (numdels)
-	out << "\n\n";
 
       // report insertions
       int numins = e.num_insertions();
       if (numins)
 	report_mem_header(out, numins, 0, ins_kind,
 			  "member function template", indent);
-      bool emitted = false;
       for (vector<insertion>::const_iterator i = e.insertions().begin();
 	   i != e.insertions().end();
 	   ++i)
@@ -1203,18 +1171,13 @@  default_reporter::report(const class_or_union_diff& d,
 	       j != i->inserted_indexes().end();
 	       ++j)
 	    {
-	      if (emitted)
-		out << "\n";
 	      mem_fn_tmpl = second->get_member_function_templates()[*j];
 	      out << indent << "  '"
 		  << mem_fn_tmpl->as_function_tdecl()->
 		get_pretty_representation()
-		  << "'";
-	      emitted = true;
+		  << "'\n";
 	    }
 	}
-      if (numins)
-	out << "\n\n";
     }
 
   // member class templates.
@@ -1229,23 +1192,18 @@  default_reporter::report(const class_or_union_diff& d,
 	   i != e.deletions().end();
 	   ++i)
 	{
-	  if (i != e.deletions().begin())
-	    out << "\n";
 	  member_class_template_sptr mem_cls_tmpl =
 	    first->get_member_class_templates()[i->index()];
 	  out << indent << "  '"
 	      << mem_cls_tmpl->as_class_tdecl()->get_pretty_representation()
-	      << "'";
+	      << "'\n";
 	}
-      if (numdels)
-	out << "\n\n";
 
       // report insertions
       int numins = e.num_insertions();
       if (numins)
 	report_mem_header(out, numins, 0, ins_kind,
 			  "member class template", indent);
-      bool emitted = false;
       for (vector<insertion>::const_iterator i = e.insertions().begin();
 	   i != e.insertions().end();
 	   ++i)
@@ -1256,18 +1214,13 @@  default_reporter::report(const class_or_union_diff& d,
 	       j != i->inserted_indexes().end();
 	       ++j)
 	    {
-	      if (emitted)
-		out << "\n";
 	      mem_cls_tmpl = second->get_member_class_templates()[*j];
 	      out << indent << "  '"
 		  << mem_cls_tmpl->as_class_tdecl()
 		->get_pretty_representation()
-		  << "'";
-	      emitted = true;
+		  << "'\n";
 	    }
 	}
-      if (numins)
-	out << "\n\n";
     }
 }
 
diff --git a/src/abg-leaf-reporter.cc b/src/abg-leaf-reporter.cc
index 77569d2c..a3c96432 100644
--- a/src/abg-leaf-reporter.cc
+++ b/src/abg-leaf-reporter.cc
@@ -489,7 +489,6 @@  leaf_reporter::report(const class_or_union_diff& d,
       if (numdels)
 	report_mem_header(out, numdels, num_filtered, del_kind,
 			  "member function", indent);
-      bool emitted = false;
       for (string_member_function_sptr_map::const_iterator i =
 	     d.get_priv()->deleted_member_functions_.begin();
 	   i != d.get_priv()->deleted_member_functions_.end();
@@ -500,16 +499,10 @@  leaf_reporter::report(const class_or_union_diff& d,
 	      && !get_member_function_is_virtual(i->second))
 	    continue;
 
-	  if (emitted
-	      && i != d.get_priv()->deleted_member_functions_.begin())
-	    out << "\n";
 	  method_decl_sptr mem_fun = i->second;
 	  out << indent << "  ";
 	  represent(*ctxt, mem_fun, out);
-	  emitted = true;
 	}
-      if (emitted)
-	out << "\n";
 
       // report insertions;
       int numins = d.get_priv()->inserted_member_functions_.size();
@@ -517,7 +510,6 @@  leaf_reporter::report(const class_or_union_diff& d,
       if (numins)
 	report_mem_header(out, numins, num_filtered, ins_kind,
 			  "member function", indent);
-      emitted = false;
       for (string_member_function_sptr_map::const_iterator i =
 	     d.get_priv()->inserted_member_functions_.begin();
 	   i != d.get_priv()->inserted_member_functions_.end();
@@ -528,22 +520,15 @@  leaf_reporter::report(const class_or_union_diff& d,
 	      && !get_member_function_is_virtual(i->second))
 	    continue;
 
-	  if (emitted
-	      && i != d.get_priv()->inserted_member_functions_.begin())
-	    out << "\n";
 	  method_decl_sptr mem_fun = i->second;
 	  out << indent << "  ";
 	  represent(*ctxt, mem_fun, out);
-	  emitted = true;
 	}
-      if (emitted)
-	out << "\n";
 
       // report member function with sub-types changes
       int numchanges = d.get_priv()->sorted_changed_member_functions_.size();
       if (numchanges)
 	report_mem_header(out, change_kind, "member function", indent);
-      emitted = false;
       for (function_decl_diff_sptrs_type::const_iterator i =
 	     d.get_priv()->sorted_changed_member_functions_.begin();
 	   i != d.get_priv()->sorted_changed_member_functions_.end();
@@ -563,15 +548,9 @@  leaf_reporter::report(const class_or_union_diff& d,
 
 	  string repr =
 	    (*i)->first_function_decl()->get_pretty_representation();
-	  if (emitted
-	      && i != d.get_priv()->sorted_changed_member_functions_.begin())
-	    out << "\n";
 	  out << indent << "  '" << repr << "' has some changes:\n";
 	  diff->report(out, indent + "    ");
-	  emitted = true;
 	}
-      if (numchanges)
-	out << "\n";
     }
 
   // data members
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 6d4e2faa..277f5721 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -135,6 +135,11 @@  test-abidiff-exit/test-leaf-peeling-v0.o \
 test-abidiff-exit/test-leaf-peeling-v1.cc \
 test-abidiff-exit/test-leaf-peeling-v1.o \
 test-abidiff-exit/test-leaf-peeling-report.txt \
+test-abidiff-exit/test-leaf-cxx-members-v0.cc \
+test-abidiff-exit/test-leaf-cxx-members-v0.o \
+test-abidiff-exit/test-leaf-cxx-members-v1.cc \
+test-abidiff-exit/test-leaf-cxx-members-v1.o \
+test-abidiff-exit/test-leaf-cxx-members-report.txt \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\
diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
new file mode 100644
index 00000000..082db83e
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
@@ -0,0 +1,42 @@ 
+Leaf changes summary: 4 artifacts changed
+Changed leaf types summary: 1 leaf type changed
+Removed/Changed/Added functions summary: 1 Removed, 1 Changed, 1 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 Removed function:
+
+  [D] 'method virtual int ops::deleted_fn()'    {_ZN3ops10deleted_fnEv}
+    note that this removes an entry from the vtable of struct ops
+
+1 Added function:
+
+  [A] 'method virtual long int ops::added_fn()'    {_ZN3ops8added_fnEv}
+    note that this adds a new entry to the vtable of struct ops
+
+1 function with some sub-type change:
+
+  [C] 'method virtual int ops::changed_fn()' at test-leaf-cxx-members-v1.cc:6:1 has some sub-type changes:
+    return type changed:
+      type name changed from 'int' to 'long int'
+      type size changed from 32 to 64 (in bits)
+
+'struct ops at test-leaf-cxx-members-v0.cc:1:1' changed:
+  type size changed from 128 to 192 (in bits)
+  1 member function deletion:
+    'method virtual int ops::deleted_fn()' at test-leaf-cxx-members-v0.cc:3:1, virtual at voffset 0/1    {_ZN3ops10deleted_fnEv}
+  1 member function insertion:
+    'method virtual long int ops::added_fn()' at test-leaf-cxx-members-v1.cc:3:1, virtual at voffset 0/1    {_ZN3ops8added_fnEv}
+  there are member function changes:
+    'method virtual int ops::changed_fn()' has some changes:
+      return type changed:
+        type name changed from 'int' to 'long int'
+        type size changed from 32 to 64 (in bits)
+  there are data member changes:
+    type 'int' of 'ops::deleted_var' changed:
+      type name changed from 'int' to 'long int'
+      type size changed from 32 to 64 (in bits)
+    and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1, size changed from 32 to 64 (in bits) (by +32 bits)
+    type 'int' of 'ops::changed_var' changed:
+      type name changed from 'int' to 'long int'
+      type size changed from 32 to 64 (in bits)
+    and offset changed from 96 to 128 (in bits) (by +32 bits), size changed from 32 to 64 (in bits) (by +32 bits)
diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.cc b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.cc
new file mode 100644
index 00000000..89612562
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.cc
@@ -0,0 +1,12 @@ 
+struct ops {
+  int deleted_var;
+  virtual int deleted_fn() { return 0; }
+
+  int changed_var;
+  virtual int changed_fn() { return 0; }
+};
+
+void reg(ops& x) {
+  ops instantiate = x;
+  (void) instantiate;
+}
diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.o b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..8202ecb5dd7cb2bbe79a0a2f3ff42a103f56387c
GIT binary patch
literal 7328
zcmcIpYiwLc6`s4-Yp=8R`mymVPRiPC+NAAny{^;7Nt?QH-E2r3LYhQTNVQ(u*Y>LR
z+S<K#9YkqGB*2OaiFidK0)9X%gak+kDGE;!1mYL{1F8@|c&Ji9NDT;yNFbH-%{^y5
zJA1dNAWn35=6vUTXU?5F_sm@9<*8?$jyR4IanvETr3tFkbaJadp}P}mx4K9B#z_aK
zjoWr=6Ws5hwv+o^)OK?p;ZEl^-^yM5ODebd(yiR3)3<W3PTkIJ{yMk$^IK$9mw$0n
zdu+aR=Q{M~Zw8DIJ-4}bJNIf2ymFVO?&LN*skwdWL=4)dZb)kjw6@C>>AlLsE2|ZC
zAfi;0X2$d!1^tjv5qNhxPHZn3yUEuTr(@JPLswHDp-saG6NNKmfvz%L+Sa_c#&(hL
zPgQLX)!4pF_}9dyTBxTc;1EO4iL_vpSgTfCJP!rHiF7Jt_glLDK$jkrDN#*n+eWZ*
znoWuWN}T9{yNW|^)3+JN9kn+_izQzrG|~~nt#@KH<kLltPgC2i^V&zfWZ7F#PHQv9
z5gF!=bM4dQv3-N=9q2`_oYl|Q+17$FBUh$XhaRODQ$|z$8l1jMKYfU>i#pAEn)*LS
zUC5i<qCbuw66e~V2p!PeD~ki)A_b9myiK+kJ#7&^2c$UHdI%i^j1eMN7S+!0lQI(O
zTjQskE@yWl8Fv$diPL>uPABCVNjh#U9zPI27$1-CN;>pbxEbe(0XI=fj14?CFsGrW
zs5+V)8=#e<Z86i<44YCd?aECYP417wEe0is2}r5dAZ7rS$nTl>v;Di05`SA`Gd=8k
z^#E-hDjvs3?e|dUP~4@5m^jQ>M<;hWdz_;><*t63lx}d47Cq@8bIG8~>28_m|NP+r
z-0}9Pa$}Nigor(3M*aYLGtQA@;4zm>#X;&L5=F-C_Gg@IvXq!h22sBdL>-8#Y!c)h
zAX~;kVZ&jk<v^LArz?1(daaTc$gp&{m1QBYt;;eK`~R!GN`YN9O23Zt&qO=@>XJ??
z)KR`y@zYCQVLm;(v5{W(mS?<jCA~h9nVr??L_1V#b@gtK?*3SG&)G<XLev=gOhe3I
zI5#3`ICmS^j%d&KqE2ii;*!N*ELK$h+;LRU$+4*qJeOC+QpGQn{9?iP)a+uRwBXI<
z=SynNTk`y%o6oQNGfVkZznu3kta|Fm@zbfv2Oh|drN%R(nQUrwBs-qXj*O<#%l=x)
zdwjtwdFA3PsfF_F;^Q0pA4xwlmP#+A(({GNh0^R|d8M?nR!OgwUM!a8(tcrP$*WYy
zUN4m2b(%Q)r^cq%<xa9ATiFu&kiWdTW!kzY5Z37~GN|#wz;GW0AhR+$b@CWZi>PCw
zK1}uTq~3Y9OzTP=nVdY3+Vhk*Q!JE%`ABCU-D}MZFSYI5Iv+l%oQ+Oru&pI_nbyTl
z!f`?Ls(mui`SHkPd-MCrfiTz}gOQ|>{d`~!imCAZE&fN?uHPE5KOflHRM${yw8j4Q
zojL;nZm_r2nOxOchXHVv-off@uEL3>UR-^G3Bz%ssTVhxnL2|@TV0sAiV9bVxJsT<
zt<hDzwE$~wu*KEcTt(F|MBHG*s<XIiR~-hxRqtznr|R$^LiKo4V|UeEHM%HN6{K6!
zx?FQ>KvG=#7bY$r#%!CbqBf*;SJj*}H1;6<4jb!1D)?cI<3t;X;Acr4R$J>qDm>0^
zL&kbY1vItV!2e~!yN&fMJIf`)hm7@uvoqwgbV*+W{BwiaA!A)t@BH1$zEqQaQfPE>
zwn`d1Wk+hO85i<u$vSnWra)ySq*=R89c7<WXO3q_@_E{b3Nyv^Z1z#=FBK{kUF7q{
z()`L4Rl!rwX?J~x`Upim9JR43|MgBF@;tC}VU@{~lKmPhY=|xqnLFST6E@oawE#!W
zvjsSwjquI*cN@;KvQTj(7nr3F95JQ<l1POR{Ph4<`ix6`!QWzB-fZCbwi4OR9pEx2
zfDRfs#_h$8kUSAI)%Z7A|B!{tI(pH<@8>w5vG6IzU$by|)xT}w+t~jH7JfhLKeg~*
zvi?U4|03(REnF(tKP>#utmk-q%p2xsALC~&e4O?3+@isn6M4n*mkR&7g^T|U3m;_t
zC)}dZUvNzLn}N$3gGr8Tl0UnvyEr+}<Qc-2W!(0cn83|l5px1T9P@rt07>llBLS>&
zi3w=Jz|H$&xGui=eqr2Ub~N)b@5_9Dfy+CPVjl7|^VwtJ;&0|}=DFYS2QKygNdq_Y
zIcMPJxD^98<A1`y!{!;78GnNPft&HC4cv_Xih-N)Ri<)bnU2BK`sJWqlwR3eDr9_b
z!&jNPf?rUXnMy@zAXD8G<SaX1q9d+ltJyFz)o4PV<h>)XU<C9s<(16R%4}hY+;~r^
zcOX-+m5{e4RyPaw^=8%<^gcqV>waNortDFMItzx%_~M{DGO!S6saWzP3eAkl%&sgi
zd-TVROligUGW11~rmvXU7dVc$n4d2fmWi@ZURhh^Y5$*_bF_eEuP}p6up;lvFzMLW
z<WEx4Hsa?(_}Kr1Cv7AC3FgcGhVfDRgePqyzI;2FeC*@GleQ6mF=YIq_rUj=Z_Xd~
za~W6K6k&D^0k!7a1^^3uO_66ZVS?2E3g5WYFU%j_TggZ8dkI5Kmo8JjWGY7A9{)kI
zA?AO}&k|SPe@*dFO2(J^DJfWP>e&YTZ!+OO@k<T(KV-htV^(YSDquyr@GK{+#IA?T
zKTh@b{C|=#J^vQ}4&jiL{@wZ!z91K=6E^-EJbsQHhpEL}2w$H6*9i+7zaJop?2Cc6
z#TWgn)Cn8EpFd$_-^2K*GxqpDA`C;(UxaF8507u|O>khi>ieq@48}A&9<t)v{5Qx~
z=MR!0InMt&^N~NkqV4>DPnaDOVP4=D;v1IHQq|uZ@O$~jrxDB$oBuZ1!t$2~x4~TG
zk3U@7`Qtwt?3kde^Y~JSfkA9vmw>SGH+bRvShIq27QQj;@o_%2V}kNC9{(EirLleS
z-4Qnae%{oknTY#Gt+dC#kH?pF%I<204?iEY1{77=(FXkQ@FD3w3COMRq&>)d%T4{p
zJY*S0;R$qt`MgX8R=;EZ)hZHn<o*RdM*Oh(yBacoDIy79>Id?%$42Z8cAVz-;BLag
j<{y7qV;9h>@+N;Q+^ix&$G(508OFcE`4=pLrK*1e`SU$a

literal 0
HcmV?d00001

diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc
new file mode 100644
index 00000000..0437584e
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.cc
@@ -0,0 +1,14 @@ 
+struct ops {
+  // TODO: type, name and size are differnent from deleted_var, but this is
+  // still reported as a change rather than a deletion and an addition.
+  long added_var;
+  virtual long added_fn() { return 0; }
+
+  long changed_var;
+  virtual long changed_fn() { return 0; }
+};
+
+void reg(ops& x) {
+  ops instantiate = x;
+  (void) instantiate;
+}
diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.o b/tests/data/test-abidiff-exit/test-leaf-cxx-members-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..617ffd21551b158227af04c6e601ebc5a12e6b9b
GIT binary patch
literal 7360
zcmcIpU2L0I89txCIC1L6apGo4+6LUL?OIlQahA2|x^C&RtnPkF)iz}s8;;|A`Oy+P
zvYjlQI;b|W5miHtkj5?|elFM@)3^Zy1r-emdVxS(z%C}JKuBCnqT>RJKpO9JzV~&G
zk3aJ>#9Mtn@B6&J=jS`;eCg*Vo_I9mI7-M-52`ImP^B&>w)ip4j;VcWueObn44>`X
zc2FB*zLVNG^Ig<-GmmhS+4VnUUw(fmyZ++4*(;OpW?!4QnO*-?cKs)RB(1u7;~j0W
z{^ITLLVorg-y=ZIuCLt6zSaY)?3Ibz+4VRzx3X8xM0`=Vkb#r>O#CWE`zUm8X}O|K
zgp_L0-Vr@kJ|7S+1nWM>i5w(lH`%&k^f)mW=xOOCv}F)s!Y~HS(Nm^J>sk-i>0T!M
zi#6Rnb-J$-{teNoHsaJ63?ks1P~;2=k#;S(m=guS3B?tvdyk&q(W8^HOIW+aw-KzI
zR@23SOO)Jwa}|ZWU3W8z9(A;Y7m8IC(@19oU5}%asHckzAEmZi*L5FpiAWeJc9gw2
z<+Qhg7aC;7*?gT0d)7$b2`;pey#p#|TN_e`HqNL{ovarlM<f1C7+s^EK9~&<)2c_S
zf0J0yZ@C4396v<P<}E^ZYTu3do!=%Qv@xZ2{fTs)H0eTE)gWhcH=(=1B133nUJYC$
z;YhHpX{VenXJ0H4bz{3?lf7L|yqEkFjvI+a?~fjf9*qtp9C}IIwDVBE8!N^}`XA_@
z(NIfRol1=K(}K~uh^cFZPN}vI<;G4WMx!u`K=5M%QmWmL=|e@bdm{Q|-#|j*Z)>im
zIp41nZ0}UjC^B{ICFa4XOLG}}n6b_{Gw~khlrFgolh6$xltmNHUbIW>@H-*JbUEE^
zV|}kb+>g%pgq0hS%0m?6juE3P0D9BT$%Jn)lSoG4)JIN~1U>IhJDEf=Hk0t9e%X(@
zGpsTRIPZe9rQ9vvFc`25s1wi7<4>+$y_iu=gTintD@0&hmK7)Z|5y741$Nai{W{Ly
zV;%IXH+x#Fj;eZ<YHHET&!!4%YpEq~Y1%7SQmdJCp`gnN$5nf6A@2z9eJOm`Q=t%r
zs8jTY_rDZA$cl*J;e0QYfpNEi?F{#PKkP(?LN00kRsP)5r?J(Jk4${>Tuv<%E7g3l
zx{$AWsxY4~&UrJr*`mtN%n-AhFRNT`wK~0+TdtOK)r-rXI(d3BIez!unUUnt^l&<p
z93ILX&18m#lc}ZZO3^z$=M}y3LV?75xiEixZS-*J@JKQ>mrTv(D;JA}`EsdPTB)Q~
ziq9_;XHwPt^rBa(rPdk!0v{QfSmipXpz8cWMcq?fT2@?tW@xKiGGuhrFMPHLXqAdi
zW9wfm73Y%JH9reU24HyN?Bg^7B94f7FKw1*^=Y)FT2<=g`1t+F1CMyq3;Cko_f+P{
zL2DFfc6aC*rd9C5S>>$7on718B3EfuU@z1A8MS)No`}U?35|EOK1K$tdw_kL>G6($
z^colcwJrZkzFO&bM)c46dR8?wR2m(*AAH!LAi(v{x(1c2+8ZzcuF@x3gU(gB$uyFy
zZ#F?VZaR(R`Xkq%aOvm^B3IG26+munmg<$R(nnuCNRq4Q3<)4t(OwupuG$rVBF;b)
z{NqjVWCQL;s9ksJ^sf3?oh$<F3BuO3F7LQ?APFx03nG^<XSU8&`kG<G6s7LUq4kI8
zchFc5vVT9Maolu$5`2=xLAA9WWPhI_e2=jnlD?Z-YGRLfquW@|a;|)V@IA)*(U&p8
zHs}$a*cyF@xIM<Ys`dOo*>vxdO-Ap*b-R_n>jiIX<F&o&{Dr!mDI@XBP3!zAwmE;{
zbY>`*qcbQ!y|9|e93g%&U#aMwJhxDsEltq&cmA9<mlcaEH&)|6KJ-cE0p}G~m&}ly
z(pXu0=n;}J11>S4qf=iK@ZHRt4RFkf_?z+XGz?`$VW*K=pcWoDVtg7Pk?ahD`zT$3
zm=XNfEXx}W9N$_(`<MYPV*==yfg^7pIzoi?43hXi@WonPg3DMu&w9ZRahxw%_({g!
zwD2(FZ(H~YmL<Q$zrpyAE&D&S{Cf-k7R$FR{4mRZwea^?&T?In7vE=~a~9soa?Zk!
zv0UI54c4O2tCsyISn-yH|B~?^SvWo~KsUHWqrdPps?^^MT-F{`aV|poCGqz$4!7Fg
zK>TPEdx?oS<|&D_1VS9Mze#{ZcKjh9*0{t3G-lvtzZh<aZ@zcPJIID+J!Zem_Zqmo
z8%g#<on}1;EL`l(`pr5=4SV3S|DP~$v!167+{{}sa5Mg^1|Brdz|8n#Y!BRwKV{%%
z{L2Pz##iae#U;A_Qd=$i?YwYhZ!w>)dTUjcp2=79Dm`7PC=H|oF1mFJ>i|jDLW!rG
zCB6pX!KKTk^kS)yUnDa=V;T)e<sU6%tsQHJ2itnnD|7l>p}IAnpPnvztIAa8%lOKm
z?&!*VvjAvuq3B5z8Wxo<l$Mq}y2Gc7rK*>v&yW;-xfGt`INp42wwzxgm$`CjWtqqP
ze`!8N3s=q#GtL+b@}3NmjB`%@R3&XQ|7?Ii&H(X~wweDK_LuVx`Ekn?KWUr!KNH}O
zb6xzTZRS58;J@d;@UODJIes|bC9kwe!tA{Ru9xsbgnpX@Ta#o?#!Qg-w|U@X-$ER`
zuTqcT`w2r#`zHJq!h*=oe~63_^P~D_$XEA2RG&@q%f2aE%S=7i#Q!aJ_;>!#HSzx;
z`^!GeV%-@9Y?dC(<(O62PXopuw|jg1KTnt*f6HHkq)XCclK2aDnV6vbH#vWn4F{>k
zSct#O|C@vb<xc`6lJn4)w){o@Ix#`{5AdgqoOj5Nea6mzjWDF5zYEpT5a&0~Bp5JU
zv&DVH#-PNXg+UWmT-)|%WTWfX)>}VMas9twf7Fk^dD!*;hA=xOqP)N_1pWkUk|nAS
zNNHoRAL5QrK`_X+eV=qe^&jAgTVr3;-%F2OKkn0ZOt@U-{IU-N12xzZ5S0IGod3G^
z^7k=(bJ+Ru9nlbz>~3=YP4<_@woNq2KgEaI6g#5-*emV)dpW<XQ#MzZ_|hZycflpU
zwD>;(@V5WY`EqoZIC3j~(vGsfWv2dOUakzK_zCnR`|~mtSlwp-H)@E_NdE=KcT3Rx
zT@M()Bsqz{>>sGd&W+et*>H;AgF6We8h`wS%|1Kee~Uj7u0t@$?mx;5@=tdAPM@(H
HEK&UvI(kBo

literal 0
HcmV?d00001

diff --git a/tests/data/test-abidiff/test-struct1-report.txt b/tests/data/test-abidiff/test-struct1-report.txt
index 1300d96c..fe5064c6 100644
--- a/tests/data/test-abidiff/test-struct1-report.txt
+++ b/tests/data/test-abidiff/test-struct1-report.txt
@@ -3,7 +3,6 @@ 
     type size changed from 192 to 256 (in bits)
     1 member function insertion:
       'method virtual int s0::foo(int, char) const', virtual at voffset 2/2
-
     1 data member deletion:
       'char s0::m1', at offset 96 (in bits)
     1 data member insertion:
diff --git a/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt b/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt
index d88db9c4..78f91a58 100644
--- a/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt
+++ b/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt
@@ -9,10 +9,8 @@  Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
         type size hasn't changed
         1 member function insertion:
           'method virtual void Interface::method2()', virtual at voffset 3/4
-
         1 member function change:
           'method virtual void Interface::method3()' has some sub-type changes:
             the vtable offset of method virtual void Interface::method3() changed from 3 to 4
               note that this is an ABI incompatible change to the vtable of class Interface
 
-
diff --git a/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt b/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt
index 451a4981..62785044 100644
--- a/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt
+++ b/tests/data/test-diff-pkg/tbb-4.1-9.20130314.fc22.x86_64--tbb-4.3-3.20141204.fc23.x86_64-report-0.txt
@@ -119,10 +119,8 @@ 
                                 'char tbb::internal::mail_outbox::pad[104]', at offset 136 (in bits) at mailbox.h:114:1
                 1 member function deletion:
                   'method virtual tbb::task* tbb::internal::generic_scheduler::receive_or_steal_task(tbb::internal::reference_count&, bool)' at scheduler.h:391:1, virtual at voffset 7/7
-
                 1 member function insertion:
                   'method virtual tbb::task* tbb::internal::generic_scheduler::receive_or_steal_task(tbb::internal::reference_count&)' at scheduler.h:362:1, virtual at voffset 7/7
-
                 3 data member deletions:
                   'unsigned int tbb::internal::generic_scheduler::hint_for_push', at offset 896 (in bits) at scheduler.h:171:1
                   'volatile intptr_t* tbb::internal::generic_scheduler::my_ref_top_priority', at offset 2560 (in bits) at scheduler.h:433:1
diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index ebac7a00..17523afa 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -167,6 +167,16 @@  InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test-leaf-peeling-report.txt",
     "output/test-abidiff-exit/test-leaf-peeling-report.txt"
   },
+  {
+    "data/test-abidiff-exit/test-leaf-cxx-members-v0.o",
+    "data/test-abidiff-exit/test-leaf-cxx-members-v1.o",
+    "",
+    "--leaf-changes-only",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE
+    | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
+    "data/test-abidiff-exit/test-leaf-cxx-members-report.txt",
+    "output/test-abidiff-exit/test-leaf-cxx-members-report.txt"
+  },
   {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
 };