Don't consider changes to basic types as being redundant

Suppose we have two types struct A and struct B.  Suppose the two
types have members of integer type.  Suppose there are members of
integer type in struct A that are modified to become members of char
type.  Suppose, at last, that we also have members of integer type in
struct B that are modified to become members of char type.

In that case, we want to see all the changes of members which types
got changed from integer type to char type.  None of these changes
should be considered redundant.

Today, unfortunately, only the first change is reported by default.
The subsequent changes are considered to be redundant.

This patch fixes that by considering that changes that modifies the type of a
decl from a basic type into another are never considered redundant.

	* include/abg-comparison.h (is_diff_of_basic_type)
	(has_basic_type_change_only): Declare these functions ...
	* src/abg-comparison.cc (is_diff_of_basic_type)
	(has_basic_type_change_only): ... and define them.
	(redundancy_marking_visitor::visit_begin): Use the new
	has_basic_type_change_only.
	* tests/data/test-diff-filter/libtest37-v0.so: New binary test input.
	* tests/data/test-diff-filter/libtest37-v1.so: Likewise.
	* tests/data/test-diff-filter/test37-report-0.txt: New test
	reference output.
	* tests/data/test-diff-filter/test37-v0.cc: Source code of the new
	binary test input.
	* tests/data/test-diff-filter/test37-v1.cc: Likewise.
	* tests/data/Makefile.am: Update to add the new test material to
	the source distribution.
	* tests/test-diff-filter.cc (in_out_spec): Add the new test input
	to this test harness.

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

include/abg-comparison.h

@@ -2457,6 +2457,12 @@ is_type_diff(const diff* diff);
 const decl_diff_base*
 is_decl_diff(const diff* diff);
 
+bool
+is_diff_of_basic_type(const diff* diff);
+
+bool
+has_basic_type_change_only(const diff* diff);
+
 const class_diff*
 is_class_diff(const diff* diff);
 

src/abg-comparison.cc

@@ -13363,6 +13363,11 @@ struct redundancy_marking_visitor : public diff_node_visitor
 		    }
 		}
 	    if (!redundant_with_sibling_node
+		// Changes to basic types should never be considered
+		// redundant.  For instance, if a member of integer
+		// type is changed into a char type in both a struct A
+		// and a struct B, we want to see both changes.
+		&& !has_basic_type_change_only(d)
 		// Functions with similar *local* changes are never marked
 		// redundant because otherwise one could miss important
 		// similar local changes that are applied to different
@@ -13675,5 +13680,40 @@ is_diff_of_variadic_parameter(const diff* d)
 bool
 is_diff_of_variadic_parameter(const diff_sptr& d)
 {return is_diff_of_variadic_parameter(d.get());}
+
+/// Test if a diff node represents a diff between two basic types.
+///
+/// @param d the diff node to consider.
+///
+/// @return true iff @p d is a diff between two basic types.
+bool
+is_diff_of_basic_type(const diff *d)
+{return dynamic_cast<const type_decl_diff*>(d);}
+
+/// Test if a diff node is a decl diff that only carries a basic type
+/// change on its type diff sub-node.
+///
+/// @param d the diff node to consider.
+///
+/// @return true iff @p d is a decl diff that only carries a basic
+/// type change on its type diff sub-node.
+bool
+has_basic_type_change_only(const diff *d)
+{
+  if (is_diff_of_basic_type(d) && d->has_changes())
+    return true;
+  else if (const var_diff * v = dynamic_cast<const var_diff*>(d))
+    return (!v->has_local_changes()
+	    && is_diff_of_basic_type(v->type_diff().get()));
+  else if (const fn_parm_diff * p = dynamic_cast<const fn_parm_diff*>(d))
+    return (!p->has_local_changes()
+	    && is_diff_of_basic_type(p->type_diff().get()));
+  else if (const function_decl_diff* f =
+	   dynamic_cast<const function_decl_diff*>(d))
+    return (!f->has_local_changes()
+	    && f->type_diff()
+	    && is_diff_of_basic_type(f->type_diff()->return_type_diff().get()));
+  return false;
+}
 }// end namespace comparison
 } // end namespace abigail

tests/data/Makefile.am

@@ -589,6 +589,11 @@ test-diff-filter/test34-libjemalloc.so.2-intel-16.0.3 \
 test-diff-filter/test34-report-0.txt \
 test-diff-filter/test35-pr18754-no-added-syms-report-0.txt \
 test-diff-filter/test35-pr18754-no-added-syms-report-1.txt \
+test-diff-filter/libtest37-v0.so \
+test-diff-filter/libtest37-v1.so \
+test-diff-filter/test37-report-0.txt \
+test-diff-filter/test37-v0.cc \
+test-diff-filter/test37-v1.cc \
 \
 test-diff-suppr/test0-type-suppr-v0.cc	\
 test-diff-suppr/test0-type-suppr-v1.cc	\

tests/data/test-diff-filter/test37-report-0.txt

@@ -0,0 +1,36 @@
+Functions changes summary: 0 Removed, 3 Changed, 0 Added functions
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+3 functions with some indirect sub-type change:
+
+  [C]'function void f1(A&)' at test37-v1.cc:19:1 has some indirect sub-type changes:
+    parameter 1 of type 'A&' has sub-type changes:
+      in referenced type 'struct A' at test37-v1.cc:3:1:
+        type size changed from 32 to 8 bits
+        1 data member change:
+         type of 'int A::m0' changed:
+           type name changed from 'int' to 'char'
+           type size changed from 32 to 8 bits
+
+
+  [C]'function void f2(B&)' at test37-v1.cc:23:1 has some indirect sub-type changes:
+    parameter 1 of type 'B&' has sub-type changes:
+      in referenced type 'struct B' at test37-v1.cc:8:1:
+        type size changed from 32 to 8 bits
+        1 data member change:
+         type of 'int B::m0' changed:
+           type name changed from 'int' to 'char'
+           type size changed from 32 to 8 bits
+
+
+  [C]'function void f3(C&)' at test37-v1.cc:27:1 has some indirect sub-type changes:
+    parameter 1 of type 'C&' has sub-type changes:
+      in referenced type 'struct C' at test37-v1.cc:13:1:
+        type size changed from 32 to 8 bits
+        1 data member change:
+         type of 'int C::m0' changed:
+           type name changed from 'int' to 'char'
+           type size changed from 32 to 8 bits
+
+
+

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

@@ -0,0 +1,28 @@
+// g++ -g -Wall -shared -o libtest37-v0.so test37-v0.cc
+
+struct A
+{
+  int m0;
+};
+
+struct B
+{
+  int m0;
+};
+
+struct C
+{
+  int m0;
+};
+
+void
+f1(A&)
+{}
+
+void
+f2(B&)
+{}
+
+void
+f3(C&)
+{}

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

@@ -0,0 +1,28 @@
+// g++ -g -Wall -shared -o libtest37-v1.so test37-v1.cc
+
+struct A
+{
+  char m0;
+};
+
+struct B
+{
+  char m0;
+};
+
+struct C
+{
+  char m0;
+};
+
+void
+f1(A&)
+{}
+
+void
+f2(B&)
+{}
+
+void
+f3(C&)
+{}

tests/test-diff-filter.cc

@@ -436,6 +436,13 @@ InOutSpec in_out_specs[] =
     "data/test-diff-filter/test35-pr18754-no-added-syms-report-1.txt",
     "output/test-diff-filter/test35-pr18754-no-added-syms-report-1.txt",
   },
+  {
+    "data/test-diff-filter/libtest37-v0.so",
+    "data/test-diff-filter/libtest37-v1.so",
+    "--no-default-suppression --no-linkage-name",
+    "data/test-diff-filter/test37-report-0.txt",
+    "output/test-diff-filter/test37-report-0.txt",
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL, NULL}
 };