Fix redundancy filtering of range types

After the support for Ada range types was added, it appeared that
redundancy propagation was not being correctly handled for range
types, especially when those are sub-types of a function parameter
type, like a const range.

This patch goes through the various problematic spots and addresses
the issues.

	* src/abg-comparison.cc (redundancy_marking_visitor::visit_end):
	Propagate redundancy category to function parameter diff nodes if
	they don't carry any local non-type change.
	* src/abg-default-reporter.cc
	(default_reporter::report_underlying_changes_of_qualified_type):
	Define new member function.
	(default_reporter::report): In the qualified_type_diff overload,
	use the new report_underlying_changes_of_qualified_type above.
	* src/abg-ir.cc (types_have_similar_structure): If two arrays are
	accessed indirectly and if they have size and dimension changes,
	then the two arrays are considered having a similar structure.
	Otherwise, if they are accessed directly, having size or dimension
	change make them considered as having non similar structure.  This
	has an impact on if a change between two array types is considered
	local or not.
	* src/abg-leaf-reporter.cc (leaf_reporter::report): Local changes
	to underlying types of a qualified type are considered local to
	the qualified type.  This change reflects that in the overload for
	qualified type diff nodes.  Otherwise, we won't report what would
	otherwise be a leaf change to the a qualified type, just because
	it's actually a leaf change to the underlying type of the
	qualified type.
	* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-{1,2}.txt:
	New reference output files.
	* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ad{b,s}:
	Source code for the new binary input below.
	* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o:
	New binary input file.
	* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ad{b,s}:
	Source code for the new binary input below.
	* tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o:
	New binary input file.
	* tests/data/Makefile.am: Add the new test input files above to
	source distribution.
	* tests/test-abidiff-exit.cc (in_out_specs): Add the new input
	tests above to this test harness.
	* tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt:
	Adjust.

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

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index c4596c8e7d4e4a763ba132bccab562958a7c65c8..59530a8abfa57b6bfb677c29db655d6c9f4c28e7 100644 (file)
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -12749,6 +12749,13 @@ struct redundancy_marking_visitor : public diff_node_visitor
                || (is_var_diff(d)
                    && (!(d->has_local_changes()
                          & LOCAL_NON_TYPE_CHANGE_KIND)))
+               // A function parameter with non-type local changes
+               // should not see redundancy propagation either.  But
+               // a function parameter with local type changes can
+               // definitely be redundant.
+               || (is_fn_parm_diff(d)
+                   && (!(d->has_local_changes()
+                         & LOCAL_NON_TYPE_CHANGE_KIND)))
                ))
          {
            bool has_non_redundant_child = false;

diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index 31832c3fec387d4b38406492b5e610b6d642bb12..b1df9300035e4713abcbac9e189808dd3b09c4ad 100644 (file)
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -348,6 +348,38 @@ default_reporter::report_local_qualified_type_changes(const qualified_type_diff&
   return false;
 }
 
+/// For a @ref qualified_type_diff node, report the changes of its
+/// underlying type.
+///
+/// @param d the @ref qualified_type_diff node to consider.
+///
+/// @param out the output stream to emit the report to.
+///
+/// @param indent the white string to use for indentation.
+///
+/// @return true iff a local change has been emitted.  In this case,
+/// the local change is a name change.
+void
+default_reporter::report_underlying_changes_of_qualified_type
+(const qualified_type_diff& d, ostream& out, const string& indent) const
+{
+  if (!d.to_be_reported())
+    return;
+
+  diff_sptr dif = d.leaf_underlying_type_diff();
+  ABG_ASSERT(dif);
+  ABG_ASSERT(dif->to_be_reported());
+  RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER2(dif,
+                                                   "unqualified "
+                                                   "underlying type");
+
+  string fltname = dif->first_subject()->get_pretty_representation();
+  out << indent << "in unqualified underlying type '" << fltname << "'";
+  report_loc_info(dif->second_subject(), *d.context(), out);
+  out << ":\n";
+  dif->report(out, indent + "  ");
+}
+
 /// Report a @ref qualified_type_diff in a serialized form.
 ///
 /// @param d the @ref qualified_type_diff node to consider.
@@ -372,18 +404,7 @@ default_reporter::report(const qualified_type_diff& d, ostream& out,
       // It makes a little sense to detail the changes in extenso here.
       return;
 
-  diff_sptr dif = d.leaf_underlying_type_diff();
-  ABG_ASSERT(dif);
-  ABG_ASSERT(dif->to_be_reported());
-  RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER2(dif,
-                                                   "unqualified "
-                                                   "underlying type");
-
-  string fltname = dif->first_subject()->get_pretty_representation();
-  out << indent << "in unqualified underlying type '" << fltname << "'";
-  report_loc_info(dif->second_subject(), *d.context(), out);
-  out << ":\n";
-  dif->report(out, indent + "  ");
+  report_underlying_changes_of_qualified_type(d, out, indent);
 }
 
 /// Report the @ref pointer_diff in a serialized form.

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index e0d4f327199d70215da62bf485de4fd95835445a..ce55365395edae27e0eb2ae56b217f7888445740 100644 (file)
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -26579,11 +26579,16 @@ types_have_similar_structure(const type_base* first,
     {
       const array_type_def* ty2 = is_array_type(second);
       // TODO: Handle int[5][2] vs int[2][5] better.
-      if (ty1->get_size_in_bits() != ty2->get_size_in_bits()
-         || ty1->get_dimension_count() != ty2->get_dimension_count()
-         || !types_have_similar_structure(ty1->get_element_type(),
-                                          ty2->get_element_type(),
-                                          /*indirect_type=*/true))
+      if (!indirect_type)
+       {
+         if (ty1->get_size_in_bits() != ty2->get_size_in_bits()
+             || ty1->get_dimension_count() != ty2->get_dimension_count())
+           return false;
+       }
+
+      if (!types_have_similar_structure(ty1->get_element_type(),
+                                       ty2->get_element_type(),
+                                       /*indirect_type=*/true))
        return false;
 
       return true;

diff --git a/src/abg-leaf-reporter.cc b/src/abg-leaf-reporter.cc
index 2006d4ff4c42fa9fd9180aef0dbcb2972c6fd1a2..10d65054e65e005650bad119937da517e7609f04 100644 (file)
--- a/src/abg-leaf-reporter.cc
+++ b/src/abg-leaf-reporter.cc
@@ -210,6 +210,12 @@ leaf_reporter::report(const qualified_type_diff& d, ostream& out,
     return;
 
   report_local_qualified_type_changes(d, out, indent);
+
+  // Note that changes that are local to the underlying type of a
+  // qualified type are considered to be local to the qualified type
+  // itself.  So let's go ahead and report the local changes of the
+  // underlying type.
+  report_underlying_changes_of_qualified_type(d, out, indent);
 }
 
 /// Report the changes carried by a @ref pointer_diff node.

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index c91c24fb03c4d3d57f5ef503ba2da1c52b3aef71..6aab630b2f67ab71ec0f5de7a12876681f82c7ed 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -306,6 +306,14 @@ test-abidiff-exit/ada-subrange/test1-ada-subrange/v0/test1.o \
 test-abidiff-exit/ada-subrange/test1-ada-subrange/v1/test1.adb \
 test-abidiff-exit/ada-subrange/test1-ada-subrange/v1/test1.ads \
 test-abidiff-exit/ada-subrange/test1-ada-subrange/v1/test1.o \
+test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt \
+test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt \
+test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.adb \
+test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ads \
+test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o \
+test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.adb \
+test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ads \
+test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o \
 \
 test-diff-dwarf/test0-v0.cc            \
 test-diff-dwarf/test0-v0.o                     \

diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt
new file mode 100644 (file)
index 0000000..742dda8
--- /dev/null
+++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt
@@ -0,0 +1,17 @@
+Functions changes summary: 0 Removed, 2 Changed, 0 Added functions
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+2 functions with some indirect sub-type change:
+
+  [C] 'function test__my_int[101] test__first_function(test__my_int[101]&)' at test.adb:6:1 has some indirect sub-type changes:
+    return type changed:
+      type name changed from 'test__my_int[101]' to 'test__my_int[201]'
+      array type size changed from 101000 to 201000
+      array type subrange 1 changed length from 101 to 201
+
+  [C] 'function test__my_index test__second_function(const test__my_index)' at test.adb:12:1 has some indirect sub-type changes:
+    return type changed:
+      upper bound of range 'test__my_index' change from '100' to '200'
+      underlying type of range '<range test__my_index>[101]' changed:
+        type size changed from 8 to 16 (in bits)
+

diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt
new file mode 100644 (file)
index 0000000..45e1c52
--- /dev/null
+++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt
@@ -0,0 +1,24 @@
+Leaf changes summary: 4 artifacts changed
+Changed leaf types summary: 2 leaf types changed
+Removed/Changed/Added functions summary: 0 Removed, 2 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+2 functions with some sub-type change:
+
+  [C] 'function test__my_int[101] test__first_function(test__my_int[101]&)' at test.adb:6:1 has some sub-type changes:
+    return type changed:
+      type name changed from 'test__my_int[101]' to 'test__my_int[201]'
+      array type size changed from 101000 to 201000
+      array type subrange 1 changed length from 101 to 201
+
+  [C] 'function test__my_index test__second_function(const test__my_index)' at test.adb:12:1 has some sub-type changes:
+    return type changed:
+      upper bound of range 'test__my_index' change from '100' to '200'
+    parameter 1 of type 'const test__my_index' changed:
+      in unqualified underlying type '<range test__my_index>[101]':
+        upper bound of range 'test__my_index' change from '100' to '200'
+
+'test__Tmy_indexB' changed:
+  type size changed from 8 to 16 (in bits)
+'<range test__my_index>[101]' changed:
+  upper bound of range 'test__my_index' change from '100' to '200'

diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.adb b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.adb
new file mode 100644 (file)
index 0000000..98b0315
--- /dev/null
+++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.adb
@@ -0,0 +1,17 @@
+-- Compile this file with:
+--   gcc -g -c test.adb
+
+package body Test is
+
+function First_Function (A: My_Int_Array) return My_Int_Array is
+begin
+  return A;
+end First_Function;
+
+
+function Second_Function (A: My_Index) return My_Index is
+begin
+  return My_Index'Last;
+end Second_Function;
+
+end Test;

diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ads b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ads
new file mode 100644 (file)
index 0000000..66a57f8
--- /dev/null
+++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.ads
@@ -0,0 +1,11 @@
+package Test is
+
+type My_Int is range 0 .. 1000;
+type My_Index is range 0 .. 100;
+type My_Int_Array is array (My_Index) of My_Int;
+
+function First_Function (A: My_Int_Array) return My_Int_Array;
+
+function Second_Function (A: My_Index) return My_Index;
+
+end Test;
\ No newline at end of file

diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o
new file mode 100644 (file)
index 0000000..483ab7c
Binary files /dev/null and b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o differ

diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.adb b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.adb
new file mode 100644 (file)
index 0000000..98b0315
--- /dev/null
+++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.adb
@@ -0,0 +1,17 @@
+-- Compile this file with:
+--   gcc -g -c test.adb
+
+package body Test is
+
+function First_Function (A: My_Int_Array) return My_Int_Array is
+begin
+  return A;
+end First_Function;
+
+
+function Second_Function (A: My_Index) return My_Index is
+begin
+  return My_Index'Last;
+end Second_Function;
+
+end Test;

diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ads b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ads
new file mode 100644 (file)
index 0000000..f2cf0a8
--- /dev/null
+++ b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.ads
@@ -0,0 +1,11 @@
+package Test is
+
+type My_Int is range 0 .. 1000;
+type My_Index is range 0 .. 200;
+type My_Int_Array is array (My_Index) of My_Int;
+
+function First_Function (A: My_Int_Array) return My_Int_Array;
+
+function Second_Function (A: My_Index) return My_Index;
+
+end Test;
\ No newline at end of file

diff --git a/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o
new file mode 100644 (file)
index 0000000..171d6bd
Binary files /dev/null and b/tests/data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o differ

diff --git a/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt b/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt
index 3bea50c981283c45320d70d6a3156ea6cbf4ff07..d8d2cfff8f79382c93f2a52520a9f975674fc927 100644 (file)
--- a/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt
+++ b/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt
@@ -16,6 +16,9 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
   [C] 'function const int foo2(S2&)' at test45-basic-type-change-v1.cc:32:1 has some sub-type changes:
     return type changed:
       'const int' changed to 'const char'
+      in unqualified underlying type 'int':
+        type name changed from 'int' to 'char'
+        type size changed from 32 to 8 (in bits)
 
   [C] 'function int foo3(S2&)' at test45-basic-type-change-v1.cc:36:1 has some sub-type changes:
     return type changed:

diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 22bc9b77a870a114ebc97d9b47d2c57af8884c04..3733dce91a5bba6ab31d80262926542049a16aa9 100644 (file)
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -702,6 +702,28 @@ InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/ada-subrange/test1-ada-subrange/test1-ada-subrange-report-2.txt",
     "output/test-abidiff-exit/ada-subrange/test1-ada-subrange/test1-ada-subrange-report-2.txt"
   },
+  {
+    "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o",
+    "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o",
+    "",
+    "",
+    "",
+    "--no-default-suppression",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt",
+    "output/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-1.txt"
+  },
+    {
+    "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v0/test.o",
+    "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/v1/test.o",
+    "",
+    "",
+    "",
+    "--no-default-suppression --leaf-changes-only",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+    "data/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt",
+    "output/test-abidiff-exit/ada-subrange/test2-ada-subrange-redundant/test2-ada-subrange-redundant-report-2.txt"
+  },
 #ifdef WITH_BTF
   {
     "data/test-abidiff-exit/btf/test0-v0.o",