Skip class types with changed names in leaf reports

In leaf reports, a class type which is said to have changed name
should not be reported because it doesn't make sense there.  In the
full report however, a class sub-type of a type foo changed name is
worth mentioning.

This patch detects name changes of class types in the context of leaf
reports and avoid reporting impacted types.

	* include/abg-comp-filter.h (has_class_or_union_type_name_change)
	(has_basic_or_class_type_name_change): Declare new functions.
	* include/abg-comparison.h (is_diff_of_class_or_union_type):
	Likewise.
	* src/abg-comp-filter.cc (has_class_or_union_type_name_change)
	(has_basic_or_class_type_name_change):
	* src/abg-comparison.cc
	(leaf_diff_node_marker_visitor::visit_begin): Use the new
	filtering::has_basic_or_class_type_name_change to test if a basic
	or class/union diff type carries a name change.  Update comment.
	* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt:
	Adjust.

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

diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h
index 2dc01c7b341b776ba9aac4e8ce8570dbb91b88e9..78d51252e783e956ba205454757f34b2327a282e 100644 (file)
--- a/include/abg-comp-filter.h
+++ b/include/abg-comp-filter.h
@@ -52,6 +52,12 @@ has_class_decl_only_def_change(const class_or_union_sptr& first,
 bool
 has_basic_type_name_change(const diff *);
 
+bool
+has_class_or_union_type_name_change(const diff *d);
+
+bool
+has_basic_or_class_type_name_change(const diff *d);
+
 struct filter_base;
 /// Convenience typedef for a shared pointer to filter_base
 typedef shared_ptr<filter_base> filter_base_sptr;

diff --git a/include/abg-comparison.h b/include/abg-comparison.h
index bf439d1f547ea08d318d96f73ac01e43c402c55b..77e20bc7d03b90b4f6a229ce2995fe16359fb8d0 100644 (file)
--- a/include/abg-comparison.h
+++ b/include/abg-comparison.h
@@ -2616,6 +2616,9 @@ is_decl_diff(const diff* diff);
 const type_decl_diff*
 is_diff_of_basic_type(const diff* diff);
 
+const class_or_union_diff*
+is_diff_of_class_or_union_type(const diff *d);
+
 bool
 has_basic_type_change_only(const diff* diff);
 

diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc
index 8f8a02410dd68f98c5d3e3eb4579d7753b8ac506..89c684dc4bb75766f98c036570c5d08c97b8e8c8 100644 (file)
--- a/src/abg-comp-filter.cc
+++ b/src/abg-comp-filter.cc
@@ -822,6 +822,35 @@ has_basic_type_name_change(const diff *d)
   return false;
 }
 
+/// Test if a diff node carries a class or union type name change.
+///
+/// @param d the diff node to consider.
+///
+/// @return true iff the diff node carries a class or union type name
+/// change.
+bool
+has_class_or_union_type_name_change(const diff *d)
+{
+  if (const class_or_union_diff *dif = is_diff_of_class_or_union_type(d))
+    if (decl_name_changed(dif))
+      return true;
+
+  return false;
+}
+
+/// Test if a diff node carries a basic or class type name change.
+///
+/// @param d the diff node to consider.
+///
+/// @return true iff the diff node carries a basic or class type name
+/// change.
+bool
+has_basic_or_class_type_name_change(const diff *d)
+{
+  return (has_basic_type_name_change(d)
+         || has_class_or_union_type_name_change(d));
+}
+
 /// Test if an enum_diff carries an enumerator insertion.
 ///
 /// @param diff the enum_diff to consider.

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index a38ddd45656713d6b6410b872c44d9ed8deaa816..25f0d95c2bf85dccc316d7689ee1dee51109e9aa 100644 (file)
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -9519,14 +9519,14 @@ struct leaf_diff_node_marker_visitor : public diff_node_visitor
   visit_begin(diff *d)
   {
     if (d->has_local_changes()
-       // A leaf basic type name change makes no sense when showing
-       // just leaf changes.  It only makes sense when it can explain
-       // the details about a non-leaf change.  In other words, it
-       // doesn't make sense to say that an "int" became "unsigned
-       // int".  But it does make sense to says that a typedef
-       // changed because its underlying type was 'int' and is now an
-       // "unsigned int".
-       && !filtering::has_basic_type_name_change(d)
+       // A leaf basic (or class/union) type name change makes no
+       // sense when showing just leaf changes.  It only makes sense
+       // when it can explain the details about a non-leaf change.
+       // In other words, it doesn't make sense to say that an "int"
+       // became "unsigned int".  But it does make sense to say that
+       // a typedef changed because its underlying type was 'int' and
+       // is now an "unsigned int".
+       && !filtering::has_basic_or_class_type_name_change(d)
        // Similarly, a *local* change describing a type that changed
        // its nature doesn't make sense.
        && !is_distinct_diff(d))
@@ -10865,6 +10865,18 @@ const type_decl_diff*
 is_diff_of_basic_type(const diff *d)
 {return dynamic_cast<const type_decl_diff*>(d);}
 
+/// Test if a diff node represents a diff between two class or union
+/// types.
+///
+/// @param d the diff node to consider.
+///
+/// @return iff @p is a diff between two class or union types then
+/// return the instance of @ref class_or_union_diff that @p derives
+/// from.  Otherwise, return nil.
+const class_or_union_diff*
+is_diff_of_class_or_union_type(const diff *d)
+{return dynamic_cast<const class_or_union_diff*>(d);}
+
 /// Test if a diff node is a decl diff that only carries a basic type
 /// change on its type diff sub-node.
 ///

diff --git a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
index 36cf212fe99265150fa8798c61f7ba6847688800..d157e4249d87de83ef51812b7b4c0c51fefb3bca 100644 (file)
--- a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
+++ b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt
@@ -1,5 +1,5 @@
 ================ changes of 'libspice-server.so.1.8.0'===============
-Leaf changes summary: 2 artifacts changed (8 filtered out)
+Leaf changes summary: 2 artifacts changed (7 filtered out)
   Added/removed functions summary: 1 Removed, 8 Added functions
   Added/removed variables summary: 0 Removed, 0 Added variable