From: Dodji Seketeli Date: Thu, 12 Apr 2018 12:51:00 +0000 (+0200) Subject: Improve function changes reporting in leaf and default mode X-Git-Tag: libabigail-1.3~24 X-Git-Url: https://sourceware.org/git/?a=commitdiff_plain;h=c8f98d8fb563627d97cf9d3330493ccfaa84c89f;p=libabigail.git Improve function changes reporting in leaf and default mode There are cases where we wrongly miss reporting function changes: - redundant parameter type changes parameters and return type - changes where the textual type representation changed. This is relevant to the leaf change report mode. - changed functions are simply not reported in the leaf reporting mode, oops. This patch fixes each one of the cases above. This patch is part of the set of patches whose titles are: Do not show decl-only-to-def changes in the leaf reporter Overhaul of the report diff stats summary Do not mark "distinct" diff nodes as being redundant Fix meaning of "harmless name change" to avoid overfiltering Better handle category propagation of pointer changes Improve function changes reporting in leaf and default mode Don't filter out typedef changes with redundant underlying type changes Only show leaf type changes in the leaf type changes section Fix leaf report of class data member changes Always show redundant changes in leaf mode Avoid reporting an enum change if it has already been reported When we say an a change was reported earlier give its source location [abipkgdiff]: in leaf mode we always show redundant changes Update tests for the "better leaf mode redundancy management" patchset * src/abg-default-reporter.cc (default_reporter::report): In the overload for fn_parm_diff, consider that parameter type changes are never redundant. * src/abg-ir.cc (equals): In the overload for function_type, consider that if the textual representation of the function return type or a function parameter changed, then that's a local change for the current instance of function_type. Likewise, in the overload for function_decl, consider that a change in the textual representation of the function_decl is a local change. Likewise, in the overload of function_decl::parameter, consider that a change in the textual representation of the parameter type is a local change. * src/abg-leaf-reporter.cc (leaf_reporter::report): Report leaf changes to functions. Signed-off-by: Dodji Seketeli --- diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc index 5769d1cc..36bdf0fa 100644 --- a/src/abg-default-reporter.cc +++ b/src/abg-default-reporter.cc @@ -455,7 +455,11 @@ default_reporter::report(const fn_parm_diff& d, ostream& out, if (d.to_be_reported()) { - assert(d.type_diff() && d.type_diff()->to_be_reported()); + diff_sptr type_diff = d.type_diff(); + assert(type_diff->has_changes()); + diff_category saved_category = type_diff->get_category(); + // Parameter type changes are never redundants. + type_diff->set_category(saved_category & ~REDUNDANT_CATEGORY); out << indent << "parameter " << f->get_index(); report_loc_info(f, *d.context(), out); @@ -467,7 +471,8 @@ default_reporter::report(const fn_parm_diff& d, ostream& out, else out << "' changed:\n"; - d.type_diff()->report(out, indent + " "); + type_diff->report(out, indent + " "); + type_diff->set_category(saved_category); } } diff --git a/src/abg-ir.cc b/src/abg-ir.cc index c402e9ae..f154b9b3 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -14234,7 +14234,12 @@ equals(const function_type& lhs, { result = false; if (k) - *k |= SUBTYPE_CHANGE_KIND; + { + if (lhs.get_return_type()->get_pretty_representation() + != rhs.get_return_type()->get_pretty_representation()) + *k |= LOCAL_CHANGE_KIND; + *k |= SUBTYPE_CHANGE_KIND; + } else RETURN(result); } @@ -14271,7 +14276,12 @@ equals(const function_type& lhs, { result = false; if (k) - *k |= SUBTYPE_CHANGE_KIND; + { + if ((*i)->get_pretty_representation() + != (*j)->get_pretty_representation()) + *k |= LOCAL_CHANGE_KIND; + *k |= SUBTYPE_CHANGE_KIND; + } else RETURN(result); } @@ -15003,7 +15013,11 @@ equals(const function_decl& l, const function_decl& r, change_kind* k) { result = false; if (k) - *k |= SUBTYPE_CHANGE_KIND; + { + if (l.get_pretty_representation() != r.get_pretty_representation()) + *k |= LOCAL_CHANGE_KIND; + *k |= SUBTYPE_CHANGE_KIND; + } else return false; } @@ -15467,7 +15481,12 @@ equals(const function_decl::parameter& l, { result = false; if (k) - *k |= SUBTYPE_CHANGE_KIND; + { + *k |= SUBTYPE_CHANGE_KIND; + if (get_pretty_representation(l_type) + != get_pretty_representation(r_type)) + *k |= LOCAL_CHANGE_KIND; + } else return false; } diff --git a/src/abg-leaf-reporter.cc b/src/abg-leaf-reporter.cc index 29f8493a..96bab861 100644 --- a/src/abg-leaf-reporter.cc +++ b/src/abg-leaf-reporter.cc @@ -1078,6 +1078,79 @@ leaf_reporter::report(const corpus_diff& d, out << "\n"; } + if (ctxt->show_changed_fns()) + { + // Show changed functions. + size_t num_changed = s.net_num_leaf_func_changes(); + if (num_changed == 1) + out << indent << "1 function with some sub-type change:\n\n"; + else if (num_changed > 1) + out << indent << num_changed + << " functions with some sub-type change:\n\n"; + + bool changed = false; + vector sorted_changed_fns; + sort_string_function_decl_diff_sptr_map(d.priv_->changed_fns_map_, + sorted_changed_fns); + for (vector::const_iterator i = + sorted_changed_fns.begin(); + i != sorted_changed_fns.end(); + ++i) + { + diff_sptr diff = *i; + if (!diff) + continue; + + if (diff_to_be_reported(diff.get())) + { + function_decl_sptr fn = (*i)->first_function_decl(); + out << indent << " [C]'" + << fn->get_pretty_representation() << "'"; + report_loc_info((*i)->second_function_decl(), *ctxt, out); + out << " has some sub-type changes:\n"; + if ((fn->get_symbol()->has_aliases() + && !(is_member_function(fn) + && get_member_function_is_ctor(fn)) + && !(is_member_function(fn) + && get_member_function_is_dtor(fn))) + || (is_c_language(get_translation_unit(fn)->get_language()) + && fn->get_name() != fn->get_linkage_name())) + { + int number_of_aliases = + fn->get_symbol()->get_number_of_aliases(); + if (number_of_aliases == 0) + { + out << indent << " " + << "Please note that the exported symbol of " + "this function is " + << fn->get_symbol()->get_id_string() + << "\n"; + } + else + { + out << indent << " " + << "Please note that the symbol of this function is " + << fn->get_symbol()->get_id_string() + << "\n and it aliases symbol"; + if (number_of_aliases > 1) + out << "s"; + out << ": " + << fn->get_symbol()->get_aliases_id_string(false) + << "\n"; + } + } + diff->report(out, indent + " "); + out << "\n"; + changed |= true; + } + } + if (changed) + { + out << "\n"; + changed = false; + } + } + if (ctxt->show_added_fns()) { if (s.net_num_func_added() == 1)