From: Dodji Seketeli Date: Wed, 26 Jun 2019 09:37:28 +0000 (+0200) Subject: Make abidiff --harmless show harmless changes in unions X-Git-Tag: libabigail-1.7~77 X-Git-Url: https://sourceware.org/git/?a=commitdiff_plain;h=2209df1b905c94077cf00788112b44dc2fe955f2;p=libabigail.git 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 --- diff --git a/include/abg-fwd.h b/include/abg-fwd.h index ddec06d8..529ef1bb 100644 --- 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 de7ae0c2..3b3eb9e2 100644 --- 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&); 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 9c1dce11..064483ae 100644 --- 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 9d4ed28a..5dd54067 100644 --- 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(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 4d5b0e79..69893b19 100644 --- 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 74b8c06c..a0e0c777 100644 --- 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} };