]> sourceware.org Git - libabigail.git/commitdiff
Improve function changes reporting in leaf and default mode
authorDodji Seketeli <dodji@redhat.com>
Thu, 12 Apr 2018 12:51:00 +0000 (14:51 +0200)
committerDodji Seketeli <dodji@redhat.com>
Fri, 13 Apr 2018 09:11:08 +0000 (11:11 +0200)
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 <dodji@redhat.com>
src/abg-default-reporter.cc
src/abg-ir.cc
src/abg-leaf-reporter.cc

index 5769d1cc1511ad415b007d8ea126e58802218e9d..36bdf0fa802433899d93121e64079ab491157224 100644 (file)
@@ -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);
     }
 }
 
index c402e9aef0b8de2eac163c334e2d4fde008ff9c4..f154b9b3ad627e61655414ac9255e4a5c0870de1 100644 (file)
@@ -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;
     }
index 29f8493a79ea4c4ae5830a7d842ae7b482991127..96bab8617f1c58dcb2f7ae6d41a6a0ccd77ce25d 100644 (file)
@@ -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<function_decl_diff_sptr> sorted_changed_fns;
+      sort_string_function_decl_diff_sptr_map(d.priv_->changed_fns_map_,
+                                             sorted_changed_fns);
+      for (vector<function_decl_diff_sptr>::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)
This page took 0.06946 seconds and 5 git commands to generate.