comparison: Represent changed unreachable anonymous unions, structs & enums

Following the changes to represent changed anonymous unreachable enums,
this patch does the same for anonymous unreachable unions, classes and
structs.

Basically, without this patch, this change:

    union
    {
      int a;
      int b;
    };

    ------

    union
    {
      int a;
      int b;
      int c;
    };

yields:

    1 removed type unreachable from any public interface:

      [D] 'union {int a; int b;}' at test_1.c:1:1

    1 added type unreachable from any public interface:

      [A] 'union {int a; int b; int c;}' at test_2.c:1:1

But with the patch, it does yield:

    1 changed type unreachable from any public interface:

      [C] 'union {int a; int b;}' changed:
	type size hasn't changed
	1 data member insertion:
	  'int c' at test-anon-union-v1.c:5:1
	type changed from:
	  union {int a; int b;}
	to:
	  union {int a; int b; int c;}

	* include/abg-fwd.h (class_or_union_types_of_same_kind)
	(is_data_member_of_anonymous_class_or_union): Declare new
	functions.
	* include/abg-ir.h (lookup_data_member): Likewise, declare a new overload.
	* src/abg-ir.cc (class_or_union_types_of_same_kind)
	(lookup_data_member, is_data_member_of_anonymous_class_or_union):
	Define news functions & overloads.
	* src/abg-reporter-priv.cc (represent): When representing a change
	in the name of a data member, if the context is an anonymous type,
	use the non-qualified name of the data member, not its qualified
	name.
	* src/abg-comparison.cc
	(corpus_diff::priv::ensure_lookup_tables_populated): Handle
	deleted/added anonymous enums, unions, classes and structs
	similarly.  That is, if an anonymous type was removed and another
	one got added, if they both have data members (or enumerators) in
	common, then we are probably looking at an anonymous type that was
	changed.  This is because these anonymous types are named using
	their flat representation.
	* tests/data/test-abidiff-exit/test-anon-types-report-1.txt: New
	reference test comparison output.
	* tests/data/test-abidiff-exit/test-anon-types-v{0,1}.o: New
	binary tests input files.
	* tests/data/test-abidiff-exit/test-anon-types-v{0,1}.c: Source
	code of new binary test input.
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
	* tests/test-abidiff-exit.cc (in_out_specs): Add the test inputs
	above to this test harness.

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

include/abg-fwd.h

@@ -495,6 +495,14 @@ is_class_or_union_type(const type_or_decl_base*);
 class_or_union_sptr
 is_class_or_union_type(const type_or_decl_base_sptr&);
 
+bool
+class_or_union_types_of_same_kind(const class_or_union *,
+				  const class_or_union*);
+
+bool
+class_or_union_types_of_same_kind(const class_or_union_sptr&,
+				  const class_or_union_sptr&);
+
 bool
 is_union_type(const type_or_decl_base&);
 
@@ -731,6 +739,15 @@ is_anonymous_data_member(const var_decl*);
 bool
 is_anonymous_data_member(const var_decl&);
 
+bool
+is_data_member_of_anonymous_class_or_union(const var_decl&);
+
+bool
+is_data_member_of_anonymous_class_or_union(const var_decl*);
+
+bool
+is_data_member_of_anonymous_class_or_union(const var_decl_sptr&);
+
 const var_decl_sptr
 get_first_non_anonymous_data_member(const var_decl_sptr);
 

include/abg-ir.h

@@ -4626,6 +4626,10 @@ const var_decl*
 lookup_data_member(const type_base* type,
 		   const char* dm_name);
 
+const var_decl_sptr
+lookup_data_member(const type_base_sptr& type,
+		   const var_decl_sptr&  dm);
+
 const function_decl::parameter*
 get_function_parameter(const decl_base* fun,
 		       unsigned parm_num);

src/abg-comparison.cc

@@ -9630,69 +9630,135 @@ corpus_diff::priv::ensure_lookup_tables_populated()
     // to show what the user expects, namely, a changed anonymous
     // enum.
     {
-      std::set<enum_type_decl_sptr> deleted_anon_enums;
-      std::set<enum_type_decl_sptr> added_anon_enums;
+      std::set<type_base_sptr> deleted_anon_types;
+      std::set<type_base_sptr> added_anon_types;
 
       for (auto entry : deleted_unreachable_types_)
-	if (is_enum_type(entry.second)
-	    && is_enum_type(entry.second)->get_is_anonymous())
-	  deleted_anon_enums.insert(is_enum_type(entry.second));
+	{
+	  if ((is_enum_type(entry.second)
+	       && is_enum_type(entry.second)->get_is_anonymous())
+	      || (is_class_or_union_type(entry.second)
+		  && is_class_or_union_type(entry.second)->get_is_anonymous()))
+	  deleted_anon_types.insert(entry.second);
+	}
+
 
       for (auto entry : added_unreachable_types_)
-	if (is_enum_type(entry.second)
-	    && is_enum_type(entry.second)->get_is_anonymous())
-	  added_anon_enums.insert(is_enum_type(entry.second));
+	if ((is_enum_type(entry.second)
+	     && is_enum_type(entry.second)->get_is_anonymous())
+	    || (is_class_or_union_type(entry.second)
+		&& is_class_or_union_type(entry.second)->get_is_anonymous()))
+	  added_anon_types.insert(entry.second);
 
-      string_type_base_sptr_map added_anon_enum_to_erase;
-      string_type_base_sptr_map removed_anon_enum_to_erase;
+      string_type_base_sptr_map added_anon_types_to_erase;
+      string_type_base_sptr_map removed_anon_types_to_erase;
+      enum_type_decl_sptr deleted_enum;
+      class_or_union_sptr deleted_class;
 
-      // Look for deleted anonymous enums which have enumerators
-      // present in an added anonymous enums ...
-      for (auto deleted_enum : deleted_anon_enums)
+      // Look for deleted anonymous types (enums, unions, structs &
+      // classes) which have enumerators or data members present in an
+      // added anonymous type ...
+      for (auto deleted: deleted_anon_types)
 	{
-	  // Look for any enumerator of 'deleted_enum' that is also
-	  // present in an added anonymous enum.
-	  for (auto enr : deleted_enum->get_enumerators())
+	  deleted_enum = is_enum_type(deleted);
+	  deleted_class = is_class_or_union_type(deleted);
+
+	  // For enums, look for any enumerator of 'deleted_enum' that
+	  // is also present in an added anonymous enum.
+	  if (deleted_enum)
 	    {
-	      bool this_enum_got_changed = false;
-	      for (auto added_enum : added_anon_enums)
+	      for (auto enr : deleted_enum->get_enumerators())
 		{
-		  if (is_enumerator_present_in_enum(enr, *added_enum))
+		  bool this_enum_got_changed = false;
+		  for (auto t : added_anon_types)
 		    {
-		      // So the enumerator 'enr' from the
-		      // 'deleted_enum' enum is also present in the
-		      // 'added_enum' enum so we assume that
-		      // 'deleted_enum' and 'added_enum' are the same
-		      // enum that got changed.  Let's represent it
-		      // using a diff node.
-		      diff_sptr d = compute_diff(deleted_enum,
-						 added_enum, ctxt);
-		      ABG_ASSERT(d->has_changes());
-		      string repr =
-			abigail::ir::get_pretty_representation(is_type(deleted_enum),
-							       /*internal=*/false);
-		      changed_unreachable_types_[repr]= d;
-		      this_enum_got_changed = true;
-		      string r1 = abigail::ir::get_pretty_representation(is_type(deleted_enum));
-		      string r2 = abigail::ir::get_pretty_representation(is_type(added_enum));
-		      removed_anon_enum_to_erase[r1] = deleted_enum;
-		      added_anon_enum_to_erase[r2] = added_enum;
-		      break;
+		      if (enum_type_decl_sptr added_enum = is_enum_type(t))
+			if (is_enumerator_present_in_enum(enr, *added_enum))
+			  {
+			    // So the enumerator 'enr' from the
+			    // 'deleted_enum' enum is also present in the
+			    // 'added_enum' enum so we assume that
+			    // 'deleted_enum' and 'added_enum' are the same
+			    // enum that got changed.  Let's represent it
+			    // using a diff node.
+			    diff_sptr d = compute_diff(deleted_enum,
+						       added_enum, ctxt);
+			    ABG_ASSERT(d->has_changes());
+			    string repr =
+			      abigail::ir::get_pretty_representation(is_type(deleted_enum),
+								     /*internal=*/false);
+			    changed_unreachable_types_[repr]= d;
+			    this_enum_got_changed = true;
+			    string r1 =
+			      abigail::ir::get_pretty_representation(is_type(deleted_enum),
+								     /*internal=*/false);
+			    string r2 =
+			      abigail::ir::get_pretty_representation(is_type(added_enum),
+								     /*internal=*/false);
+			    removed_anon_types_to_erase[r1] = deleted_enum;
+			    added_anon_types_to_erase[r2] = added_enum;
+			    break;
+			  }
 		    }
+		  if (this_enum_got_changed)
+		    break;
+		}
+	    }
+	  else if (deleted_class)
+	    {
+	      // For unions, structs & classes, look for any data
+	      // member of 'deleted_class' that is also present in an
+	      // added anonymous class.
+	      for (auto dm : deleted_class->get_data_members())
+		{
+		  bool this_class_got_changed = false;
+		  for (auto klass : added_anon_types)
+		    {
+		      if (class_or_union_sptr added_class =
+			  is_class_or_union_type(klass))
+			if (class_or_union_types_of_same_kind(deleted_class,
+							      added_class)
+			    && lookup_data_member(added_class, dm))
+			  {
+			    // So the data member 'dm' from the
+			    // 'deleted_class' class is also present in
+			    // the 'added_class' class so we assume that
+			    // 'deleted_class' and 'added_class' are the
+			    // same anonymous class that got changed.
+			    // Let's represent it using a diff node.
+			    diff_sptr d = compute_diff(is_type(deleted_class),
+						       is_type(added_class),
+						       ctxt);
+			    ABG_ASSERT(d->has_changes());
+			    string repr =
+			      abigail::ir::get_pretty_representation(is_type(deleted_class),
+								     /*internal=*/false);
+			    changed_unreachable_types_[repr]= d;
+			    this_class_got_changed = true;
+			    string r1 =
+			      abigail::ir::get_pretty_representation(is_type(deleted_class),
+								     /*internal=*/false);
+			    string r2 =
+			      abigail::ir::get_pretty_representation(is_type(added_class),
+								     /*internal=*/false);
+			    removed_anon_types_to_erase[r1] = deleted_class;
+			    added_anon_types_to_erase[r2] = added_class;
+			    break;
+			  }
+		    }
+		  if (this_class_got_changed)
+		    break;
 		}
-	      if (this_enum_got_changed)
-		break;
 	    }
 	}
 
-      // Now remove the added/removed anonymous enums from their maps,
-      // as they are now represented as a changed enum, not an added
-      // and removed enum.
-
-      for (auto entry : added_anon_enum_to_erase)
+      // Now remove the added/removed anonymous types from their maps,
+      // as they are now represented as a changed type, not an added
+      // and removed anonymous type.
+      for (auto entry : added_anon_types_to_erase)
 	added_unreachable_types_.erase(entry.first);
 
-      for (auto entry : removed_anon_enum_to_erase)
+      for (auto entry : removed_anon_types_to_erase)
 	deleted_unreachable_types_.erase(entry.first);
     }
   }

src/abg-ir.cc

@@ -6061,6 +6061,47 @@ is_anonymous_data_member(const var_decl& d)
 	  && is_class_or_union_type(d.get_type()));
 }
 
+/// Test if a @ref var_decl is a data member belonging to an anonymous
+/// type.
+///
+/// @param d the @ref var_decl to consider.
+///
+/// @return true iff @p d is a data member belonging to an anonymous
+/// type.
+bool
+is_data_member_of_anonymous_class_or_union(const var_decl& d)
+{
+  if (is_data_member(d))
+    {
+      scope_decl* scope = d.get_scope();
+      if (scope && scope->get_is_anonymous())
+	return true;
+    }
+  return false;
+}
+
+/// Test if a @ref var_decl is a data member belonging to an anonymous
+/// type.
+///
+/// @param d the @ref var_decl to consider.
+///
+/// @return true iff @p d is a data member belonging to an anonymous
+/// type.
+bool
+is_data_member_of_anonymous_class_or_union(const var_decl* d)
+{return is_data_member_of_anonymous_class_or_union(*d);}
+
+/// Test if a @ref var_decl is a data member belonging to an anonymous
+/// type.
+///
+/// @param d the @ref var_decl to consider.
+///
+/// @return true iff @p d is a data member belonging to an anonymous
+/// type.
+bool
+is_data_member_of_anonymous_class_or_union(const var_decl_sptr& d)
+{return is_data_member_of_anonymous_class_or_union(d.get());}
+
 /// Get the @ref class_or_union type of a given anonymous data member.
 ///
 /// @param d the anonymous data member to consider.
@@ -10791,6 +10832,36 @@ shared_ptr<class_or_union>
 is_class_or_union_type(const shared_ptr<type_or_decl_base>& t)
 {return dynamic_pointer_cast<class_or_union>(t);}
 
+/// Test if two class or union types are of the same kind.
+///
+/// @param first the first type to consider.
+///
+/// @param second the second type to consider.
+///
+/// @return true iff @p first is of the same kind as @p second.
+bool
+class_or_union_types_of_same_kind(const class_or_union* first,
+				  const class_or_union* second)
+{
+  if ((is_class_type(first) && is_class_type(second))
+      || (is_union_type(first) && is_union_type(second)))
+    return true;
+
+  return false;
+}
+
+/// Test if two class or union types are of the same kind.
+///
+/// @param first the first type to consider.
+///
+/// @param second the second type to consider.
+///
+/// @return true iff @p first is of the same kind as @p second.
+bool
+class_or_union_types_of_same_kind(const class_or_union_sptr& first,
+				  const class_or_union_sptr& second)
+{return class_or_union_types_of_same_kind(first.get(), second.get());}
+
 /// Test if a type is a @ref union_decl.
 ///
 /// @param t the type to consider.
@@ -27430,6 +27501,27 @@ lookup_data_member(const type_base* type,
   return cou->find_data_member(dm_name).get();
 }
 
+/// Look for a data member of a given class, struct or union type and
+/// return it.
+///
+/// The data member is designated by its name.
+///
+/// @param type the class, struct or union type to consider.
+///
+/// @param dm the data member to lookup.
+///
+/// @return the data member iff it was found in @type or NULL if no
+/// data member with that name was found.
+const var_decl_sptr
+lookup_data_member(const type_base_sptr& type, const var_decl_sptr& dm)
+{
+  class_or_union_sptr cou = is_class_or_union_type(type);
+  if (!cou)
+    return var_decl_sptr();
+
+  return cou->find_data_member(dm);
+}
+
 /// Get the function parameter designated by its index.
 ///
 /// Note that the first function parameter has index 0.

src/abg-reporter-priv.cc

@@ -402,8 +402,12 @@ represent(const var_diff_sptr	&diff,
   const bool o_anon = !!is_anonymous_data_member(o);
   const bool n_anon = !!is_anonymous_data_member(n);
   const bool is_strict_anonymous_data_member_change = o_anon && n_anon;
-  const string o_name = o->get_qualified_name();
-  const string n_name = n->get_qualified_name();
+  const string o_name = (is_data_member_of_anonymous_class_or_union(o)
+			 ? o->get_name()
+			 : o->get_qualified_name());
+  const string n_name = (is_data_member_of_anonymous_class_or_union(n)
+			 ? n->get_name()
+			 : n->get_qualified_name());
   const uint64_t o_size = get_var_size_in_bits(o);
   const uint64_t n_size = get_var_size_in_bits(n);
   const uint64_t o_offset = get_data_member_offset(o);

tests/data/Makefile.am

@@ -362,6 +362,11 @@ test-abidiff-exit/test-anonymous-enums-change-v0.c \
 test-abidiff-exit/test-anonymous-enums-change-v0.o \
 test-abidiff-exit/test-anonymous-enums-change-v1.c \
 test-abidiff-exit/test-anonymous-enums-change-v1.o \
+test-abidiff-exit/test-anon-types-report-1.txt \
+test-abidiff-exit/test-anon-types-v0.c \
+test-abidiff-exit/test-anon-types-v0.o \
+test-abidiff-exit/test-anon-types-v1.c \
+test-abidiff-exit/test-anon-types-v1.o \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\

tests/data/test-abidiff-exit/test-anon-types-report-1.txt

@@ -0,0 +1,31 @@
+Functions changes summary: 0 Removed, 0 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+Unreachable types summary: 2 removed, 3 changed, 0 added types
+
+2 removed types unreachable from any public interface:
+
+  [D] 'struct {char z; int w;}' at test-anon-types-v0.c:34:1
+  [D] 'union {char x; unsigned int y;}' at test-anon-types-v0.c:28:1
+
+3 changed types unreachable from any public interface:
+
+  [C] 'struct {int a; int b;}' changed:
+    type size changed from 64 to 96 (in bits)
+    1 data member insertion:
+      'int c', at offset 64 (in bits) at test-anon-types-v1.c:18:1
+
+  [C] 'enum {a=0, b=1, c=2, d=3, }' changed:
+    type size hasn't changed
+    2 enumerator insertions:
+      'e' value '4'
+      'f' value '5'
+
+  [C] 'union {int a; int b; int d;}' changed:
+    type size hasn't changed
+    1 data member change:
+      name of 'd' changed to 'c' at test-anon-types-v1.c:11:1
+    type changed from:
+      union {int a; int b; int d;}
+    to:
+      union {int a; int b; int c;}
+

tests/data/test-abidiff-exit/test-anon-types-v0.c

@@ -0,0 +1,42 @@
+/**
+ * Compile with:
+ *
+ * gcc -c -g -fno-eliminate-unused-debug-types test-anon-types-v{0,1}.c
+ */
+
+union
+{
+  int a;
+  int b;
+  int d;
+};
+
+struct
+{
+  int a;
+  int b;
+};
+
+enum
+{
+  a,
+  b,
+  c,
+  d
+};
+
+union
+{
+  char x;
+  unsigned y;
+};
+
+struct
+{
+  char z;
+  int w;
+};
+
+void
+fun()
+{}

tests/data/test-abidiff-exit/test-anon-types-v1.c

@@ -0,0 +1,33 @@
+/**
+ * Compile with:
+ *
+ * gcc -c -g -fno-eliminate-unused-debug-types test-anon-types-v{0,1}.c
+ */
+
+union
+{
+  int a;
+  int b;
+  int c;
+};
+
+struct
+{
+  int a;
+  int b;
+  int c;
+};
+
+enum
+{
+  a,
+  b,
+  c,
+  d,
+  e,
+  f
+};
+
+void
+fun()
+{}

tests/test-abidiff-exit.cc

@@ -1064,6 +1064,22 @@ InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test-anonymous-enums-change-report-v1.txt",
     "output/test-abidiff-exit/test-anonymous-enums-change-report-v1.txt"
   },
+  {
+    "data/test-abidiff-exit/test-anon-types-v0.o",
+    "data/test-abidiff-exit/test-anon-types-v1.o",
+    "",
+    "",
+    "",
+    "",
+    "",
+    "",
+    "",
+    "--no-default-suppression --harmless --non-reachable-types",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE
+    | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
+    "data/test-abidiff-exit/test-anon-types-report-1.txt",
+    "output/test-abidiff-exit/test-anon-types-report-1.txt"
+  },
 #ifdef WITH_BTF
   {
     "data/test-abidiff-exit/btf/test0-v0.o",