This function was not aware of --leaf-changes-only mode.
- Stats counters for changed variables and types have
different names in the different modes.
- Net leaf type changes were not included in leaf mode.
For some inputs, this resulted in abidiff producing an empty report
but returning a non-zero exit status in --leaf-changes-only mode.
For other inputs the combination of both issues still resulted in the
correct return code. This included the following test-abidiff-exit
test cases:
- test-leaf-peeling
- test-leaf2
- test-no-stray-comma
This patch makes corpus_diff::has_net_changes mirror emit_diff_stats,
modulo flags like --non-reachable-types which if absent can still
result in discrepancies between output and return code.
To achieve this in a more maintainable way, the patch introduces a new interface
reporter_base::diff_has_net_changes. That interface is implemented by
all current reporters. Each reporter focuses on its own
particularities to provide the required behavious. Then
corpus_diff:has_net_changes just has to invoke
reporter_base::diff_has_net_changes on the reporter that is currently
in used.
The tests below verify that the exit code is zero when all the changes
between the test files are suppressed.
* include/abg-reporter.h ({reporter_base, default_reporter,
leaf_reporter}::diff_has_net_changes): Add new virtual function.
This breaks binary compatibility but should conserve source
compatibility.
* src/abg-default-reporter.cc
(default_reporter::diff_has_net_changes): Define new member
function.
* src/abg-leaf-reporter.cc (leaf_reporter::diff_has_net_changes):
Likewise.
* src/abg-comparison.cc (corpus_diff::has_net_changes): Invoke
reporter_base::diff_has_net_changes on the current reporter,
rather than trying to handle all the different kinds of reporters
here.
(corpus_diff::priv::apply_filters_and_compute_diff_stats): Add a
TODO to possibly delegate the implementation of this function to
the reporters.
* tests/data/Makefile.am: Add new test case files.
* tests/data/test-abidiff-exit/test-net-change-report0.txt:
Normal mode, nothing suppressed.
* tests/data/test-abidiff-exit/test-net-change-report1.txt:
Normal mode, everything suppressed.
* tests/data/test-abidiff-exit/test-net-change-report2.txt:
Leaf mode, nothing suppressed.
* tests/data/test-abidiff-exit/test-net-change-report3.txt:
Leaf mode, everything suppressions.
* tests/data/test-abidiff-exit/test-net-change-v0.c: Test file
* tests/data/test-abidiff-exit/test-net-change-v0.o: Test file
* tests/data/test-abidiff-exit/test-net-change-v1.c: Test file
* tests/data/test-abidiff-exit/test-net-change-v1.o: Test file
* tests/data/test-abidiff-exit/test-net-change.abignore: This
suppresses changes for all variables, functions and types in
the test files, except for the 'victim' function.
* tests/test-abidiff-exit.cc: Run new test cases.
Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
virtual bool diff_to_be_reported(const diff *d) const;
+ virtual bool diff_has_net_changes(const corpus_diff *d) const = 0;
+
virtual void
report(const type_decl_diff& d, std::ostream& out,
const std::string& indent = "") const = 0;
{
public:
+ virtual bool diff_has_net_changes(const corpus_diff *d) const;
+
virtual void
report(const type_decl_diff& d, std::ostream& out,
const std::string& indent = "") const;
virtual bool diff_to_be_reported(const diff *d) const;
+ virtual bool diff_has_net_changes(const corpus_diff *d) const;
+
void
report_changes_from_diff_maps(const diff_maps&, std::ostream& out,
const std::string& indent) const;
/// Emit the summary of the functions & variables that got
/// removed/changed/added.
///
+/// TODO: This should be handled by the reporters, just like what is
+/// done for reporter_base::diff_to_be_reported.
+///
/// @param out the output stream to emit the stats to.
///
/// @param indent the indentation string to use in the summary.
/// carries subtype changes that are deemed incompatible ABI changes.
bool
corpus_diff::has_net_changes() const
-{
- const diff_stats& stats = const_cast<corpus_diff*>(this)->
- apply_filters_and_suppressions_before_reporting();
-
- return (architecture_changed()
- || soname_changed()
- || stats.net_num_func_changed()
- || stats.net_num_vars_changed()
- || stats.net_num_func_added()
- || stats.net_num_added_func_syms()
- || stats.net_num_func_removed()
- || stats.net_num_removed_func_syms()
- || stats.net_num_vars_added()
- || stats.net_num_added_var_syms()
- || stats.net_num_vars_removed()
- || stats.net_num_removed_var_syms()
- || stats.net_num_added_unreachable_types()
- || stats.net_num_removed_unreachable_types()
- || stats.net_num_changed_unreachable_types());
-}
+{return context()->get_reporter()->diff_has_net_changes(this);}
/// Apply the different filters that are registered to be applied to
/// the diff tree; that includes the categorization filters. Also,
namespace comparison
{
+/// Test if a given instance of @ref corpus_diff carries changes whose
+/// reports are not suppressed by any suppression specification. In
+/// effect, these are deemed incompatible ABI changes.
+///
+/// @param d the @ref corpus_diff to consider
+///
+/// @return true iff @p d carries subtype changes that are deemed
+/// incompatible ABI changes.
+bool
+default_reporter::diff_has_net_changes(const corpus_diff *d) const
+{
+ if (!d)
+ return false;
+
+ const corpus_diff::diff_stats& stats = const_cast<corpus_diff*>(d)->
+ apply_filters_and_suppressions_before_reporting();
+
+ // Logic here should match emit_diff_stats.
+ return (d->architecture_changed()
+ || d->soname_changed()
+ || stats.net_num_func_removed()
+ || stats.net_num_func_changed()
+ || stats.net_num_func_added()
+ || stats.net_num_vars_removed()
+ || stats.net_num_vars_changed()
+ || stats.net_num_vars_added()
+ || stats.net_num_removed_unreachable_types()
+ || stats.net_num_changed_unreachable_types()
+ || stats.net_num_added_unreachable_types()
+ || stats.net_num_removed_func_syms()
+ || stats.net_num_added_func_syms()
+ || stats.net_num_removed_var_syms()
+ || stats.net_num_added_var_syms());
+}
+
/// Ouputs a report of the differences between of the two type_decl
/// involved in the @ref type_decl_diff.
///
leaf_reporter::diff_to_be_reported(const diff *d) const
{return d && d->to_be_reported() && d->has_local_changes();}
+/// Test if a given instance of @ref corpus_diff carries changes whose
+/// reports are not suppressed by any suppression specification. In
+/// effect, these are deemed incompatible ABI changes.
+///
+/// @param d the @ref corpus_diff to consider
+///
+/// @return true iff @p d carries subtype changes that are deemed
+/// incompatible ABI changes.
+bool
+leaf_reporter::diff_has_net_changes(const corpus_diff *d) const
+{
+ if (!d)
+ return false;
+
+ const corpus_diff::diff_stats& stats = const_cast<corpus_diff*>(d)->
+ apply_filters_and_suppressions_before_reporting();
+
+ // Logic here should match emit_diff_stats.
+ return (d->architecture_changed()
+ || d->soname_changed()
+ || stats.net_num_func_removed()
+ || stats.net_num_leaf_type_changes()
+ || stats.net_num_leaf_func_changes()
+ || stats.net_num_func_added()
+ || stats.net_num_vars_removed()
+ || stats.net_num_leaf_var_changes()
+ || stats.net_num_vars_added()
+ || stats.net_num_removed_unreachable_types()
+ || stats.net_num_changed_unreachable_types()
+ || stats.net_num_added_unreachable_types()
+ || stats.net_num_removed_func_syms()
+ || stats.net_num_added_func_syms()
+ || stats.net_num_removed_var_syms()
+ || stats.net_num_added_var_syms());
+}
+
/// Report the changes carried by the diffs contained in an instance
/// of @ref string_diff_ptr_map.
///
test-abidiff-exit/test-decl-enum-report.txt \
test-abidiff-exit/test-decl-enum-report-2.txt \
test-abidiff-exit/test-decl-enum-report-3.txt \
+test-abidiff-exit/test-net-change.abignore \
+test-abidiff-exit/test-net-change-v0.c \
+test-abidiff-exit/test-net-change-v0.o \
+test-abidiff-exit/test-net-change-v1.c \
+test-abidiff-exit/test-net-change-v1.o \
+test-abidiff-exit/test-net-change-report0.txt \
+test-abidiff-exit/test-net-change-report1.txt \
+test-abidiff-exit/test-net-change-report2.txt \
+test-abidiff-exit/test-net-change-report3.txt \
\
test-diff-dwarf/test0-v0.cc \
test-diff-dwarf/test0-v0.o \
--- /dev/null
+Functions changes summary: 1 Removed, 2 Changed, 1 Added functions
+Variables changes summary: 1 Removed, 1 Changed, 1 Added variables
+
+1 Removed function:
+
+ [D] 'function int fun_removed()' {fun_removed}
+
+1 Added function:
+
+ [A] 'function long int fun_added()' {fun_added}
+
+2 functions with some indirect sub-type change:
+
+ [C] 'function int fun_changed()' has some indirect sub-type changes:
+ return type changed:
+ type name changed from 'int' to 'long int'
+ type size changed from 32 to 64 (in bits)
+
+ [C] 'function void victim(type_changed*)' has some indirect sub-type changes:
+ parameter 1 of type 'type_changed*' has sub-type changes:
+ in pointed to type 'struct type_changed':
+ type size changed from 32 to 64 (in bits)
+ 1 data member change:
+ type of 'int type_changed::x' changed:
+ type name changed from 'int' to 'long int'
+ type size changed from 32 to 64 (in bits)
+
+1 Removed variable:
+
+ [D] 'int var_removed' {var_removed}
+
+1 Added variable:
+
+ [A] 'long int var_added' {var_added}
+
+1 Changed variable:
+
+ [C] 'int var_changed' was changed to 'long int var_changed':
+ size of symbol changed from 4 to 8
+ type of variable changed:
+ type name changed from 'int' to 'long int'
+ type size changed from 32 to 64 (in bits)
+
--- /dev/null
+Functions changes summary: 0 Removed (1 filtered out), 0 Changed (2 filtered out), 0 Added (1 filtered out) functions
+Variables changes summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added (1 filtered out) variables
+
--- /dev/null
+Leaf changes summary: 7 artifacts changed
+Changed leaf types summary: 1 leaf type changed
+Removed/Changed/Added functions summary: 1 Removed, 1 Changed, 1 Added function
+Removed/Changed/Added variables summary: 1 Removed, 1 Changed, 1 Added variable
+
+1 Removed function:
+
+ [D] 'function int fun_removed()' {fun_removed}
+
+1 Added function:
+
+ [A] 'function long int fun_added()' {fun_added}
+
+1 function with some sub-type change:
+
+ [C] 'function int fun_changed()' has some sub-type changes:
+ return type changed:
+ type name changed from 'int' to 'long int'
+ type size changed from 32 to 64 (in bits)
+
+1 Removed variable:
+
+ [D] 'int var_removed' {var_removed}
+
+1 Added variable:
+
+ [A] 'long int var_added' {var_added}
+
+1 Changed variable:
+
+ [C] 'int var_changed' was changed to 'long int var_changed':
+ size of symbol changed from 4 to 8
+ type of variable changed:
+ type name changed from 'int' to 'long int'
+ type size changed from 32 to 64 (in bits)
+
+'struct type_changed' changed:
+ type size changed from 32 to 64 (in bits)
+ there are data member changes:
+ type 'int' of 'type_changed::x' changed:
+ type name changed from 'int' to 'long int'
+ type size changed from 32 to 64 (in bits)
--- /dev/null
+Leaf changes summary: 0 artifact changed (3 filtered out)
+Changed leaf types summary: 0 (1 filtered out) leaf type changed
+Removed/Changed/Added functions summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added function (1 filtered out)
+Removed/Changed/Added variables summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added variable (1 filtered out)
+
--- /dev/null
+int var_removed = 0;
+int fun_removed() { return 0; }
+
+int var_changed = 0;
+int fun_changed() { return 0; }
+
+struct type_removed {
+ int x;
+};
+struct type_changed {
+ int x;
+};
+
+void victim(struct type_changed * dummy) { (void)dummy->x; }
--- /dev/null
+long var_added = 0;
+long fun_added() { return 0; }
+
+long var_changed = 0;
+long fun_changed() { return 0; }
+
+struct type_added {
+ long x;
+};
+struct type_changed {
+ long x;
+};
+
+void victim(struct type_changed * dummy) { (void)dummy->x; }
--- /dev/null
+[suppress_function]
+ name_regexp = ^fun_
+
+[suppress_variable]
+ name_regexp = ^var_
+
+[suppress_type]
+ name_regexp = ^type_
"data/test-abidiff-exit/test-decl-enum-report-3.txt",
"output/test-abidiff-exit/test-decl-enum-report-3.txt"
},
+ {
+ "data/test-abidiff-exit/test-net-change-v0.o",
+ "data/test-abidiff-exit/test-net-change-v1.o",
+ "",
+ "--no-default-suppression --no-show-locs",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE
+ | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
+ "data/test-abidiff-exit/test-net-change-report0.txt",
+ "output/test-abidiff-exit/test-net-change-report0.txt"
+ },
+ {
+ "data/test-abidiff-exit/test-net-change-v0.o",
+ "data/test-abidiff-exit/test-net-change-v1.o",
+ "data/test-abidiff-exit/test-net-change.abignore",
+ "--no-default-suppression --no-show-locs",
+ abigail::tools_utils::ABIDIFF_OK,
+ "data/test-abidiff-exit/test-net-change-report1.txt",
+ "output/test-abidiff-exit/test-net-change-report1.txt"
+ },
+ {
+ "data/test-abidiff-exit/test-net-change-v0.o",
+ "data/test-abidiff-exit/test-net-change-v1.o",
+ "",
+ "--no-default-suppression --no-show-locs --leaf-changes-only",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE
+ | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
+ "data/test-abidiff-exit/test-net-change-report2.txt",
+ "output/test-abidiff-exit/test-net-change-report2.txt"
+ },
+ {
+ "data/test-abidiff-exit/test-net-change-v0.o",
+ "data/test-abidiff-exit/test-net-change-v1.o",
+ "data/test-abidiff-exit/test-net-change.abignore",
+ "--no-default-suppression --no-show-locs --leaf-changes-only",
+ abigail::tools_utils::ABIDIFF_OK,
+ "data/test-abidiff-exit/test-net-change-report3.txt",
+ "output/test-abidiff-exit/test-net-change-report3.txt"
+ },
{0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
};