comparison: Better sort function difference report

Looking at the result of builds on various platforms, it appears that
the order of the method change reports in the
test29-vtable-changes-report-0.txt of runtestdiffdwarf was not stable
across all platforms. debian-armhf was exhibiting this change in
output, for instance:

    --- /var/lib/buildbot/workers/wildebeest/libabigail-try-debian-armhf/build/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt
	2024-03-04 08:56:42.307366590 +0000
    +++ /var/lib/buildbot/workers/wildebeest/libabigail-try-debian-armhf/build/tests/output/test-diff-dwarf/test29-vtable-changes-report-0.txt
	2024-03-04 09:20:22.146334907 +0000
    @@ -19,13 +19,13 @@
		 implicit parameter 0 of type 'S* const' has sub-type
	changes:
		   in unqualified underlying type 'S*':
		     pointed to type 'struct S' changed, as being reported
    +          'method virtual S::~S(int)' has some sub-type changes:
    +            implicit parameter 0 of type 'S*' has sub-type changes:
    +              pointed to type 'struct S' changed, as being reported
	       'method virtual S::~S()' has some sub-type changes:
		 implicit parameter 0 of type 'S* const' has sub-type
	changes:
		   in unqualified underlying type 'S*':
		     pointed to type 'struct S' changed, as being reported
    -          'method virtual S::~S(int)' has some sub-type changes:
    -            implicit parameter 0 of type 'S*' has sub-type changes:
    -              pointed to type 'struct S' changed, as being reported
	       'method virtual void S::fn0()' has some sub-type changes:
		 implicit parameter 0 of type 'S*' has sub-type changes:
		   pointed to type 'struct S' changed, as being reported

Better handling the sorting of function changes should hopefully fix
this issue.

	* src/abg-comparison-priv.h (is_less_than): Declare new helper
	function.
	(function_decl_diff_comp::operator(const function_decl_diff&,
	                                   const function_decl_diff&)):
	Use it here.
	(virtual_member_function_diff_comp::operator(const function_decl_diff&,
	                                             const function_decl_diff&)):
	Likewise.
	* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust.
	* tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt:
	Adjust.
	* tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt:
	Adjust.
	* tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt:
	Adjust.
	* tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt:
	Adjust.

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

diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h
index 434e62673bfbe216dab1cbe6aa3e59e0fdb570ff..adc1a23c3184c9a6ca89dcf1fc14b328b32af79a 100644 (file)
--- a/src/abg-comparison-priv.h
+++ b/src/abg-comparison-priv.h
@@ -774,6 +774,9 @@ struct data_member_diff_comp
   }
 }; // end struct var_diff_comp
 
+bool
+is_less_than(const function_decl_diff& first, const function_decl_diff& second);
+
 /// A comparison functor for instances of @ref function_decl_diff that
 /// represent changes between two virtual member functions.
 struct virtual_member_function_diff_comp
@@ -785,8 +788,12 @@ struct virtual_member_function_diff_comp
     ABG_ASSERT(get_member_function_is_virtual(l.first_function_decl()));
     ABG_ASSERT(get_member_function_is_virtual(r.first_function_decl()));
 
-    return (get_member_function_vtable_offset(l.first_function_decl())
-           < get_member_function_vtable_offset(r.first_function_decl()));
+    size_t l_offset = get_member_function_vtable_offset(l.first_function_decl());
+    size_t r_offset = get_member_function_vtable_offset(r.first_function_decl());
+    if (l_offset != r_offset)
+      return l_offset < r_offset;
+
+    return is_less_than(l, r);
   }
 
   bool
@@ -1257,30 +1264,7 @@ struct function_decl_diff_comp
   operator()(const function_decl_diff& first,
             const function_decl_diff& second)
   {
-    function_decl_sptr f = first.first_function_decl(),
-      s = second.first_function_decl();
-
-    string fr = f->get_qualified_name(),
-      sr = s->get_qualified_name();
-
-    if (fr == sr)
-      {
-       if (f->get_symbol())
-         fr = f->get_symbol()->get_id_string();
-       else if (!f->get_linkage_name().empty())
-         fr = f->get_linkage_name();
-       else
-         fr = f->get_pretty_representation();
-
-       if (s->get_symbol())
-         sr = s->get_symbol()->get_id_string();
-       else if (!s->get_linkage_name().empty())
-         sr = s->get_linkage_name();
-       else
-         sr = s->get_pretty_representation();
-      }
-
-    return (fr.compare(sr) < 0);
+    return is_less_than(first, second);
   }
 
   /// The actual less than operator.

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 1af10529433afd68ca9ce31263bcfa761405dd3e..bd7c224e8a3fff5a66e619e8417ccd8f5fede5fa 100644 (file)
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -168,6 +168,47 @@ sort_changed_data_members(changed_var_sptrs_type& to_sort)
   std::sort(to_sort.begin(), to_sort.end(), comp);
 }
 
+/// Compare two @ref function_decl_diff for the purpose of sorting.
+///
+/// @param first the first @ref function_decl_diff to consider.
+///
+/// @param second the second @ref function_decl_diff to consider.
+///
+/// @return true iff @p first compares less than @p second.
+bool
+is_less_than(const function_decl_diff& first, const function_decl_diff& second)
+{
+  function_decl_sptr f = first.first_function_decl(),
+    s = second.first_function_decl();
+
+  string fr = f->get_qualified_name(), sr = s->get_qualified_name();
+
+  if (fr != sr)
+    return fr < sr;
+
+  if (!f->get_linkage_name().empty()
+      && !s->get_linkage_name().empty())
+    {
+      fr = f->get_linkage_name();
+      sr = s->get_linkage_name();
+      if (fr != sr)
+       return fr < sr;
+    }
+
+  if (f->get_symbol() && s->get_symbol())
+    {
+      fr = f->get_symbol()->get_id_string();
+      sr = s->get_symbol()->get_id_string();
+      if (fr != sr)
+       return fr < sr;
+    }
+       
+  fr = f->get_pretty_representation(true, true);
+  sr = s->get_pretty_representation(true, true);
+
+  return fr < sr;
+}
+
 /// Sort an instance of @ref string_function_ptr_map map and stuff a
 /// resulting sorted vector of pointers to function_decl.
 ///

diff --git a/tests/data/test-abidiff/test-PR18791-report0.txt b/tests/data/test-abidiff/test-PR18791-report0.txt
index 18cb0cf95838301f1964f4e032cf579444072f82..4d2a3c86d971bda66a11c6944397d9ea4a519a0f 100644 (file)
--- a/tests/data/test-abidiff/test-PR18791-report0.txt
+++ b/tests/data/test-abidiff/test-PR18791-report0.txt
@@ -54,16 +54,16 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
       in referenced type 'const sigc::connection':
         unqualified underlying type 'struct sigc::connection' changed, as reported earlier
 
-  [C] 'method sigc::connection::connection()' has some indirect sub-type changes:
-    implicit parameter 0 of type 'sigc::connection*' has sub-type changes:
-      pointed to type 'struct sigc::connection' changed, as reported earlier
-
   [C] 'method sigc::connection::connection(sigc::slot_base&)' has some indirect sub-type changes:
     implicit parameter 0 of type 'sigc::connection*' has sub-type changes:
       pointed to type 'struct sigc::connection' changed, as reported earlier
     parameter 1 of type 'sigc::slot_base&' has sub-type changes:
       referenced type 'class sigc::slot_base' changed, as reported earlier
 
+  [C] 'method sigc::connection::connection()' has some indirect sub-type changes:
+    implicit parameter 0 of type 'sigc::connection*' has sub-type changes:
+      pointed to type 'struct sigc::connection' changed, as reported earlier
+
   [C] 'method void sigc::connection::disconnect()' has some indirect sub-type changes:
     implicit parameter 0 of type 'sigc::connection*' has sub-type changes:
       pointed to type 'struct sigc::connection' changed, as reported earlier
@@ -215,10 +215,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
       in referenced type 'const sigc::signal_base':
         unqualified underlying type 'struct sigc::signal_base' changed, as reported earlier
 
-  [C] 'method sigc::signal_base::signal_base()' has some indirect sub-type changes:
-    implicit parameter 0 of type 'sigc::signal_base*' has sub-type changes:
-      pointed to type 'struct sigc::signal_base' changed, as reported earlier
-
   [C] 'method sigc::signal_base::signal_base(const sigc::signal_base&)' has some indirect sub-type changes:
     implicit parameter 0 of type 'sigc::signal_base*' has sub-type changes:
       pointed to type 'struct sigc::signal_base' changed, as reported earlier
@@ -226,6 +222,10 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
       in referenced type 'const sigc::signal_base':
         unqualified underlying type 'struct sigc::signal_base' changed, as reported earlier
 
+  [C] 'method sigc::signal_base::signal_base()' has some indirect sub-type changes:
+    implicit parameter 0 of type 'sigc::signal_base*' has sub-type changes:
+      pointed to type 'struct sigc::signal_base' changed, as reported earlier
+
   [C] 'method sigc::signal_base::size_type sigc::signal_base::size()' has some indirect sub-type changes:
     implicit parameter 0 of type 'const sigc::signal_base*' has sub-type changes:
       in pointed to type 'const sigc::signal_base':
@@ -245,10 +245,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
     parameter 1 of type 'sigc::slot_base::rep_type*' has sub-type changes:
       pointed to type 'typedef sigc::slot_base::rep_type' changed, as reported earlier
 
-  [C] 'method sigc::slot_base::slot_base()' has some indirect sub-type changes:
-    implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes:
-      pointed to type 'class sigc::slot_base' changed, as reported earlier
-
   [C] 'method sigc::slot_base::slot_base(const sigc::slot_base&)' has some indirect sub-type changes:
     implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes:
       pointed to type 'class sigc::slot_base' changed, as reported earlier
@@ -256,6 +252,10 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
       in referenced type 'const sigc::slot_base':
         unqualified underlying type 'class sigc::slot_base' changed, as reported earlier
 
+  [C] 'method sigc::slot_base::slot_base()' has some indirect sub-type changes:
+    implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes:
+      pointed to type 'class sigc::slot_base' changed, as reported earlier
+
   [C] 'method sigc::slot_base::~slot_base(int)' has some indirect sub-type changes:
     implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes:
       pointed to type 'class sigc::slot_base' changed, as reported earlier

diff --git a/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt b/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt
index d9873f9bb1511bb1af69235e9d86fa1a49e9ac8d..780d59457cf079df6285f414ca65c0bad8d45056 100644 (file)
--- a/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt
+++ b/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt
@@ -15,6 +15,9 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
         1 member function insertion:
           'method virtual void S::fn1()', virtual at voffset 3/3
         4 member function changes:
+          'method virtual void S::fn0()' has some sub-type changes:
+            implicit parameter 0 of type 'S*' has sub-type changes:
+              pointed to type 'struct S' changed, as being reported
           'method virtual S::~S()' has some sub-type changes:
             implicit parameter 0 of type 'S* const' has sub-type changes:
               in unqualified underlying type 'S*':
@@ -26,9 +29,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
           'method virtual S::~S(int)' has some sub-type changes:
             implicit parameter 0 of type 'S*' has sub-type changes:
               pointed to type 'struct S' changed, as being reported
-          'method virtual void S::fn0()' has some sub-type changes:
-            implicit parameter 0 of type 'S*' has sub-type changes:
-              pointed to type 'struct S' changed, as being reported
 
   [C] 'method virtual S::~S()' has some indirect sub-type changes:
     implicit parameter 0 of type 'S* const' has sub-type changes:

diff --git a/tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt b/tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt
index e75be9413ed1bcedb15484ebf230b01fcd654fd4..d32f99b2f2cae1981efb713507cefc87ba219cf8 100644 (file)
--- a/tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt
+++ b/tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt
@@ -15,6 +15,14 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
         1 member function insertion:
           'method virtual void S::fvtable_breaker()', virtual at voffset 3/4
         5 member function changes:
+          'method virtual void S::fn0()' has some sub-type changes:
+            implicit parameter 0 of type 'S*' has sub-type changes:
+              pointed to type 'struct S' changed, as being reported
+          'method virtual void S::fn1()' has some sub-type changes:
+            the vtable offset of method virtual void S::fn1() changed from 3 to 4
+              note that this is an ABI incompatible change to the vtable of struct S
+            implicit parameter 0 of type 'S*' has sub-type changes:
+              pointed to type 'struct S' changed, as being reported
           'method virtual S::~S()' has some sub-type changes:
             implicit parameter 0 of type 'S* const' has sub-type changes:
               in unqualified underlying type 'S*':
@@ -26,14 +34,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
           'method virtual S::~S(int)' has some sub-type changes:
             implicit parameter 0 of type 'S*' has sub-type changes:
               pointed to type 'struct S' changed, as being reported
-          'method virtual void S::fn0()' has some sub-type changes:
-            implicit parameter 0 of type 'S*' has sub-type changes:
-              pointed to type 'struct S' changed, as being reported
-          'method virtual void S::fn1()' has some sub-type changes:
-            the vtable offset of method virtual void S::fn1() changed from 3 to 4
-              note that this is an ABI incompatible change to the vtable of struct S
-            implicit parameter 0 of type 'S*' has sub-type changes:
-              pointed to type 'struct S' changed, as being reported
 
   [C] 'method virtual void S::fn1()' has some indirect sub-type changes:
     the vtable offset of method virtual void S::fn1() changed from 3 to 4

diff --git a/tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt b/tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt
index fa904b7cd7024fbad3ae687884354cfd19cb0f5f..dcfde0f5ffc591465300b9f186e5ceb1d2b2119d 100644 (file)
--- a/tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt
+++ b/tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt
@@ -17,6 +17,11 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
         1 member function deletion:
           'method virtual void S::fn0()', virtual at voffset 2/3
         4 member function changes:
+          'method virtual void S::fn1()' has some sub-type changes:
+            the vtable offset of method virtual void S::fn1() changed from 3 to 2
+              note that this is an ABI incompatible change to the vtable of struct S
+            implicit parameter 0 of type 'S*' has sub-type changes:
+              pointed to type 'struct S' changed, as being reported
           'method virtual S::~S()' has some sub-type changes:
             implicit parameter 0 of type 'S* const' has sub-type changes:
               in unqualified underlying type 'S*':
@@ -28,11 +33,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
           'method virtual S::~S(int)' has some sub-type changes:
             implicit parameter 0 of type 'S*' has sub-type changes:
               pointed to type 'struct S' changed, as being reported
-          'method virtual void S::fn1()' has some sub-type changes:
-            the vtable offset of method virtual void S::fn1() changed from 3 to 2
-              note that this is an ABI incompatible change to the vtable of struct S
-            implicit parameter 0 of type 'S*' has sub-type changes:
-              pointed to type 'struct S' changed, as being reported
 
   [C] 'method virtual S::~S()' has some indirect sub-type changes:
     implicit parameter 0 of type 'S* const' has sub-type changes:

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 17a1662e739e0f83a35b40110c123c7831028a38..0a69585691b1aac895bc493e38fc99b0443492aa 100644 (file)
--- a/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt
+++ b/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt
@@ -11,6 +11,14 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
           1 member function insertion:
             'method virtual void Interface::method2()', virtual at voffset 3/4
           5 member function changes:
+            'method virtual void Interface::method1()' has some sub-type changes:
+              implicit parameter 0 of type 'Interface*' has sub-type changes:
+                pointed to type 'class Interface' changed, as being reported
+            '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
+              implicit parameter 0 of type 'Interface*' has sub-type changes:
+                pointed to type 'class Interface' changed, as being reported
             'method virtual Interface::~Interface()' has some sub-type changes:
               implicit parameter 0 of type 'Interface* const' has sub-type changes:
                 in unqualified underlying type 'Interface*':
@@ -22,14 +30,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
             'method virtual Interface::~Interface(int)' has some sub-type changes:
               implicit parameter 0 of type 'Interface*' has sub-type changes:
                 pointed to type 'class Interface' changed, as being reported
-            'method virtual void Interface::method1()' has some sub-type changes:
-              implicit parameter 0 of type 'Interface*' has sub-type changes:
-                pointed to type 'class Interface' changed, as being reported
-            '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
-              implicit parameter 0 of type 'Interface*' has sub-type changes:
-                pointed to type 'class Interface' changed, as being reported
 
   [C] 'method virtual Interface::~Interface()' has some indirect sub-type changes:
     implicit parameter 0 of type 'Interface* const' has sub-type changes: