Do not mark "distinct" diff nodes as being redundant

When a char is changed into a const char several times in different
spots of the type graph, we want to see all the occurence of those
changes.  Generalizing this in Abigail parlance, we'd say that we want
to see all occurences of distinct diff nodes, so we don't want to mark
them as being redundant.

Right now, libabigail will only show the first occurence of that
change will flag subsequent onces as being redundant, by virtue of the
redundancy marking pass.

This patch avoids marking distinct diff nodes as being redundant.

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

	* include/abg-comp-filter.h (is_mostly_distinct_diff): Declare new
	function.
	* include/abg-fwd.h (peel_typedef_pointer_or_reference_type): Take
	a boolean to decide to peel qualified types or not.
	* src/abg-comp-filter.cc (is_mostly_distinct_diff): Define this function.
	* src/abg-comparison.cc (redundancy_marking_visitor::visit_begin):
	Do not mark distinct_diff nodes as being redundant.
	* src/abg-ir.cc (peel_typedef_pointer_or_reference_type):
	Implement taking a boolean to decide to peel qualified types or
	not.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This commit is contained in:
Dodji Seketeli 2018-04-11 12:45:25 +02:00
parent dc62bd5e71
commit e73901a523
5 changed files with 74 additions and 10 deletions

View File

@ -60,6 +60,9 @@ has_class_or_union_type_name_change(const diff *d);
bool
has_basic_or_class_type_name_change(const diff *d);
bool
is_mostly_distinct_diff(const diff *d);
struct filter_base;
/// Convenience typedef for a shared pointer to filter_base
typedef shared_ptr<filter_base> filter_base_sptr;

View File

@ -727,10 +727,12 @@ const type_base_sptr
peel_qualified_type(const type_base_sptr&);
type_base_sptr
peel_typedef_pointer_or_reference_type(const type_base_sptr);
peel_typedef_pointer_or_reference_type(const type_base_sptr,
bool peel_qualified_type = true);
type_base*
peel_typedef_pointer_or_reference_type(const type_base*);
peel_typedef_pointer_or_reference_type(const type_base* type,
bool peel_qualified_type = true);
string
get_name(const type_or_decl_base*, bool qualified = true);

View File

@ -851,6 +851,51 @@ has_basic_or_class_type_name_change(const diff *d)
|| has_class_or_union_type_name_change(d));
}
/// Test if a diff node carries a distinct type change or a
/// pointer/reference/typedef to distinct type change.
///
/// Note that a distinct type change is a change where the two
/// subjects of the change are not of the same kind, e.g, a basic type
/// that got changed into a qualified type.
///
/// @param d the diff node to consider.
///
/// @return true iff @p d is mostly a distinct diff.
bool
is_mostly_distinct_diff(const diff *d)
{
if (is_distinct_diff(d))
return true;
// Let's consider that 'd' is a type diff ...
type_diff_base* td = const_cast<type_diff_base*>(is_type_diff(d));
if (!td)
{
// ... or a function parameter diff. In which case, let's get
// its child type diff.
fn_parm_diff *pd = const_cast<fn_parm_diff*>(is_fn_parm_diff(d));
if (pd)
td = const_cast<type_diff_base*>(is_type_diff(pd->type_diff().get()));
else
return false;
}
// At this point, if we are not looking at a type diff we must have
// bailed out already.
assert(td);
type_base_sptr first = is_type(td->first_subject());
type_base_sptr second = is_type(td->second_subject());
first = peel_typedef_pointer_or_reference_type(first,
/*peel_qualified_type=*/false);
second = peel_typedef_pointer_or_reference_type(second,
/*peel_qual_type=*/false);
assert(first && second);
return distinct_diff::entities_are_of_distinct_kinds(first, second);
}
/// Test if an enum_diff carries an enumerator insertion.
///
/// @param diff the enum_diff to consider.

View File

@ -10779,6 +10779,8 @@ struct redundancy_marking_visitor : public diff_node_visitor
// type is changed into a char type in both a struct A
// and a struct B, we want to see both changes.
&& !has_basic_type_change_only(d)
// The same goes for distinct type changes
&& !filtering::is_mostly_distinct_diff(d)
// Functions with similar *local* changes are never marked
// redundant because otherwise one could miss important
// similar local changes that are applied to different

View File

@ -4634,15 +4634,20 @@ peel_qualified_type(const type_base_sptr& type)
/// typedef_decl, @ref pointer_type_def, @ref reference_type_def or
/// @ref qualified_type_def node.
///
/// @param type the type to peel.
///
/// @param peel_qualified_type if true, also peel qualified types.
///
/// @return the leaf underlying or pointed-to type node of @p type.
type_base_sptr
peel_typedef_pointer_or_reference_type(const type_base_sptr type)
peel_typedef_pointer_or_reference_type(const type_base_sptr type,
bool peel_qual_type)
{
type_base_sptr typ = type;
while (is_typedef(typ)
|| is_pointer_type(typ)
|| is_reference_type(typ)
|| is_qualified_type(typ))
|| (peel_qual_type && is_qualified_type(typ)))
{
if (typedef_decl_sptr t = is_typedef(typ))
typ = peel_typedef_type(t);
@ -4656,6 +4661,7 @@ peel_typedef_pointer_or_reference_type(const type_base_sptr type)
if (array_type_def_sptr t = is_array_type(typ))
typ = peel_array_type(t);
if (peel_qual_type)
if (qualified_type_def_sptr t = is_qualified_type(typ))
typ = peel_qualified_type(t);
}
@ -4667,14 +4673,19 @@ peel_typedef_pointer_or_reference_type(const type_base_sptr type)
/// typedef_decl, @ref pointer_type_def, @ref reference_type_def or
/// @ref qualified_type_def type node.
///
/// @param type the type to peel.
///
/// @param peel_qualified_type if true, also peel qualified types.
///
/// @return the leaf underlying or pointed-to type node of @p type.
type_base*
peel_typedef_pointer_or_reference_type(const type_base* type)
peel_typedef_pointer_or_reference_type(const type_base* type,
bool peel_qual_type)
{
while (is_typedef(type)
|| is_pointer_type(type)
|| is_reference_type(type)
|| is_qualified_type(type))
|| (peel_qual_type && is_qualified_type(type)))
{
if (const typedef_decl* t = is_typedef(type))
type = peel_typedef_type(t);
@ -4688,6 +4699,7 @@ peel_typedef_pointer_or_reference_type(const type_base* type)
if (const array_type_def* t = is_array_type(type))
type = peel_array_type(t);
if (peel_qual_type)
if (const qualified_type_def* t = is_qualified_type(type))
type = peel_qualified_type(t);
}