diff --git a/include/Makefile.am b/include/Makefile.am index e4bc2b62..69a77e4b 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -6,6 +6,7 @@ abg-reader.h \ abg-dwarf-reader.h \ abg-writer.h \ abg-comparison.h \ +abg-comp-filter.h \ abg-diff-utils.h \ abg-libxml-utils.h \ abg-libzip-utils.h \ diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h new file mode 100644 index 00000000..88c56f9a --- /dev/null +++ b/include/abg-comp-filter.h @@ -0,0 +1,94 @@ +// -*- Mode: C++ -*- +// +// Copyright (C) 2013 Red Hat, Inc. +// +// This file is part of the GNU Application Binary Interface Generic +// Analysis and Instrumentation Library (libabigail). This library is +// free software; you can redistribute it and/or modify it under the +// terms of the GNU Lesser General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) any +// later version. + +// This library is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Lesser Public License for more details. + +// You should have received a copy of the GNU Lesser General Public +// License along with this program; see the file COPYING-LGPLV3. If +// not, see . +// +// Author: Dodji Seketeli + +/// @file +/// +/// This header declares filters for the diff trees resulting from +/// comparing ABI Corpora. + +#ifndef __ABG_COMP_FILTER_H__ +#define __ABG_COMP_FILTER_H__ + +#include "abg-comparison.h" + +namespace abigail +{ +namespace comparison +{ +/// Facilities to walk, categorize and possibly filter nodes of the +/// diff tree. +namespace filtering +{ + +class filter_base; +/// Convenience typedef for a shared pointer to filter_base +typedef shared_ptr filter_base_sptr; +/// Convenience typedef for a vector of filter_base_sptr +typedef std::vector filters; + +/// The base class for the diff tree node filter. +/// +/// It's intended to walk a tree of diff nodes and tag each relevant +/// name into one or several categories depending on well choosen +/// properties of the diff nodes. +struct filter_base : public diff_node_visitor +{ + friend void + apply_filter(filter_base_sptr f, diff_sptr deef); +}; //end class filter_base + +void +apply_filter(filter_base& filter, diff_sptr d); + +void +apply_filter(filter_base_sptr filter, diff_sptr d); + +class harmless_filter; +/// Convenience typedef for a shared pointer to a harmless_filter. +typedef shared_ptr harmless_filter_sptr; + +/// A filter that walks the diff nodes tree and tags relevant diff +/// nodes into categories considered to represent harmless changes. +class harmless_filter : public filter_base +{ + virtual bool + visit(diff*, bool); +}; // end class harmless_filter + +class harmful_filter; +/// A convenience typedef for a shared pointer to harmful_filter. +typedef shared_ptr harmful_filter_sptr; + +/// A filter that walks the diff nodes tree tags relevant diff nodes +/// into categories considered to represent potentially harmful +/// changes. +class harmful_filter : public filter_base +{ + virtual bool + visit(diff*, bool); +}; // end class harmful_filter + +} // end namespace filtering +} // end namespace comparison +} // end namespace abigail + +#endif // __ABG_COMP_FILTER_H__ diff --git a/include/abg-comparison.h b/include/abg-comparison.h index 09a74aad..78f0431b 100644 --- a/include/abg-comparison.h +++ b/include/abg-comparison.h @@ -39,6 +39,13 @@ namespace abigail namespace comparison { +namespace filtering +{ +class filter_base; +typedef shared_ptr filter_base_sptr; +typedef std::vector filters; +} + // Inject types we need into this namespace. using std::ostream; using std::vector; @@ -155,20 +162,78 @@ class diff_context; /// Convenience typedef for a shared pointer of @ref diff_context. typedef shared_ptr diff_context_sptr; -struct diff_node_visitor; +class diff_node_visitor; struct diff_traversable_base; /// Convenience typedef for shared_ptr on diff_traversable_base. typedef shared_ptr diff_traversable_base_sptr; -/// The base class for the diff classes that are to be traversed. -struct diff_traversable_base : public traversable_base +/// An enum for the different ways to visit a diff tree node. +/// +/// This is used by the node traversing code to know when to invoke a +/// visitor on a diff tree node. +enum visiting_kind { + /// The default enumerator value of this enum. It doesn't have any + /// particular meaning yet. + NO_VISITING_KIND = 0, + /// This says that a visitor should be invoked on a tree node + /// *before* visiting the node's children. + PRE_VISITING_KIND = 1, + /// This says that a visotor should be invoked on a tree node + /// *after* visiting the node's children. + POST_VISITING_KIND = 1 << 1 +}; + +visiting_kind +operator|(visiting_kind l, visiting_kind r); + +/// The base class for the diff classes that are to be traversed. +class diff_traversable_base : public traversable_base +{ +public: + virtual bool traverse(diff_node_visitor& v); }; // end struct diff_traversable_base +/// An enum for the different categories that a diff tree node falls +/// into, regarding the kind of changes it represents. +enum diff_category +{ + /// This means the diff node does not carry any (meaningful) change. + NO_CHANGE_CATEGORY = 0, + /// This means the diff node (or at least one of its descendant + /// nodes) carries access related changes, e.g, a private member + /// that becomes public. + ACCESS_CHANGED_CATEGORY = 1, + /// This means the diff node (or at least one of its descendant + /// nodes) carries a change that modifies the size of a type. + SIZE_CHANGED_CATEGORY = 1 << 1, + + /// A special enumerator that is the logical 'or' all the + /// enumerators above. + /// + /// This one must stay the last enumerator. Please update it each + /// time you add a new enumerator above. + EVERYTHING_CATEGORY = + ACCESS_CHANGED_CATEGORY + | SIZE_CHANGED_CATEGORY +}; + +diff_category +operator|(diff_category c1, diff_category c2); + +diff_category +operator^(diff_category c1, diff_category c2); + +diff_category +operator&(diff_category c1, diff_category c2); + +diff_category +operator~(diff_category c); + /// The context of the diff. This type holds various bits of /// information that is going to be used throughout the diffing of two /// entities and the reporting that follows. @@ -196,6 +261,27 @@ public: const decl_base_sptr second, diff_sptr d); + diff_category + get_allowed_category() const; + + void + set_allowed_category(diff_category c); + + void + switch_categories_on(diff_category c); + + void + switch_categories_off(diff_category c); + + const filtering::filters& + diff_filters() const; + + void + add_diff_filter(filtering::filter_base_sptr); + + void + maybe_apply_filters(diff_sptr dif); + void show_stats_only(bool f); @@ -244,33 +330,69 @@ public: /// constructs are called the "subjects" of the diff. class diff : public diff_traversable_base { - decl_base_sptr first_subject_; - decl_base_sptr second_subject_; - diff_context_sptr ctxt_; - mutable bool reported_once_; - mutable bool currently_reporting_; + diff* parent_; + decl_base_sptr first_subject_; + decl_base_sptr second_subject_; + diff_context_sptr ctxt_; + diff_category category_; + mutable bool reported_once_; + mutable bool currently_reporting_; + + // Forbidden + diff(); protected: diff(decl_base_sptr first_subject, decl_base_sptr second_subject) - : first_subject_(first_subject), + : parent_(0), + first_subject_(first_subject), second_subject_(second_subject), + category_(NO_CHANGE_CATEGORY), reported_once_(false), currently_reporting_(false) {} -diff(decl_base_sptr first_subject, - decl_base_sptr second_subject, - diff_context_sptr ctxt) - : first_subject_(first_subject), +diff(decl_base_sptr first_subject, + decl_base_sptr second_subject, + diff_context_sptr ctxt) + : parent_(0), + first_subject_(first_subject), second_subject_(second_subject), ctxt_(ctxt), + category_(NO_CHANGE_CATEGORY), + reported_once_(false), + currently_reporting_(false) + {} + + diff(diff* parent, + decl_base_sptr first_subject, + decl_base_sptr second_subject, + diff_context_sptr ctxt) + : parent_(parent), + first_subject_(first_subject), + second_subject_(second_subject), + ctxt_(ctxt), + category_(NO_CHANGE_CATEGORY), reported_once_(false), currently_reporting_(false) {} public: + /// Getter of the parent diff node of this one. + /// + /// Return the parent diff node if any, null otherwise. + diff* + get_parent() const + {return parent_;} + + /// Setter for the parent diff node of this one. + /// + /// @param parent the parent diff node. + void + set_parent(diff* parent) + {parent_ = parent;} + /// Getter of the first subject of the diff. /// /// @return the first subject of the diff. @@ -334,6 +456,34 @@ public: reported_once(bool f) const {reported_once_ = f;} + /// Getter for the category of the current diff tree node. + /// + /// @return the category of the current diff tree node. + diff_category + get_category() const + {return category_;} + + /// Adds the current diff tree node to an additional set of + /// categories. + /// + /// @param c a bit-map representing the set of categories to add the + /// current diff tree node to. + /// + /// @return the resulting bit-map representing the categories this + /// current diff tree node belongs to. + diff_category + add_to_category(diff_category c) + { + category_ = category_ | c; + return category_; + } + + bool + is_filtered_out() const; + + bool + to_be_reported() const; + /// Pure interface to get the length of the changes /// encapsulated by this diff. This is to be implemented by all /// descendants of this class. @@ -1177,49 +1327,90 @@ compute_diff(const corpus_sptr, /// The base class for the node visitors. These are the types used to /// visit each node traversed by the diff_traversable_base::traverse() method. -struct diff_node_visitor +class diff_node_visitor : public node_visitor_base { - virtual bool - visit(distinct_diff*); +protected: + visiting_kind visiting_kind_; + +public: + + diff_node_visitor() + : visiting_kind_(PRE_VISITING_KIND) + {} + + /// Getter for the visiting policy of the traversing code while + /// invoking this visitor. + /// + /// @return the visiting policy used by the traversing code when + /// invoking this visitor. + visiting_kind + get_visiting_kind() const + {return visiting_kind_;} + + /// Setter for the visiting policy of the traversing code while + /// invoking this visitor. + /// + /// @param v a bit map representing the new visiting policy used by + /// the traversing code when invoking this visitor. + void + set_visiting_kind(visiting_kind v) + {visiting_kind_ = v;} + + /// Setter for the visiting policy of the traversing code while + /// invoking this visitor. This one makes a logical or between the + /// current policy and the bitmap given in argument and assigns the + /// current policy to the result. + /// + /// @param v a bitmap representing the visiting policy to or with + /// the current policy. + void + or_visiting_kind(visiting_kind v) + {visiting_kind_ = visiting_kind_ | v;} virtual bool - visit(var_diff*); + visit(diff*, bool); virtual bool - visit(pointer_diff*); + visit(distinct_diff*, bool); virtual bool - visit(reference_diff*); + visit(var_diff*, bool); virtual bool - visit(qualified_type_diff*); + visit(pointer_diff*, bool); virtual bool - visit(enum_diff*); + visit(reference_diff*, bool); virtual bool - visit(class_diff*); + visit(qualified_type_diff*, bool); virtual bool - visit(base_diff*); + visit(enum_diff*, bool); virtual bool - visit(scope_diff*); + visit(class_diff*, bool); virtual bool - visit(function_decl_diff*); + visit(base_diff*, bool); virtual bool - visit(type_decl_diff*); + visit(scope_diff*, bool); virtual bool - visit(typedef_diff*); + visit(function_decl_diff*, bool); virtual bool - visit(translation_unit_diff*); + visit(type_decl_diff*, bool); virtual bool - visit(corpus_diff*); + visit(typedef_diff*, bool); + + virtual bool + visit(translation_unit_diff*, bool); + + virtual bool + visit(corpus_diff*, bool); }; // end struct diff_node_visitor }// end namespace comparison diff --git a/src/Makefile.am b/src/Makefile.am index 95f7e676..70e4603d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -8,6 +8,7 @@ $(h)/abg-ir.cc \ $(h)/abg-corpus.cc \ $(h)/abg-diff-utils.cc \ $(h)/abg-comparison.cc \ +$(h)/abg-comp-filter.cc \ $(h)/abg-reader.cc \ $(h)/abg-dwarf-reader.cc \ $(h)/abg-libxml-utils.cc \ diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc new file mode 100644 index 00000000..035ad16a --- /dev/null +++ b/src/abg-comp-filter.cc @@ -0,0 +1,137 @@ +// -*- Mode: C++ -*- +// +// Copyright (C) 2013 Red Hat, Inc. +// +// This file is part of the GNU Application Binary Interface Generic +// Analysis and Instrumentation Library (libabigail). This library is +// free software; you can redistribute it and/or modify it under the +// terms of the GNU Lesser General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) any +// later version. + +// This library is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Lesser Public License for more details. + +// You should have received a copy of the GNU Lesser General Public +// License along with this program; see the file COPYING-LGPLV3. If +// not, see . +// +// Author: Dodji Seketeli + +/// @file +/// +/// This file contains definitions of diff objects filtering +/// facilities. + +#include "abg-comp-filter.h" + +namespace abigail +{ +namespace comparison +{ +namespace filtering +{ + +/// Walk and categorize the nodes of a diff sub-tree. +/// +/// @param filter the filter invoked on each node of the walked +/// sub-tree. +/// +/// @param d the diff sub-tree node to start the walk from. +void +apply_filter(filter_base& filter, diff_sptr d) +{ + filter.set_visiting_kind(PRE_VISITING_KIND | POST_VISITING_KIND); + d->traverse(filter); +} + +/// Walk and categorize the nodes of a diff sub-tree. +/// +/// @param filter the filter invoked on each node of the walked +/// sub-tree. +/// +/// @param d the diff sub-tree node to start the walk from. +void +apply_filter(filter_base_sptr filter, diff_sptr d) +{apply_filter(*filter, d);} + +/// The visiting code of the harmless_filter. +/// +/// @param d the diff node being visited. +/// +/// @param pre this is true iff the node is being visited *before* the +/// children nodes of @p d. +/// +/// @return true iff the traversal shall keep going after the +/// completion of this function. +bool +harmless_filter::visit(diff* d, bool pre) +{ + if (pre) + { + decl_base_sptr f = d->first_subject(), + s = d->second_subject(); + + if (!is_member_decl(f) + && !is_member_decl(s)) + return true; + + access_specifier fa = get_member_access_specifier(f), + sa = get_member_access_specifier(s); + + if (sa != fa) + d->add_to_category(ACCESS_CHANGED_CATEGORY); + // Add non-virtual member function deletions and changes due to + // details that are being reported or got reported earlier. + } + + // Propagate the categorization to the parent nodes. + if (d->get_parent()) + d->get_parent()->add_to_category(d->get_category() + & ACCESS_CHANGED_CATEGORY); + + return true; +} + +/// The visiting code of the harmful_filter. +/// +/// @param d the diff node being visited. +/// +/// @param pre this is true iff the node is being visited *before* the +/// children nodes of @p d. +/// +/// @return true iff the traversal shall keep going after the +/// completion of this function. +bool +harmful_filter::visit(diff* d, bool pre) +{ + if (pre) + { + decl_base_sptr f = d->first_subject(), + s = d->second_subject(); + bool size_changed = false; + type_base_sptr tf, ts; + + if (is_type(f) && is_type(s)) + { + tf= is_type(f), ts = is_type(s); + if (tf->get_size_in_bits() != ts->get_size_in_bits()) + size_changed = true; + } + + if (size_changed) + d->add_to_category(SIZE_CHANGED_CATEGORY); + } + + // Propagate the categorization to the parent nodes. + if (d->get_parent()) + d->get_parent()->add_to_category(d->get_category() & SIZE_CHANGED_CATEGORY); + + return true; +} + +} // end namespace filtering +} // end namespace comparison +} // end namespace abigail diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index 3ca07b51..bb86bfae 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -22,9 +22,9 @@ /// @file - -#include "abg-comparison.h" #include "abg-hash.h" +#include "abg-comparison.h" +#include "abg-comp-filter.h" namespace abigail { @@ -74,6 +74,26 @@ struct decls_equal typedef unordered_map decls_diff_map_type; +/// The overloaded or operator for @ref visiting_kind. +visiting_kind +operator|(visiting_kind l, visiting_kind r) +{return static_cast(static_cast(l) + | static_cast(r));} + +#define TRY_PRE_VISIT(v) \ + do { \ + if (v.get_visiting_kind() & PRE_VISITING_KIND) \ + if (!v.visit(this, /*pre=*/true)) \ + return false; \ + } while(false) + +#define TRY_POST_VISIT(v) \ + do { \ + if (v.get_visiting_kind() & POST_VISITING_KIND) \ + if (!v.visit(this, /*pre=*/false)) \ + return false; \ + } while(false) + /// The default traverse function. /// /// @return true. @@ -84,19 +104,20 @@ diff_traversable_base::traverse(diff_node_visitor&) /// The private member (pimpl) for @ref diff_context. struct diff_context::priv { - decls_diff_map_type decls_diff_map; - vector regexps_fns_to_suppress; - vector regexps_vars_to_suppress; - bool show_stats_only_; - bool show_deleted_fns_; - bool show_changed_fns_; - bool show_added_fns_; - bool show_deleted_vars_; - bool show_changed_vars_; - bool show_added_vars_; + diff_category allowed_category_; + decls_diff_map_type decls_diff_map; + vector filters_; + bool show_stats_only_; + bool show_deleted_fns_; + bool show_changed_fns_; + bool show_added_fns_; + bool show_deleted_vars_; + bool show_changed_vars_; + bool show_added_vars_; priv() - : show_stats_only_(false), + : allowed_category_(EVERYTHING_CATEGORY), + show_stats_only_(false), show_deleted_fns_(true), show_changed_fns_(true), show_added_fns_(true), @@ -109,6 +130,14 @@ struct diff_context::priv diff_context::diff_context() : priv_(new diff_context::priv) { + // Setup all the diff output filters we have. + filtering::filter_base_sptr f; + + f.reset(new filtering::harmless_filter); + add_diff_filter(f); + + f.reset(new filtering::harmful_filter); + add_diff_filter(f); } /// Tests if the current diff context already has a diff for two decls. @@ -155,6 +184,48 @@ diff_sptr diff_context::has_diff_for(const diff_sptr d) const {return has_diff_for(d->first_subject(), d->second_subject());} +/// Getter for the bitmap that represents the set of categories that +/// the user wants to see reported. +/// +/// @return a bitmap that represents the set of categories that the +/// user wants to see reported. +diff_category +diff_context::get_allowed_category() const +{return priv_->allowed_category_;} + +/// Setter for the bitmap that represents the set of categories that +/// the user wants to see reported. +/// +/// @param c a bitmap that represents the set of categories that the +/// user wants to see represented. +void +diff_context::set_allowed_category(diff_category c) +{priv_->allowed_category_ = c;} + +/// Setter for the bitmap that represents the set of categories that +/// the user wants to see reported +/// +/// This function perform a bitwise or between the new set of +/// categories and the current ones, and then sets the current +/// categories to the result of the or. +/// +/// @param c a bitmap that represents the set of categories that the +/// user wants to see represented. +void +diff_context::switch_categories_on(diff_category c) +{priv_->allowed_category_ = priv_->allowed_category_ | c;} + +/// Setter for the bitmap that represents the set of categories that +/// the user wants to see reported +/// +/// This function actually unsets bits from the current categories. +/// +/// @param c a bitmap that represents the set of categories to unset +/// from the current categories. +void +diff_context::switch_categories_off(diff_category c) +{priv_->allowed_category_ = priv_->allowed_category_ & ~c;} + /// Add a diff for two decls to the cache of the current diff_context /// /// @param first the first decl to consider. @@ -168,6 +239,40 @@ diff_context::add_diff(decl_base_sptr first, diff_sptr d) {priv_->decls_diff_map[std::make_pair(first, second)] = d;} +/// Getter for the diff tree nodes filters to apply to diff sub-trees. +/// +/// @return the vector of tree filters to apply to diff sub-trees. +const filtering::filters& +diff_context::diff_filters() const +{return priv_->filters_;} + +/// Setter for the diff filters to apply to a given diff sub-tree. +/// +/// @param f the new diff filter to add to the vector of diff filters +/// to apply to diff sub-trees. +void +diff_context::add_diff_filter(filtering::filter_base_sptr f) +{priv_->filters_.push_back(f);} + +/// Apply the diff filters to a given diff sub-tree. +/// +/// If the current context is instructed to filter out some categories +/// then this function walks the given sub-tree and categorizes its +/// nodes by using the filters held by the context. +/// +/// @param diff the diff sub-tree to apply the filters to. +void +diff_context::maybe_apply_filters(diff_sptr diff) +{ + if (get_allowed_category() == EVERYTHING_CATEGORY) + return; + + for (filtering::filters::const_iterator i = diff_filters().begin(); + i != diff_filters().end(); + ++i) + filtering::apply_filter(*i, diff); +} + /// Set a flag saying if the comparison module should only show the /// diff stats. /// @@ -261,6 +366,32 @@ diff_context::show_added_vars() const {return priv_->show_added_vars_;} // +// + +/// Test if this diff tree node is to be filtered out for reporting +/// purposes. +/// +/// The function tests if the categories of the diff tree node are +/// "forbidden" by the context or not. +/// +/// @return true iff the current diff node should NOT be reported. +bool +diff::is_filtered_out() const +{return (get_category() != NO_CHANGE_CATEGORY + && !(get_category() & context()->get_allowed_category()));} + +/// Test if this diff tree node should be reported. +/// +/// @return true iff the current node should be reported. +bool +diff::to_be_reported() const +{ + if (length() && !is_filtered_out()) + return true; + return false; +} +// + // /// The private data structure for @ref distinct_diff. @@ -343,7 +474,7 @@ distinct_diff::length() const void distinct_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; decl_base_sptr f = first(), s = second(); @@ -362,7 +493,10 @@ distinct_diff::report(ostream& out, const string& indent) const /// otherwise. bool distinct_diff::traverse(diff_node_visitor& v) -{return v.visit(this);} +{ + TRY_PRE_VISIT(v); + return true; +} /// Try to diff entities that are of distinct kinds. /// @@ -525,6 +659,25 @@ compute_diff_for_types(const decl_base_sptr first, return d; } +diff_category +operator|(diff_category c1, diff_category c2) +{return static_cast(static_cast(c1) + | static_cast(c2));} + +diff_category +operator^(diff_category c1, diff_category c2) +{return static_cast(static_cast(c1) + ^ static_cast(c2));} + +diff_category +operator&(diff_category c1, diff_category c2) +{return static_cast(static_cast(c1) + & static_cast(c2));} + +diff_category +operator~(diff_category c) +{return static_cast(~static_cast(c));} + /// Compute the difference between two types. /// /// The function considers every possible types known to libabigail @@ -604,9 +757,9 @@ compute_diff_for_decls(const decl_base_sptr first, /// @return the resulting diff, or NULL if the diff could not be /// computed. diff_sptr -compute_diff(const decl_base_sptr first, - const decl_base_sptr second, - diff_context_sptr ctxt) +compute_diff(const decl_base_sptr first, + const decl_base_sptr second, + diff_context_sptr ctxt) { if (!first || !second) return diff_sptr(); @@ -631,9 +784,9 @@ compute_diff(const decl_base_sptr first, /// @return the resulting diff, or NULL if the diff couldn't be /// computed. diff_sptr -compute_diff(const type_base_sptr first, - const type_base_sptr second, - diff_context_sptr ctxt) +compute_diff(const type_base_sptr first, + const type_base_sptr second, + diff_context_sptr ctxt) { if (!first || !second) return diff_sptr(); @@ -1000,7 +1153,10 @@ var_diff::var_diff(var_decl_sptr first, diff_context_sptr ctxt) : diff(first, second, ctxt), priv_(new priv) -{priv_->type_diff_ = type_diff;} +{ + priv_->type_diff_ = type_diff; + type_diff->set_parent(this); +} /// Getter for the first @ref var_decl of the diff. /// @@ -1052,7 +1208,7 @@ var_diff::length() const void var_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; decl_base_sptr first = first_var(), second = second_var(); @@ -1066,7 +1222,7 @@ var_diff::report(ostream& out, const string& indent) const if (diff_sptr d = type_diff()) { - if (d->length()) + if (d->to_be_reported()) { out << indent << "type of variable changed:\n"; d->report(out, indent + " "); @@ -1082,7 +1238,17 @@ var_diff::report(ostream& out, const string& indent) const /// otherwise. bool var_diff::traverse(diff_node_visitor& v) -{return v.visit(this);} +{ + TRY_PRE_VISIT(v); + + if (diff_sptr d = type_diff()) + if (!d->traverse(v)) + return false; + + TRY_POST_VISIT(v); + + return true; +} /// Compute the diff between two instances of @ref var_decl. /// @@ -1094,9 +1260,9 @@ var_diff::traverse(diff_node_visitor& v) /// /// @return the resulting diff between the two @ref var_decl. var_diff_sptr -compute_diff(const var_decl_sptr first, - const var_decl_sptr second, - diff_context_sptr ctxt) +compute_diff(const var_decl_sptr first, + const var_decl_sptr second, + diff_context_sptr ctxt) { if (diff_sptr di = ctxt->has_diff_for(first, second)) { @@ -1217,7 +1383,10 @@ pointer_diff::underlying_type_diff() const /// of this diff. void pointer_diff::underlying_type_diff(const diff_sptr d) -{priv_->underlying_type_diff_ = d;} +{ + priv_->underlying_type_diff_ = d; + d->set_parent(this); +} /// Report the diff in a serialized form. /// @@ -1228,7 +1397,7 @@ pointer_diff::underlying_type_diff(const diff_sptr d) void pointer_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; if (diff_sptr d = underlying_type_diff()) @@ -1272,13 +1441,14 @@ pointer_diff::report(ostream& out, const string& indent) const bool pointer_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) - return false; + TRY_PRE_VISIT(v); if (diff_sptr d = underlying_type_diff()) if (!d->traverse(v)) return false; + TRY_POST_VISIT(v); + return true; } @@ -1292,9 +1462,9 @@ pointer_diff::traverse(diff_node_visitor& v) /// /// @param ctxt the diff context to use. pointer_diff_sptr -compute_diff(pointer_type_def_sptr first, - pointer_type_def_sptr second, - diff_context_sptr ctxt) +compute_diff(pointer_type_def_sptr first, + pointer_type_def_sptr second, + diff_context_sptr ctxt) { if (diff_sptr dif = ctxt->has_diff_for(first, second)) { @@ -1364,7 +1534,11 @@ reference_diff::underlying_type_diff() const /// @param d the new diff betweend the two referred-to types. diff_sptr& reference_diff::underlying_type_diff(diff_sptr d) -{return priv_->underlying_type_diff_ = d;} +{ + priv_->underlying_type_diff_ = d; + d->set_parent(this); + return priv_->underlying_type_diff_; +} /// Getter of the length of the diff. /// @@ -1383,7 +1557,7 @@ reference_diff::length() const void reference_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; if (diff_sptr d = underlying_type_diff()) @@ -1428,11 +1602,13 @@ reference_diff::report(ostream& out, const string& indent) const bool reference_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) - return false; + TRY_PRE_VISIT(v); if (diff_sptr d = underlying_type_diff()) - return d->traverse(v); + if (!d->traverse(v)) + return false; + + TRY_POST_VISIT(v); return true; } @@ -1520,7 +1696,10 @@ qualified_type_diff::underlying_type_diff() const /// types. void qualified_type_diff::underlying_type_diff(const diff_sptr d) -{priv_->underlying_type_diff = d;} +{ + priv_->underlying_type_diff = d; + d->set_parent(this); +} /// Return the length of the diff, or zero if the two qualified types /// are equal. @@ -1579,7 +1758,7 @@ get_leaf_type(qualified_type_def_sptr t) void qualified_type_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; string fname = first_qualified_type()->get_pretty_representation(), @@ -1645,11 +1824,13 @@ qualified_type_diff::report(ostream& out, const string& indent) const bool qualified_type_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) - return false; + TRY_PRE_VISIT(v); if (diff_sptr d = underlying_type_diff()) - return d->traverse(v); + if (!d->traverse(v)) + return false; + + TRY_POST_VISIT(v); return true; } @@ -1662,9 +1843,9 @@ qualified_type_diff::traverse(diff_node_visitor& v) /// /// @param ctxt the diff context to use. qualified_type_diff_sptr -compute_diff(const qualified_type_def_sptr first, - const qualified_type_def_sptr second, - diff_context_sptr ctxt) +compute_diff(const qualified_type_def_sptr first, + const qualified_type_def_sptr second, + diff_context_sptr ctxt) { if (diff_sptr dif = ctxt->has_diff_for(first, second)) { @@ -1790,7 +1971,10 @@ enum_diff::enum_diff(const enum_type_decl_sptr first, const diff_context_sptr ctxt) : diff(first, second,ctxt), priv_(new priv) -{priv_->underlying_type_diff_ = underlying_type_diff;} +{ + priv_->underlying_type_diff_ = underlying_type_diff; + underlying_type_diff->set_parent(this); +} /// @return the first enum of the diff. const enum_type_decl_sptr @@ -1840,7 +2024,7 @@ enum_diff::length() const void enum_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; string name = first_enum()->get_pretty_representation(); @@ -1940,13 +2124,14 @@ enum_diff::report(ostream& out, const string& indent) const bool enum_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) - return false; + TRY_PRE_VISIT(v); + if (diff_sptr d = underlying_type_diff()) - { - if (!d->traverse(v)) - return false; - } + if (!d->traverse(v)) + return false; + + TRY_POST_VISIT(v); + return true; } @@ -2511,9 +2696,7 @@ class_diff::member_class_tmpls_changes() void class_diff::report(ostream& out, const string& indent) const { - int changes_length = length(); - - if (changes_length == 0) + if (!to_be_reported()) return; string name = first_subject()->get_pretty_representation(); @@ -2834,21 +3017,26 @@ class_diff::report(ostream& out, const string& indent) const i != priv_->changed_member_functions_.end(); ++i) { - if (i !=priv_->changed_member_functions_.begin()) - out << "\n"; class_decl::method_decl_sptr f = i->second.first; class_decl::method_decl_sptr s = i->second.second; - string repr = f->get_pretty_representation(); - out << indent << " '" - << repr << "' has some indirect sub-type changes:\n"; diff_sptr diff = compute_diff_for_decls(f, s, context()); - if (diff) + if (!diff) + continue; + + string repr = f->get_pretty_representation(); + if (i !=priv_->changed_member_functions_.begin()) + out << "\n"; + if (!diff->to_be_reported()) { - diff->report(out, indent + " "); - emitted = true; + out << indent << " " << repr + << " has some filtered out sub-type changes\n"; + continue; } - if (emitted) - out << "\n"; + + out << indent << " '" + << repr << "' has some sub-type changes:\n"; + diff->report(out, indent + " "); + emitted = true; } if (numchanges) out << "\n"; @@ -2972,8 +3160,7 @@ class_diff::report(ostream& out, const string& indent) const bool class_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) - return false; + TRY_PRE_VISIT(v); // base class changes. for (string_changed_base_map::const_iterator i = @@ -2984,6 +3171,7 @@ class_diff::traverse(diff_node_visitor& v) diff_sptr d = compute_diff(i->second.first, i->second.second, context()); + d->set_parent(this); if (d && !d->traverse(v)) return false; } @@ -2996,8 +3184,11 @@ class_diff::traverse(diff_node_visitor& v) if (diff_sptr d = compute_diff_for_decls(i->second.first, i->second.second, context())) - if (!d->traverse(v)) - return false; + { + d->set_parent(this); + if (!d->traverse(v)) + return false; + } // member types changes for (string_changed_type_or_decl_map::const_iterator i = @@ -3007,8 +3198,11 @@ class_diff::traverse(diff_node_visitor& v) if (diff_sptr d = compute_diff_for_types(i->second.first, i->second.second, context())) - if (!d->traverse(v)) - return false; + { + d->set_parent(this); + if (!d->traverse(v)) + return false; + } // member function changes for (string_changed_member_function_sptr_map::const_iterator i = @@ -3019,10 +3213,15 @@ class_diff::traverse(diff_node_visitor& v) if (diff_sptr d = compute_diff_for_decls(i->second.first, i->second.second, context())) - if (!d->traverse(v)) - return false; + { + d->set_parent(this); + if (!d->traverse(v)) + return false; + } } + TRY_POST_VISIT(v); + return true; } @@ -3158,7 +3357,10 @@ base_diff::get_underlying_class_diff() const /// classes. void base_diff::set_underlying_class_diff(class_diff_sptr d) -{priv_->underlying_class_diff_ = d;} +{ + priv_->underlying_class_diff_ = d; + d->set_parent(this); +} /// Getter for the length of the diff. /// @@ -3175,7 +3377,7 @@ base_diff::length() const void base_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; class_decl::base_spec_sptr f = first_base(), s = second_base(); @@ -3207,7 +3409,7 @@ base_diff::report(ostream& out, const string& indent) const if (class_diff_sptr d = get_underlying_class_diff()) { - if (d->length()) + if (d->to_be_reported()) { if (emitted) out << "\n"; @@ -3224,13 +3426,14 @@ base_diff::report(ostream& out, const string& indent) const bool base_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) - return false; + TRY_PRE_VISIT(v); if (class_diff_sptr d = get_underlying_class_diff()) if (!d->traverse(v)) return false; + TRY_POST_VISIT(v); + return true; } @@ -3666,7 +3869,7 @@ scope_diff::length() const void scope_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; // Report changed types. @@ -3791,8 +3994,7 @@ scope_diff::report(ostream& out, const string& indent) const bool scope_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) - return false; + TRY_PRE_VISIT(v); for (string_changed_type_or_decl_map::const_iterator i = changed_types().begin(); @@ -3801,8 +4003,11 @@ scope_diff::traverse(diff_node_visitor& v) if (diff_sptr d = compute_diff_for_types(i->second.first, i->second.second, context())) - if (!d->traverse(v)) - return false; + { + d->set_parent(this); + if (!d->traverse(v)) + return false; + } for (string_changed_type_or_decl_map::const_iterator i = changed_decls().begin(); @@ -3811,8 +4016,13 @@ scope_diff::traverse(diff_node_visitor& v) if (diff_sptr d = compute_diff_for_decls(i->second.first, i->second.second, context())) - if (!d->traverse(v)) - return false; + { + d->set_parent(this); + if (!d->traverse(v)) + return false; + } + + TRY_POST_VISIT(v); return true; } @@ -4083,7 +4293,7 @@ function_decl_diff::length() const void function_decl_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; maybe_report_diff_for_member(first_function_decl(), @@ -4104,7 +4314,7 @@ function_decl_diff::report(ostream& out, const string& indent) const } // Report about return type differences. - if (priv_->return_type_diff_ && priv_->return_type_diff_->length()) + if (priv_->return_type_diff_ && priv_->return_type_diff_->to_be_reported()) { out << indent << "return type changed:\n"; priv_->return_type_diff_->report(out, indent + " "); @@ -4119,15 +4329,21 @@ function_decl_diff::report(ostream& out, const string& indent) const i != priv_->changed_parms_.end(); ++i) { - out << indent - << "parameter " << i->second.first->get_index() - << " of type '" << i->second.first->get_type_pretty_representation() - << "' changed:\n"; diff_sptr d = compute_diff_for_types(i->second.first->get_type(), i->second.second->get_type(), context()); if (d) - d->report(out, indent + " "); + { + if (d->to_be_reported()) + { + out << indent + << "parameter " << i->second.first->get_index() + << " of type '" + << i->second.first->get_type_pretty_representation() + << "' changed:\n"; + d->report(out, indent + " "); + } + } } // Report about the parameters that got removed. @@ -4168,12 +4384,14 @@ function_decl_diff::report(ostream& out, const string& indent) const bool function_decl_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) - return false; + TRY_PRE_VISIT(v); if (diff_sptr d = return_type_diff()) - if (!d->traverse(v)) - return false; + { + d->set_parent(this); + if (!d->traverse(v)) + return false; + } for (string_changed_parm_map::const_iterator i = changed_parms().begin(); i != changed_parms().end(); @@ -4181,8 +4399,13 @@ function_decl_diff::traverse(diff_node_visitor& v) if (diff_sptr d = compute_diff_for_types(i->second.first->get_type(), i->second.second->get_type(), context())) - if (!d->traverse(v)) - return false; + { + d->set_parent(this); + if (!d->traverse(v)) + return false; + } + + TRY_POST_VISIT(v); return true; } @@ -4289,7 +4512,7 @@ type_decl_diff::length() const void type_decl_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; type_decl_sptr f = first_type_decl(), s = second_type_decl(); @@ -4332,8 +4555,10 @@ type_decl_diff::report(ostream& out, const string& indent) const /// otherwise. bool type_decl_diff::traverse(diff_node_visitor& v) -{return v.visit(this);} - +{ + TRY_PRE_VISIT(v); + return true; +} /// Compute a diff between two type_decl. /// /// This function doesn't actually compute a diff. As a type_decl is @@ -4425,7 +4650,10 @@ typedef_diff::underlying_type_diff() const /// the two underlying types of the typedefs. void typedef_diff::underlying_type_diff(const diff_sptr d) -{priv_->underlying_type_diff_ = d;} +{ + priv_->underlying_type_diff_ = d; + d->set_parent(this); +} /// Getter of the length of the diff between the two typedefs. /// @@ -4447,7 +4675,7 @@ typedef_diff::length() const void typedef_diff::report(ostream& out, const string& indent) const { - if (length() == 0) + if (!to_be_reported()) return; bool emit_nl = false; @@ -4477,7 +4705,7 @@ typedef_diff::report(ostream& out, const string& indent) const } diff_sptr d = underlying_type_diff(); - if (d && d->length()) + if (d && d->to_be_reported()) { if (diff_sptr d2 = context()->has_diff_for(d)) { @@ -4517,13 +4745,14 @@ typedef_diff::report(ostream& out, const string& indent) const bool typedef_diff::traverse(diff_node_visitor& v) { - if (v.visit(this)) - return false; + TRY_PRE_VISIT(v); if (diff_sptr d = underlying_type_diff()) if (!d->traverse(v)) return false; + TRY_POST_VISIT(v); + return true; } @@ -4623,14 +4852,16 @@ translation_unit_diff::report(ostream& out, const string& indent) const bool translation_unit_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) - return false; + TRY_PRE_VISIT(v); if (diff_sptr d = compute_diff(first_translation_unit(), second_translation_unit(), context())) if (!d->traverse(v)) return false; + + TRY_POST_VISIT(v); + return true; } @@ -5024,18 +5255,28 @@ corpus_diff::report(ostream& out, const string& indent) const i != priv_->changed_fns_.end(); ++i) { - out << indent << " [C]'" - << i->second.first->get_pretty_representation() - << "' has some indirect sub-type changes:\n"; - { - function_decl_sptr f(i->second.first, noop_deleter()); - function_decl_sptr s(i->second.second, noop_deleter()); + function_decl_sptr f(i->second.first, noop_deleter()); + function_decl_sptr s(i->second.second, noop_deleter()); - diff_sptr diff = compute_diff_for_decls(f, s, context()); - if (diff) + diff_sptr diff = compute_diff_for_decls(f, s, context()); + if (!diff) + continue; + + context()->maybe_apply_filters(diff); + if (diff->to_be_reported()) + { + out << indent << " [C]'" + << i->second.first->get_pretty_representation() + << "' has some indirect sub-type changes:\n"; diff->report(out, indent + " "); + } + else if (diff->is_filtered_out()) + { + out << indent << " [C]{F}'" + << i->second.first->get_pretty_representation() + << "' has some filtered out sub-type changes\n"; + } } - } if (priv_->changed_fns_.size()) out << "\n"; } @@ -5181,7 +5422,7 @@ corpus_diff::report(ostream& out, const string& indent) const bool corpus_diff::traverse(diff_node_visitor& v) { - if (!v.visit(this)) + if (!v.visit(this, true)) return false; for (string_changed_function_ptr_map::const_iterator i = @@ -5277,102 +5518,174 @@ compute_diff(const corpus_sptr f, // -/// Default visitor implementation. +/// Default visitor implementation /// /// @return true bool -diff_node_visitor::visit(distinct_diff*) +diff_node_visitor::visit(diff*, bool) {return true;} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(var_diff*) -{return true;} +diff_node_visitor::visit(distinct_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(pointer_diff*) -{return true;} +diff_node_visitor::visit(var_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(reference_diff*) -{return true;} +diff_node_visitor::visit(pointer_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(qualified_type_diff*) -{return true;} +diff_node_visitor::visit(reference_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(enum_diff*) -{return true;} +diff_node_visitor::visit(qualified_type_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(class_diff*) -{return true;} +diff_node_visitor::visit(enum_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(base_diff*) -{return true;} +diff_node_visitor::visit(class_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(scope_diff*) -{return true;} +diff_node_visitor::visit(base_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(function_decl_diff*) -{return true;} +diff_node_visitor::visit(scope_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(type_decl_diff*) -{return true;} +diff_node_visitor::visit(function_decl_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(typedef_diff*) -{return true;} +diff_node_visitor::visit(type_decl_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(translation_unit_diff*) -{return true;} +diff_node_visitor::visit(typedef_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} /// Default visitor implementation. /// /// @return true bool -diff_node_visitor::visit(corpus_diff*) +diff_node_visitor::visit(translation_unit_diff* dif, bool pre) +{ + diff* d = dif; + visit(d, pre); + + return true; +} + +/// Default visitor implementation. +/// +/// @return true +bool +diff_node_visitor::visit(corpus_diff*, bool) {return true;} // diff --git a/tools/bidiff.cc b/tools/bidiff.cc index 783cfccd..2eb80d3d 100644 --- a/tools/bidiff.cc +++ b/tools/bidiff.cc @@ -26,7 +26,7 @@ #include #include #include -#include "abg-comparison.h" +#include "abg-comp-filter.h" #include "abg-tools-utils.h" #include "abg-reader.h" #include "abg-dwarf-reader.h" @@ -63,6 +63,8 @@ struct options bool show_changed_vars; bool show_added_vars; bool show_all_vars; + bool show_harmful_changes; + bool show_harmless_changes; options() : show_stats_only(false), @@ -74,7 +76,9 @@ struct options show_deleted_vars(false), show_changed_vars(false), show_added_vars(false), - show_all_vars(true) + show_all_vars(true), + show_harmful_changes(true), + show_harmless_changes(true) {} };//end struct options; @@ -98,6 +102,8 @@ display_usage(const string prog_name, ostream& out) << " --keep keep only functions and variables matching a regex\n" << " --keep-fn keep only functions matching a regex\n" << " --keep-var keep only variables matching a regex\n" + << " --no-harmless do not display the harmless changes\n" + << " --no-harmful do not display the harmful changes\n" << " --help display this message\n"; } @@ -219,6 +225,10 @@ parse_command_line(int argc, char* argv[], options& opts) return false; opts.keep_var_regex_patterns.push_back(argv[j]); } + else if (!strcmp(argv[i], "--no-harmless")) + opts.show_harmless_changes = false; + else if (!strcmp(argv[i], "--no-harmful")) + opts.show_harmful_changes = false; else return false; } @@ -281,6 +291,11 @@ set_diff_context_from_opts(diff_context_sptr ctxt, ctxt->show_deleted_vars(opts.show_all_vars || opts.show_deleted_vars); ctxt->show_changed_vars(opts.show_all_vars || opts.show_changed_vars); ctxt->show_added_vars(opts.show_all_vars || opts.show_added_vars); + + if (!opts.show_harmless_changes) + ctxt->switch_categories_off(abigail::comparison::ACCESS_CHANGED_CATEGORY); + if (!opts.show_harmful_changes) + ctxt->switch_categories_off(abigail::comparison::SIZE_CHANGED_CATEGORY); } /// Set the regex patterns describing the functions to drop from the