Make abidiff --harmless show harmless changes in unions

Since the previous commit filters out harmless changes inside unions,
this one allows those changes to be reported whenever the user runs
abidiff with the --harmless option.

The patch just displays the "before" and "after" of the union because
some of the harmless changes are not tracked anymore.

Because we want to display the before and after of the union we use
the function get_class_or_union_flat_representation.  The patch adds a
"qualified_name" boolean parameter to that function so that we can
choose to display union members names in a non-qualified fashion,
which is the natural way of displaying those names in the context of a
union (or class) representation.  Because
get_class_or_union_flat_representation uses
type_or_decl_base::get_pretty_representation, the patch has also added a
"qualified_name" boolean parameter to that function so that we can
choose to display names in a non-qualified manner.

	* include/abg-fwd.h (get_class_or_union_flat_representation): Add
	a "qualified_name" boolean parameter.
	* include/abg-ir.h ({type_or_decl_base, decl_base, type_decl,
	namespace_decl, array_type_def::subrange_type, array_type_def,
	enum_type_decl, typedef_decl, var_decl, function_decl,
	function_decl::parameter, function_type, method_type, class_decl,
	union_decl}::get_pretty_representation): Likewise.
	* src/abg-ir.cc ({type_or_decl_base, decl_base, type_decl,
	namespace_decl, array_type_def::subrange_type, array_type_def, enum_type_decl,
	typedef_decl, var_decl, function_decl, function_decl::parameter,
	function_type, method_type, class_decl, union_decl,
	}::get_pretty_representation): Adjust the code to emit qualified
	or non-qualified names depending on the new "qualified_name"
	boolean parameter.
	(get_class_or_union_flat_representation): Likewise.
	* src/abg-default-reporter.cc (default_reporter::report): Use
	get_class_or_union_flat_representation with the new
	"qualified_name" boolean set to false.
	* tests/data/test-diff-dwarf/test38-union-report-0.txt: Adjust.
	* tests/test-diff-filter.cc (in_out_specs): Run the test harness
	on test-PR24731-v{0,1}.o make abidiff use the --harmless option.

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

diff --git a/include/abg-fwd.h b/include/abg-fwd.h
index ddec06d8d29c500b7bf41279dd3a98ef11b59087..529ef1bb2c5a3278479a42b4c51848189fdb4d0d 100644 (file)
--- a/include/abg-fwd.h
+++ b/include/abg-fwd.h
@@ -916,17 +916,20 @@ get_pretty_representation(const method_type_sptr&,
 string
 get_class_or_union_flat_representation(const class_or_union& cou,
                                       const string& indent,
-                                      bool one_line);
+                                      bool one_line,
+                                      bool qualified_name = true);
 
 string
 get_class_or_union_flat_representation(const class_or_union* cou,
                                       const string& indent,
-                                      bool one_line);
+                                      bool one_line,
+                                      bool qualified_name = true);
 
 string
 get_class_or_union_flat_representation(const class_or_union_sptr& cou,
                                       const string& indent,
-                                      bool one_line);
+                                      bool one_line,
+                                      bool qualified_name = true);
 
 bool
 odr_is_relevant(const type_or_decl_base&);

diff --git a/include/abg-ir.h b/include/abg-ir.h
index de7ae0c205024af88f9d0fa59a3d2c5a1987755f..3b3eb9e2b191439208a0fa47d6f81597c411ce3c 100644 (file)
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -1210,7 +1210,8 @@ public:
   traverse(ir_node_visitor&);
 
   virtual string
-  get_pretty_representation(bool internal = false) const = 0;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const = 0;
 }; // end class type_or_decl_base
 
 bool
@@ -1332,7 +1333,8 @@ public:
   get_hash() const;
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   virtual void
   get_qualified_name(interned_string& qualified_name,
@@ -1777,7 +1779,8 @@ public:
   bool operator!=(const type_decl&)const;
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = false) const;
 
   virtual bool
   traverse(ir_node_visitor&);
@@ -1829,7 +1832,8 @@ public:
                 const location& locus, visibility vis = VISIBILITY_DEFAULT);
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   virtual bool
   operator==(const decl_base&) const;
@@ -2203,7 +2207,8 @@ public:
     vector_as_string(const vector<subrange_sptr>&);
 
     virtual string
-    get_pretty_representation(bool internal = false) const;
+    get_pretty_representation(bool internal = false,
+                             bool qualified_name = true) const;
 
     virtual bool
     traverse(ir_node_visitor&);
@@ -2248,7 +2253,8 @@ public:
   is_infinite() const;
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   virtual string
   get_subrange_representation() const;
@@ -2332,7 +2338,8 @@ public:
   get_enumerators();
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   virtual bool
   operator==(const decl_base&) const;
@@ -2450,7 +2457,8 @@ public:
   operator==(const type_base&) const;
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   type_base_sptr
   get_underlying_type() const;
@@ -2595,7 +2603,8 @@ public:
   get_hash() const;
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   virtual bool
   traverse(ir_node_visitor& v);
@@ -2672,7 +2681,8 @@ public:
                binding bind = BINDING_GLOBAL);
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   string
   get_pretty_representation_of_declarator (bool internal = false) const;
@@ -2875,7 +2885,8 @@ public:
                     bool internal = false) const;
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 }; // end class function_decl::parameter
 
 bool
@@ -2971,7 +2982,8 @@ public:
   operator==(const type_base&) const;
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   virtual bool
   traverse(ir_node_visitor&);
@@ -3045,7 +3057,8 @@ public:
   virtual ~method_type();
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   friend interned_string
   get_method_type_name(const method_type& fn_type, bool internal);
@@ -3832,7 +3845,8 @@ public:
   get_naked_definition_of_declaration() const;
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   void
   is_struct(bool f);
@@ -4034,7 +4048,8 @@ public:
             bool is_declaration_only = true);
 
   virtual string
-  get_pretty_representation(bool internal = false) const;
+  get_pretty_representation(bool internal = false,
+                           bool qualified_name = true) const;
 
   virtual bool
   operator==(const decl_base&) const;

diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index 9c1dce116b97a5400914c5e5f3e93717e5b8f293..064483ae3f6d51c0a1fc998ba8b871a47480cd82 100644 (file)
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -1455,6 +1455,23 @@ default_reporter::report(const union_diff& d, ostream& out,
 
   d.class_or_union_diff::report(out, indent);
 
+  if (d.context()->get_allowed_category() & HARMLESS_UNION_CHANGE_CATEGORY
+      && filtering::union_diff_has_harmless_changes(&d))
+    {
+      // The user wants to see harmless changes and the union diff we
+      // are looking at does carry some harmless changes.  Let's show
+      // the "before" and "after" carried by the diff node.
+      out << indent << "type changed from:\n"
+         << get_class_or_union_flat_representation(first, indent + "  ",
+                                                   /*one_line=*/true,
+                                                   /*qualified_names=*/false)
+         << "\n"
+         << indent << "to:\n"
+         << get_class_or_union_flat_representation(second, indent + "  ",
+                                                   /*one_line=*/true,
+                                                   /*qualified_names=*/false);
+    }
+
  d.currently_reporting(false);
 
   d.reported_once(true);

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 9d4ed28a3adaabfb72e4cf52b36a9bea6a84562a..5dd54067bd7488db97d451c3da8bc0e82fff6d53 100644 (file)
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -3288,12 +3288,20 @@ decl_base::get_qualified_name(interned_string& qn, bool internal) const
 /// otherwise.  If you don't know what this is for, then set it to
 /// false.
 ///
+/// @param qualified_name if true, names emitted in the pretty
+/// representation are fully qualified.
+///
 /// @return the default pretty representation for a decl.  This is
 /// basically the fully qualified name of the decl optionally prefixed
 /// with a meaningful string to add context for the user.
 string
-decl_base::get_pretty_representation(bool internal) const
-{return get_qualified_name(internal);}
+decl_base::get_pretty_representation(bool internal,
+                                    bool qualified_name) const
+{
+  if (qualified_name)
+    return get_qualified_name(internal);
+  return get_name();
+}
 
 /// Return the qualified name of the decl.
 ///
@@ -6459,7 +6467,8 @@ get_pretty_representation(const method_type_sptr method, bool internal)
 string
 get_class_or_union_flat_representation(const class_or_union& cou,
                                       const string& indent,
-                                      bool one_line)
+                                      bool one_line,
+                                      bool qualified_names)
 {
   string repr;
   string local_indent = "  ";
@@ -6507,22 +6516,28 @@ get_class_or_union_flat_representation(const class_or_union& cou,
        repr +=
          get_class_or_union_flat_representation
          (anonymous_data_member_to_class_or_union(*dm),
-          real_indent, one_line);
+          real_indent, one_line, qualified_names);
       else
        {
          if (one_line)
            {
              if (dm != dmems.begin())
                repr += real_indent;
-             repr += (*dm)->get_pretty_representation();
+             repr += (*dm)->get_pretty_representation(/*internal=*/false,
+                                                      qualified_names);
            }
          else
-           repr +=  real_indent+ (*dm)->get_pretty_representation();
+           repr +=
+             real_indent+ (*dm)->get_pretty_representation(/*internal=*/false,
+                                                           qualified_names);
        }
       repr += ";";
     }
 
-  repr += indent + "}";
+  if (one_line)
+    repr += "}";
+  else
+    repr += indent + "}";
 
   return repr;
 }
@@ -6546,10 +6561,12 @@ get_class_or_union_flat_representation(const class_or_union& cou,
 string
 get_class_or_union_flat_representation(const class_or_union* cou,
                                       const string& indent,
-                                      bool one_line)
+                                      bool one_line,
+                                      bool qualified_names)
 {
   if (cou)
-    return get_class_or_union_flat_representation(*cou, indent, one_line);
+    return get_class_or_union_flat_representation(*cou, indent, one_line,
+                                                 qualified_names);
   return "";
 }
 
@@ -6572,8 +6589,12 @@ get_class_or_union_flat_representation(const class_or_union* cou,
 string
 get_class_or_union_flat_representation(const class_or_union_sptr& cou,
                                       const string& indent,
-                                      bool one_line)
-{return get_class_or_union_flat_representation(cou.get(), indent, one_line);}
+                                      bool one_line,
+                                      bool qualified_names)
+{return get_class_or_union_flat_representation(cou.get(),
+                                              indent,
+                                              one_line,
+                                              qualified_names);}
 
 /// By looking at the language of the TU a given ABI artifact belongs
 /// to, test if the ONE Definition Rule should apply.
@@ -11566,10 +11587,18 @@ operator!=(const type_decl_sptr& l, const type_decl_sptr& r)
 /// otherwise.  If you don't know what this is for, then set it to
 /// false.
 ///
+/// @param qualified_name if true, names emitted in the pretty
+/// representation are fully qualified.
+///
 /// @return the pretty representatin of the @ref type_decl.
 string
-type_decl::get_pretty_representation(bool internal) const
-{return get_qualified_name(internal);}
+type_decl::get_pretty_representation(bool internal,
+                                    bool qualified_name) const
+{
+  if (qualified_name)
+    return get_qualified_name(internal);
+  return get_name();
+}
 
 /// This implements the ir_traversable_base::traverse pure virtual
 /// function.
@@ -11771,11 +11800,17 @@ namespace_decl::namespace_decl(const environment*     env,
 /// otherwise.  If you don't know what this is for, then set it to
 /// false.
 ///
+/// @param qualified_name if true, names emitted in the pretty
+/// representation are fully qualified.
+///
 /// @return a copy of the pretty representation of the namespace.
 string
-namespace_decl::get_pretty_representation(bool internal) const
+namespace_decl::get_pretty_representation(bool internal,
+                                         bool qualified_name) const
 {
-  string r = "namespace " + scope_decl::get_pretty_representation(internal);
+  string r =
+    "namespace " + scope_decl::get_pretty_representation(internal,
+                                                        qualified_name);
   return r;
 }
 
@@ -13279,7 +13314,7 @@ array_type_def::subrange_type::operator!=(const subrange_type& o) const
 /// @return a copy of the pretty representation of the current
 /// instance of typedef_decl.
 string
-array_type_def::subrange_type::get_pretty_representation(bool) const
+array_type_def::subrange_type::get_pretty_representation(bool, bool) const
 {
   string name = get_name();
   string repr;
@@ -13407,7 +13442,8 @@ get_type_representation(const array_type_def& a, bool internal)
 /// otherwise.  If you don't know what this is for, then set it to
 /// false.
 string
-array_type_def::get_pretty_representation(bool internal) const
+array_type_def::get_pretty_representation(bool internal,
+                                         bool /*qualified_name*/) const
 {return get_type_representation(*this, internal);}
 
 /// Compares two instances of @ref array_type_def.
@@ -13807,11 +13843,16 @@ enum_type_decl::get_enumerators()
 /// otherwise.  If you don't know what this is for, then set it to
 /// false.
 ///
+/// @param qualified_name if true, names emitted in the pretty
+/// representation are fully qualified.
+///
 /// @return the pretty representation of the enum type.
 string
-enum_type_decl::get_pretty_representation(bool internal) const
+enum_type_decl::get_pretty_representation(bool internal,
+                                         bool qualified_name) const
 {
-  string r = "enum " + decl_base::get_pretty_representation(internal);
+  string r = "enum " + decl_base::get_pretty_representation(internal,
+                                                           qualified_name);
   return r;
 }
 
@@ -14393,12 +14434,22 @@ typedef_decl::operator==(const type_base& o) const
 /// otherwise.  If you don't know what this is for, then set it to
 /// false.
 ///
+/// @param qualified_name if true, names emitted in the pretty
+/// representation are fully qualified.
+///
 /// @return a copy of the pretty representation of the current
 /// instance of typedef_decl.
 string
-typedef_decl::get_pretty_representation(bool internal) const
+typedef_decl::get_pretty_representation(bool internal,
+                                       bool qualified_name) const
 {
-  string result = "typedef " + get_qualified_name(internal);
+
+  string result = "typedef ";
+  if (qualified_name)
+    result += get_qualified_name(internal);
+  else
+    result += get_name();
+
   return result;
 }
 
@@ -14815,9 +14866,12 @@ var_decl::get_qualified_name(bool internal) const
 /// otherwise.  If you don't know what this is for, then set it to
 /// false.
 ///
+/// @param qualified_name if true, names emitted in the pretty
+/// representation are fully qualified.
+///
 /// @return a copy of the pretty representation of this variable.
 string
-var_decl::get_pretty_representation(bool internal) const
+var_decl::get_pretty_representation(bool internal, bool qualified_name) const
 {
   string result;
 
@@ -14834,7 +14888,7 @@ var_decl::get_pretty_representation(bool internal) const
   if (array_type_def_sptr t = is_array_type(get_type()))
     {
       string name;
-      if (member_of_anonymous_class)
+      if (member_of_anonymous_class || !qualified_name)
        name = get_name();
       else
        name = get_qualified_name(internal);
@@ -14860,7 +14914,7 @@ var_decl::get_pretty_representation(bool internal) const
            (is_class_or_union_type(get_type()),
             "", /*one_line=*/true);
          result += " ";
-         if (member_of_anonymous_class)
+         if (member_of_anonymous_class || !qualified_name)
            // It doesn't make sense to name the member of an
            // anonymous class or union like:
            // "__anonymous__::data_member_name".  So let's just use
@@ -14875,7 +14929,7 @@ var_decl::get_pretty_representation(bool internal) const
            get_type_declaration(get_type())->get_qualified_name(internal)
            + " ";
 
-         if (member_of_anonymous_class)
+         if (member_of_anonymous_class || !qualified_name)
            // It doesn't make sense to name the member of an
            // anonymous class or union like:
            // "__anonymous__::data_member_name".  So let's just use
@@ -15432,7 +15486,8 @@ function_type::operator==(const type_base& other) const
 /// @return a copy of the pretty representation of the current @ref
 /// function_type.
 string
-function_type::get_pretty_representation(bool internal) const
+function_type::get_pretty_representation(bool internal,
+                                        bool /*qualified_name*/) const
 {return ir::get_pretty_representation(this, internal);}
 
 /// Traverses an instance of @ref function_type, visiting all the
@@ -15646,7 +15701,8 @@ method_type::set_class_type(const class_or_union_sptr& t)
 /// @return a copy of the pretty representation of the current @ref
 /// method_type.
 string
-method_type::get_pretty_representation(bool internal) const
+method_type::get_pretty_representation(bool internal,
+                                      bool /*qualified_name*/) const
 {return ir::get_pretty_representation(*this, internal);}
 
 /// Setter of the "is-const" property of @ref method_type.
@@ -15765,7 +15821,8 @@ function_decl::function_decl(const string&      name,
 ///
 /// @return the pretty representation for a function.
 string
-function_decl::get_pretty_representation(bool internal) const
+function_decl::get_pretty_representation(bool internal,
+                                        bool /*qualified_name*/) const
 {
   const method_decl* mem_fn =
     dynamic_cast<const method_decl*>(this);
@@ -16661,7 +16718,8 @@ function_decl::parameter::get_qualified_name(interned_string& qualified_name,
 /// @return a copy of the textual representation of the current
 /// function parameter.
 string
-function_decl::parameter::get_pretty_representation(bool internal) const
+function_decl::parameter::get_pretty_representation(bool internal,
+                                                   bool /*qualified_name*/) const
 {
   const environment* env = get_environment();
   ABG_ASSERT(env);
@@ -18354,9 +18412,13 @@ class_decl::sort_virtual_mem_fns()
 /// otherwise.  If you don't know what this is for, then set it to
 /// false.
 ///
+/// @param qualified_name if true, names emitted in the pretty
+/// representation are fully qualified.
+///
 /// @return the pretty representaion for a class_decl.
 string
-class_decl::get_pretty_representation(bool internal) const
+class_decl::get_pretty_representation(bool internal,
+                                     bool qualified_name) const
 {
   string cl = "class ";
   if (!internal && is_struct())
@@ -18381,7 +18443,13 @@ class_decl::get_pretty_representation(bool internal) const
                                                      /*one_line=*/true);
     }
 
-  return cl + get_qualified_name(internal);
+  string result = cl;
+  if (qualified_name)
+    result += get_qualified_name(internal);
+  else
+    result += get_name();
+
+  return result;
 }
 
 decl_base_sptr
@@ -19987,16 +20055,26 @@ union_decl::union_decl(const environment* env,
 /// otherwise.  If you don't know what this is for, then set it to
 /// false.
 ///
+/// @param qualified_name if true, names emitted in the pretty
+/// representation are fully qualified.
+///
 /// @return the pretty representaion for a union_decl.
 string
-union_decl::get_pretty_representation(bool internal) const
+union_decl::get_pretty_representation(bool internal,
+                                     bool qualified_name) const
 {
   string repr;
   if (get_is_anonymous() && !internal)
     repr = get_class_or_union_flat_representation(this, "",
                                                  /*one_line=*/true);
   else
-    repr = "union " + get_qualified_name(internal);
+    {
+      repr = "union ";
+      if (qualified_name)
+       repr += get_qualified_name(internal);
+      else
+       repr += get_name();
+    }
 
   return repr;
 }

diff --git a/tests/data/test-diff-dwarf/test38-union-report-0.txt b/tests/data/test-diff-dwarf/test38-union-report-0.txt
index 4d5b0e79f1daf56d41d5402b54e550f1bae4ed5e..69893b1921b121d7b7a34961acb285c7bc72905b 100644 (file)
--- a/tests/data/test-diff-dwarf/test38-union-report-0.txt
+++ b/tests/data/test-diff-dwarf/test38-union-report-0.txt
@@ -10,5 +10,8 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
         1 data member deletion:
           'int some_union_type::m0'
 
-
+        type changed from:
+          union some_union_type{int m0; char m1; S m2;}
+        to:
+          union some_union_type{char m1; S m2;}
 

diff --git a/tests/test-diff-filter.cc b/tests/test-diff-filter.cc
index 74b8c06c9a5cd00d7fc6d15cc3884ea0cf2d9300..a0e0c777fe0e83e418d3fd41eb15af5fe495e296 100644 (file)
--- a/tests/test-diff-filter.cc
+++ b/tests/test-diff-filter.cc
@@ -556,6 +556,13 @@ InOutSpec in_out_specs[] =
     "data/test-diff-filter/test-PR24731-report-0.txt",
     "output/test-diff-filter/test-PR24731-report-0.txt",
   },
+  {
+    "data/test-diff-filter/test-PR24731-v0.o ",
+    "data/test-diff-filter/test-PR24731-v1.o ",
+    "--no-default-suppression --harmless",
+    "data/test-diff-filter/test-PR24731-report-1.txt",
+    "output/test-diff-filter/test-PR24731-report-1.txt",
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL, NULL}
 };