Represent a removed+added data member at a given offset as changed

* include/abg-fwd.h (get_data_member_offset): Declare new overload for
	decl_base_sptr.
	* include/abg-comparison.h (unsigned_decl_base_sptr_map)
	(unsigned_changed_type_or_decl_map): New typedefs.
	* src/abg-ir.cc (get_data_member_offset): Define new overload for decl_base_sptr.
	* src/abg-comparison.cc (diff_kind::subtype_change_kind): New
	enumerator for a change about a type or sub-type of a member of a
	structure/enum.
	(report_mem_header): Handle the new enumerator above.
	(class_diff::priv::{deleted_dm_by_offset_, inserted_dm_by_offset_,
	changed_dm_}): New data members.
	(class_diff::priv::subtype_changed_dm_): Renamed
	class_diff::priv::changed_data_members_ into this.
	(class_diff::priv::subtype_changed_dm): Renamed
	class_diff::priv::data_member_has_changed into this. Adjust.
	(class_diff::count_filtered_subtype_changed_dm): Renamed
	count_filtered_data_members into this.  Adjust.
	(class_diff::priv::count_filtered_changed_dm): New member
	function.
	(class_diff::lookup_tables_empty): Adjust.
	(class_diff::ensure_lookup_tables_populated): Adjust.  Detect when
	a data member is deleted and added back to offset N, and be
	prepared to present that as a change of data member at offset N.
	(class_diff::report): Adjust.  Report data members of a given
	offset that have changed.
	* tests/data/test-diff-dwarf/test6-report.txt: New reference
	report for new test input.
	* tests/data/test-diff-dwarf/test6-v0.cc: Source code for new test
	input binary.
	* tests/data/test-diff-dwarf/test6-v0.o: New test input binary.
	* tests/data/test-diff-dwarf/test6-v1.cc: Source code for new test
	input binary.
	* tests/data/test-diff-dwarf/test6-v1.o: New test input binary.
	* tests/test-diff-dwarf.cc: Adjust to include the new test inputs above.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

include/abg-comparison.h

@@ -69,6 +69,10 @@ typedef unordered_map<size_t, bool> pointer_map;
 /// value is a @ref decl_base_sptr.
 typedef unordered_map<string, decl_base_sptr> string_decl_base_sptr_map;
 
+/// Convenience typedef for a map which key is an unsigned integer and
+/// which value is a @ref decl_base_sptr
+typedef unordered_map<unsigned, decl_base_sptr> unsigned_decl_base_sptr_map;
+
 /// Convenience typedef for a changed type or decl.  The first element
 /// of the pair is the old type/decl and the second is the new one.
 typedef pair<decl_base_sptr, decl_base_sptr> changed_type_or_decl;
@@ -102,11 +106,16 @@ typedef unordered_map<unsigned, changed_parm> unsigned_changed_parm_map;
 typedef unordered_map<unsigned,
 		      function_decl::parameter_sptr> unsigned_parm_map;
 
-/// Convenience typedef for a map which value is changed type of decl.
+/// Convenience typedef for a map which value is changed type or decl.
 /// The key of the map is the qualified name of the type/decl.
 typedef unordered_map<string,
 		      changed_type_or_decl> string_changed_type_or_decl_map;
 
+/// Convience typedef for a map which value is a changed type or decl.
+/// The key of the map is an unsigned integer.
+typedef unordered_map<unsigned,
+		      changed_type_or_decl> unsigned_changed_type_or_decl_map;
+
 /// Convenience typedef for a map which value is a function
 /// parameter.  The key is the name of the function parm.
 typedef unordered_map<string, function_decl::parameter_sptr> string_parm_map;

include/abg-fwd.h

@@ -254,6 +254,9 @@ get_data_member_offset(const var_decl&);
 size_t
 get_data_member_offset(const shared_ptr<var_decl>);
 
+size_t
+get_data_member_offset(const shared_ptr<decl_base>);
+
 void
 set_data_member_is_laid_out(shared_ptr<var_decl>, bool);
 

src/abg-comparison.cc

@@ -1315,6 +1315,7 @@ enum diff_kind
 {
   del_kind,
   ins_kind,
+  subtype_change_kind,
   change_kind
 };
 
@@ -1344,7 +1345,7 @@ report_mem_header(ostream& out,
 		  const string& indent)
 {
   size_t net_number = number;
-  if (k == change_kind)
+  if (k == change_kind || subtype_change_kind)
     net_number = number - num_filtered;
 
   string change;
@@ -1356,8 +1357,10 @@ report_mem_header(ostream& out,
     case ins_kind:
       change = (number > 1) ? "insertions" : "insertion";
       break;
+    case subtype_change_kind:
     case change_kind:
       change = (number > 1) ? "changes" : "change";
+      break;
     }
 
   if (net_number == 0)
@@ -2434,8 +2437,11 @@ struct class_diff::priv
   string_decl_base_sptr_map inserted_member_types_;
   string_changed_type_or_decl_map changed_member_types_;
   string_decl_base_sptr_map deleted_data_members_;
+  unsigned_decl_base_sptr_map deleted_dm_by_offset_;
   string_decl_base_sptr_map inserted_data_members_;
-  string_changed_type_or_decl_map changed_data_members_;
+  unsigned_decl_base_sptr_map inserted_dm_by_offset_;
+  string_changed_type_or_decl_map subtype_changed_dm_;
+  unsigned_changed_type_or_decl_map changed_dm_;
   string_member_function_sptr_map deleted_member_functions_;
   string_member_function_sptr_map inserted_member_functions_;
   string_changed_member_function_sptr_map changed_member_functions_;
@@ -2450,7 +2456,7 @@ struct class_diff::priv
   member_type_has_changed(decl_base_sptr) const;
 
   decl_base_sptr
-  data_member_has_changed(decl_base_sptr) const;
+  subtype_changed_dm(decl_base_sptr) const;
 
   decl_base_sptr
   member_class_tmpl_has_changed(decl_base_sptr) const;
@@ -2459,7 +2465,10 @@ struct class_diff::priv
   count_filtered_bases(const diff_context_sptr&);
 
   size_t
-  count_filtered_data_members(const diff_context_sptr&);
+  count_filtered_subtype_changed_dm(const diff_context_sptr&);
+
+  size_t
+  count_filtered_changed_dm(const diff_context_sptr&);
 
   size_t
   count_filtered_member_functions(const diff_context_sptr&);
@@ -2484,7 +2493,7 @@ class_diff::clear_lookup_tables(void)
   priv_->changed_member_types_.clear();
   priv_->deleted_data_members_.clear();
   priv_->inserted_data_members_.clear();
-  priv_->changed_data_members_.clear();
+  priv_->subtype_changed_dm_.clear();
   priv_->deleted_member_functions_.clear();
   priv_->inserted_member_functions_.clear();
   priv_->changed_member_functions_.clear();
@@ -2507,7 +2516,7 @@ class_diff::lookup_tables_empty(void) const
 	  && priv_->changed_member_types_.empty()
 	  && priv_->deleted_data_members_.empty()
 	  && priv_->inserted_data_members_.empty()
-	  && priv_->changed_data_members_.empty()
+	  && priv_->subtype_changed_dm_.empty()
 	  && priv_->inserted_member_functions_.empty()
 	  && priv_->deleted_member_functions_.empty()
 	  && priv_->changed_member_functions_.empty()
@@ -2652,7 +2661,7 @@ class_diff::ensure_lookup_tables_populated(void) const
 	    if (j != priv_->deleted_data_members_.end())
 	      {
 		if (*j->second != *d)
-		  priv_->changed_data_members_[qname]=
+		  priv_->subtype_changed_dm_[qname]=
 		    std::make_pair(j->second, d);
 		priv_->deleted_data_members_.erase(j);
 	      }
@@ -2660,6 +2669,51 @@ class_diff::ensure_lookup_tables_populated(void) const
 	      priv_->inserted_data_members_[qname] = d;
 	  }
       }
+
+    // Now detect when a data member is deleted from offset N and
+    // another one is added to offset N.  In that case, we want to be
+    // able to say that the data member at offset N changed.
+    for (string_decl_base_sptr_map::const_iterator i =
+	   priv_->deleted_data_members_.begin();
+	 i != priv_->deleted_data_members_.end();
+	 ++i)
+      {
+	unsigned offset = get_data_member_offset(i->second);
+	priv_->deleted_dm_by_offset_[offset] = i->second;
+      }
+
+    for (string_decl_base_sptr_map::const_iterator i =
+	   priv_->inserted_data_members_.begin();
+	 i != priv_->inserted_data_members_.end();
+	 ++i)
+      {
+	unsigned offset = get_data_member_offset(i->second);
+	priv_->inserted_dm_by_offset_[offset] = i->second;
+      }
+
+    for (unsigned_decl_base_sptr_map::const_iterator i =
+	   priv_->inserted_dm_by_offset_.begin();
+	 i != priv_->inserted_dm_by_offset_.end();
+	 ++i)
+      {
+	unsigned_decl_base_sptr_map::const_iterator j =
+	  priv_->deleted_dm_by_offset_.find(i->first);
+	if (j != priv_->deleted_dm_by_offset_.end())
+	  priv_->changed_dm_[i->first] = std::make_pair(j->second, i->second);
+      }
+
+    for (unsigned_changed_type_or_decl_map::const_iterator i =
+	   priv_->changed_dm_.begin();
+	 i != priv_->changed_dm_.end();
+	 ++i)
+      {
+	priv_->deleted_dm_by_offset_.erase(i->first);
+	priv_->inserted_dm_by_offset_.erase(i->first);
+	priv_->deleted_data_members_.erase
+	  (i->second.first->get_qualified_name());
+	priv_->inserted_data_members_.erase
+	  (i->second.second->get_qualified_name());
+      }
   }
 
   {
@@ -2811,13 +2865,13 @@ class_diff::priv::member_type_has_changed(decl_base_sptr d) const
 /// @return the new data member if the given data member has changed,
 /// or NULL if if hasn't.
 decl_base_sptr
-class_diff::priv::data_member_has_changed(decl_base_sptr d) const
+class_diff::priv::subtype_changed_dm(decl_base_sptr d) const
 {
   string qname = d->get_qualified_name();
   string_changed_type_or_decl_map::const_iterator it =
-    changed_data_members_.find(qname);
+    subtype_changed_dm_.find(qname);
 
-  return ((it == changed_data_members_.end())
+  return ((it == subtype_changed_dm_.end())
 	  ? decl_base_sptr()
 	  : it->second.second);
 }
@@ -2874,12 +2928,12 @@ class_diff::priv::count_filtered_bases(const diff_context_sptr& ctxt)
 ///
 /// @return the number of data members whose changes got filtered out.
 size_t
-class_diff::priv::count_filtered_data_members(const diff_context_sptr& ctxt)
+class_diff::priv::count_filtered_subtype_changed_dm(const diff_context_sptr& ctxt)
 {
   size_t num_filtered= 0;
   for (string_changed_type_or_decl_map::const_iterator i =
-	 changed_data_members_.begin();
-       i != changed_data_members_.end();
+	 subtype_changed_dm_.begin();
+       i != subtype_changed_dm_.end();
        ++i)
     {
       var_decl_sptr o =
@@ -2921,6 +2975,31 @@ class_diff::priv::count_filtered_member_functions(const diff_context_sptr& ctxt)
   return num_filtered;
 }
 
+/// Count the number of data member offsets that have changed.
+///
+/// @param ctxt the diff context to use.
+size_t
+class_diff::priv::count_filtered_changed_dm(const diff_context_sptr& ctxt)
+{
+  size_t num_filtered= 0;
+  for (unsigned_changed_type_or_decl_map::const_iterator i =
+	 changed_dm_.begin();
+       i != changed_dm_.end();
+       ++i)
+    {
+      var_decl_sptr o =
+	dynamic_pointer_cast<var_decl>(i->second.first);
+      var_decl_sptr n =
+	dynamic_pointer_cast<var_decl>(i->second.second);
+      diff_sptr diff = compute_diff(o, n, ctxt);
+      ctxt->maybe_apply_filters(diff);
+      if (diff->is_filtered_out())
+	++num_filtered;
+    }
+  return num_filtered;
+
+}
+
 /// Constructor of class_diff
 ///
 /// @param first_scope the first class of the diff.
@@ -3261,16 +3340,36 @@ class_diff::report(ostream& out, const string& indent) const
 	}
 
       // report change
-      size_t numchanges = priv_->changed_data_members_.size();
-      size_t num_filtered = priv_->count_filtered_data_members(context());
+      size_t numchanges = priv_->subtype_changed_dm_.size();
+      size_t num_filtered = priv_->count_filtered_subtype_changed_dm(context());
+      if (numchanges)
+	{
+	  report_mem_header(out, numchanges, num_filtered,
+			    subtype_change_kind, "data member", indent);
+
+	  for (string_changed_type_or_decl_map::const_iterator it =
+		 priv_->subtype_changed_dm_.begin();
+	       it != priv_->subtype_changed_dm_.end();
+	       ++it)
+	    {
+	      var_decl_sptr o =
+		dynamic_pointer_cast<var_decl>(it->second.first);
+	      var_decl_sptr n =
+		dynamic_pointer_cast<var_decl>(it->second.second);
+	      represent(o, n, context(), out, indent + " ");
+	    }
+	  out << "\n";
+	}
+
+      numchanges = priv_->changed_dm_.size();
+      num_filtered = priv_->count_filtered_changed_dm(context());
       if (numchanges)
 	{
 	  report_mem_header(out, numchanges, num_filtered,
 			    change_kind, "data member", indent);
-
-	  for (string_changed_type_or_decl_map::const_iterator it =
-		 priv_->changed_data_members_.begin();
-	       it != priv_->changed_data_members_.end();
+	  for (unsigned_changed_type_or_decl_map::const_iterator it =
+		 priv_->changed_dm_.begin();
+	       it != priv_->changed_dm_.end();
 	       ++it)
 	    {
 	      var_decl_sptr o =
@@ -3517,8 +3616,17 @@ class_diff::traverse(diff_node_visitor& v)
 
   // data member changes
   for (string_changed_type_or_decl_map::const_iterator i =
-	 priv_->changed_data_members_.begin();
-       i != priv_->changed_data_members_.end();
+	 priv_->subtype_changed_dm_.begin();
+       i != priv_->subtype_changed_dm_.end();
+       ++i)
+    if (diff_sptr d = compute_diff_for_decls(i->second.first,
+					     i->second.second,
+					     context()))
+      TRAVERSE_MEM_DIFF_NODE_AND_PROPAGATE_CATEGORY(d, v);
+
+  for (unsigned_changed_type_or_decl_map::const_iterator i =
+	 priv_->changed_dm_.begin();
+       i != priv_->changed_dm_.end();
        ++i)
     if (diff_sptr d = compute_diff_for_decls(i->second.first,
 					     i->second.second,

src/abg-ir.cc

@@ -862,6 +862,15 @@ size_t
 get_data_member_offset(const var_decl_sptr m)
 {return get_data_member_offset(*m);}
 
+/// Get the offset of a data member.
+///
+/// @param m the data member to consider.
+///
+/// @return the offset (in bits) of @p m in its containing class.
+size_t
+get_data_member_offset(const decl_base_sptr d)
+{return get_data_member_offset(dynamic_pointer_cast<var_decl>(d));}
+
 /// Set a flag saying if a data member is laid out.
 ///
 /// @param m the data member to consider.

tests/data/test-diff-dwarf/test6-report.txt

@@ -0,0 +1,13 @@
+Functions changes summary: 0 Removed, 1 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 function with some indirect sub-type change:
+
+  [C]'function void foo(const S0&)' has some indirect sub-type changes:
+    parameter 0 of type 'const S0&' has sub-type changes:
+      in unqualified underlying type 'S0&':
+        in referenced type 'struct S0':
+          1 data member change:
+           'char S0::m2' name changed to 'S0::m12'
+
+

tests/data/test-diff-dwarf/test6-v0.cc

@@ -0,0 +1,14 @@
+struct S0
+{
+  int m1;
+  char m2;
+
+  S0()
+    : m1(0),
+      m2(0)
+  {}
+};
+
+void
+foo(S0&)
+{}

tests/data/test-diff-dwarf/test6-v1.cc

@@ -0,0 +1,14 @@
+struct S0
+{
+  int m1;
+  char m12;
+
+  S0()
+    : m1(0),
+      m12(0)
+  {}
+};
+
+void
+foo(S0&)
+{}

tests/data/test-diff-filter/test8-report.txt

@@ -0,0 +1,4 @@
+Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+

tests/data/test-diff-filter/test8-v0.cc

@@ -0,0 +1,14 @@
+struct S0
+{
+  int m1;
+  char m2;
+
+  S0()
+    : m1(0),
+      m2(0)
+  {}
+};
+
+void
+foo(S0&)
+{}

tests/data/test-diff-filter/test8-v1.cc

@@ -0,0 +1,14 @@
+struct S0
+{
+  int m1;
+  char m12;
+
+  S0()
+    : m1(0),
+      m12(0)
+  {}
+};
+
+void
+foo(S0&)
+{}

tests/test-diff-dwarf.cc

@@ -92,6 +92,18 @@ InOutSpec in_out_specs[] =
     "data/test-diff-dwarf/test4-report.txt",
     "output/test-diff-dwarf/test4-report.txt"
   },
+  {
+    "data/test-diff-dwarf/test5-v0.o",
+    "data/test-diff-dwarf/test5-v1.o",
+    "data/test-diff-dwarf/test5-report.txt",
+    "output/test-diff-dwarf/test5-report.txt"
+  },
+  {
+    "data/test-diff-dwarf/test6-v0.o",
+    "data/test-diff-dwarf/test6-v1.o",
+    "data/test-diff-dwarf/test6-report.txt",
+    "output/test-diff-dwarf/test6-report.txt"
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL}
 };

tests/test-diff-filter.cc

@@ -117,6 +117,13 @@ InOutSpec in_out_specs[] =
     "data/test-diff-filter/test7-report.txt",
     "output/test-diff-filter/test7-report.txt",
   },
+  {
+    "data/test-diff-filter/test8-v0.o",
+    "data/test-diff-filter/test8-v1.o",
+    "--no-harmless",
+    "data/test-diff-filter/test8-report.txt",
+    "output/test-diff-filter/test8-report.txt",
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL, NULL}
 };