From e73901a523679e784fb36135aa241ca680fc37d9 Mon Sep 17 00:00:00 2001 From: Dodji Seketeli Date: Wed, 11 Apr 2018 12:45:25 +0200 Subject: [PATCH] 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 --- include/abg-comp-filter.h | 3 +++ include/abg-fwd.h | 6 ++++-- src/abg-comp-filter.cc | 45 +++++++++++++++++++++++++++++++++++++++ src/abg-comparison.cc | 2 ++ src/abg-ir.cc | 28 +++++++++++++++++------- 5 files changed, 74 insertions(+), 10 deletions(-) diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h index a25c1cf8..db9feca4 100644 --- a/include/abg-comp-filter.h +++ b/include/abg-comp-filter.h @@ -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_sptr; diff --git a/include/abg-fwd.h b/include/abg-fwd.h index a2c62c63..9fd2e3e8 100644 --- a/include/abg-fwd.h +++ b/include/abg-fwd.h @@ -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); diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc index 25c3f6a0..2a16bb00 100644 --- a/src/abg-comp-filter.cc +++ b/src/abg-comp-filter.cc @@ -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(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(is_fn_parm_diff(d)); + if (pd) + td = const_cast(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. diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index 7bb433c0..0cca5ad7 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -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 diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 708f112e..dd7ddf0d 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -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,8 +4661,9 @@ 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 (qualified_type_def_sptr t = is_qualified_type(typ)) - typ = peel_qualified_type(t); + if (peel_qual_type) + if (qualified_type_def_sptr t = is_qualified_type(typ)) + typ = peel_qualified_type(t); } return typ; @@ -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,8 +4699,9 @@ 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 (const qualified_type_def* t = is_qualified_type(type)) - type = peel_qualified_type(t); + if (peel_qual_type) + if (const qualified_type_def* t = is_qualified_type(type)) + type = peel_qualified_type(t); } return const_cast(type); -- 2.43.5