* [PATCH] Bug 29144 - abidiff doesn't report base class re-ordering
@ 2022-06-07 9:59 Dodji Seketeli
0 siblings, 0 replies; only message in thread
From: Dodji Seketeli @ 2022-06-07 9:59 UTC (permalink / raw)
To: gprocida; +Cc: libabigail
Hello Giuliano,
Libabigail fails to report when a base class gets re-ordered, even
though the comparison engine detects the change.
This patch detects the base class re-ordering in
class_diff::ensure_lookup_tables_populated by interpreting the edit
script resulting of the comparison, populates a new
class_diff::priv::moved_bases_ vector with the bases that "changed
position" and adds a new class_diff::moved_bases() accessor for the "base
classes that changed position".
A new function maybe_report_base_class_reordering is defined to
actually report a base class re-ordering if class_diff::moved_bases()
returns a non-empty vector, meaning that class has some base classes
that got re-ordered.
That new function is thus called in the overload of
{default_reporter, leaf_reporter}::report() for class_diff to emit a
description of the change when necessary.
Would you this patch work for your testing case?
Thanks.
* include/abg-comparison.h (class_diff::moved_bases): Declare new
method.
* src/abg-comparison-priv.h (class_diff::priv::moved_bases_):
Define new data member.
* src/abg-reporter-priv.h (maybe_report_base_class_reordering):
Declare new function.
* src/abg-reporter-priv.cc (maybe_report_base_class_reordering):
Define new function.
* src/abg-comparison.cc
(class_diff::ensure_lookup_tables_populated): Detect that a base
class moved position and record it into
class_diff::priv::moved_bases_ data member.
* src/abg-default-reporter.cc (default_reporter::report): Use the
new maybe_report_base_class_reordering.
* src/abg-leaf-reporter.cc (leaf_reporter::report): Likewise.
* tests/data/test-abidiff-exit/test-PR29144-report-2.txt: New
testing input file.
* tests/data/test-abidiff-exit/test-PR29144-report.txt: Likewise.
* tests/data/test-abidiff-exit/test-PR29144-v0.cc: Likewise.
* tests/data/test-abidiff-exit/test-PR29144-v0.o: Likewise.
* tests/data/test-abidiff-exit/test-PR29144-v1.cc: Likewise.
* tests/data/test-abidiff-exit/test-PR29144-v1.o: Likewise.
* tests/data/Makefile.am: Add the new files to source
distribution.
* tests/test-abidiff-exit.cc (in_out_specs): Add the new tests to
this harness.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
include/abg-comparison.h | 3 ++
src/abg-comparison-priv.h | 1 +
src/abg-comparison.cc | 15 ++++++
src/abg-default-reporter.cc | 3 ++
src/abg-leaf-reporter.cc | 2 +
src/abg-reporter-priv.cc | 48 ++++++++++++++++++
src/abg-reporter-priv.h | 4 ++
tests/data/Makefile.am | 16 ++++--
.../test-PR29144-report-2.txt | 8 +++
.../test-abidiff-exit/test-PR29144-report.txt | 10 ++++
.../data/test-abidiff-exit/test-PR29144-v0.cc | 17 +++++++
.../data/test-abidiff-exit/test-PR29144-v0.o | Bin 0 -> 2560 bytes
.../data/test-abidiff-exit/test-PR29144-v1.cc | 18 +++++++
.../data/test-abidiff-exit/test-PR29144-v1.o | Bin 0 -> 2560 bytes
tests/test-abidiff-exit.cc | 22 ++++++++
15 files changed, 162 insertions(+), 5 deletions(-)
create mode 100644 tests/data/test-abidiff-exit/test-PR29144-report-2.txt
create mode 100644 tests/data/test-abidiff-exit/test-PR29144-report.txt
create mode 100644 tests/data/test-abidiff-exit/test-PR29144-v0.cc
create mode 100644 tests/data/test-abidiff-exit/test-PR29144-v0.o
create mode 100644 tests/data/test-abidiff-exit/test-PR29144-v1.cc
create mode 100644 tests/data/test-abidiff-exit/test-PR29144-v1.o
diff --git a/include/abg-comparison.h b/include/abg-comparison.h
index 78e4102e..f5169aeb 100644
--- a/include/abg-comparison.h
+++ b/include/abg-comparison.h
@@ -1716,6 +1716,9 @@ public:
const base_diff_sptrs_type&
changed_bases();
+ const vector<class_decl::base_spec_sptr>&
+ moved_bases() const;
+
virtual bool
has_changes() const;
diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h
index ef271d8b..0953d567 100644
--- a/src/abg-comparison-priv.h
+++ b/src/abg-comparison-priv.h
@@ -613,6 +613,7 @@ struct class_diff::priv
class_decl::base_specs sorted_inserted_bases_;
string_base_diff_sptr_map changed_bases_;
base_diff_sptrs_type sorted_changed_bases_;
+ vector<class_decl::base_spec_sptr> moved_bases_;
class_decl::base_spec_sptr
base_has_changed(class_decl::base_spec_sptr) const;
diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 2ce3f5d4..1313e8fd 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -5217,6 +5217,11 @@ class_diff::ensure_lookup_tables_populated(void) const
if (j->second != b)
get_priv()->changed_bases_[name] =
compute_diff(j->second, b, context());
+ else
+ // The base class changed place. IOW, the base
+ // classes got re-arranged. Let's keep track of the
+ // base classes that moved.
+ get_priv()->moved_bases_.push_back(b);
get_priv()->deleted_bases_.erase(j);
}
else
@@ -5547,6 +5552,16 @@ const base_diff_sptrs_type&
class_diff::changed_bases()
{return get_priv()->sorted_changed_bases_;}
+/// Getter for the vector of bases that "moved".
+/// That is, the vector of base types which position changed. If this
+/// vector is not empty, it means the bases of the underlying class
+/// type got re-ordered.
+///
+/// @return the vector of bases that moved.
+const vector<class_decl::base_spec_sptr>&
+class_diff::moved_bases() const
+{return get_priv()->moved_bases_;}
+
/// @return the edit script of the bases of the two classes.
edit_script&
class_diff::base_changes()
diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index a5cebef4..909d1de4 100644
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -1348,6 +1348,9 @@ default_reporter::report(const class_diff& d, ostream& out,
out << "\n";
}
}
+
+ // Report base classes re-organisation
+ maybe_report_base_class_reordering(d, out, indent);
}
d.class_or_union_diff::report(out, indent);
diff --git a/src/abg-leaf-reporter.cc b/src/abg-leaf-reporter.cc
index 07391720..79b0286b 100644
--- a/src/abg-leaf-reporter.cc
+++ b/src/abg-leaf-reporter.cc
@@ -691,6 +691,8 @@ leaf_reporter::report(const class_diff& d,
d.class_or_union_diff::report(out, indent);
+ maybe_report_base_class_reordering(d, out, indent);
+
maybe_report_interfaces_impacted_by_diff(&d, out, indent);
d.reported_once(true);
diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
index 7012f5dc..06145a1b 100644
--- a/src/abg-reporter-priv.cc
+++ b/src/abg-reporter-priv.cc
@@ -1451,5 +1451,53 @@ maybe_report_data_members_replaced_by_anon_dm(const class_or_union_diff &d,
}
}
+/// Report about the base classes of a class having been re-ordered.
+///
+/// @param d the class diff to consider.
+///
+/// @param out the output stream to report the change to.
+///
+/// @param indent the indentation string to use.
+void
+maybe_report_base_class_reordering(const class_diff &d,
+ ostream &out,
+ const string &indent)
+{
+ if (d.moved_bases().empty())
+ return;
+
+ class_decl_sptr first = d.first_class_decl(),
+ second = d.second_class_decl();
+
+ ABG_ASSERT(!first->get_base_specifiers().empty());
+ ABG_ASSERT(!second->get_base_specifiers().empty());
+
+ out << indent << "base classes of '"
+ << first->get_pretty_representation()
+ << "' are re-ordered from: ";
+
+ vector<class_decl_sptr> classes = {first, second};
+ unsigned nb_classes_seen = 0;
+ for (auto &klass : classes)
+ {
+ if (nb_classes_seen >= 1)
+ out << " to: ";
+ out << "'";
+ bool needs_comma = false;
+ for (auto &b : klass->get_base_specifiers())
+ {
+ if (needs_comma)
+ out << ", ";
+ if (b->get_is_virtual())
+ out << "virtual ";
+ out << b->get_base_class()->get_qualified_name();
+ needs_comma = true;
+ }
+ out << "'";
+ nb_classes_seen++;
+ }
+ if (nb_classes_seen)
+ out << "\n";
+}
} // Namespace comparison
} // end namespace abigail
diff --git a/src/abg-reporter-priv.h b/src/abg-reporter-priv.h
index a7c4878c..42425ef2 100644
--- a/src/abg-reporter-priv.h
+++ b/src/abg-reporter-priv.h
@@ -241,6 +241,10 @@ maybe_report_data_members_replaced_by_anon_dm(const class_or_union_diff &d,
const string indent);
+void
+maybe_report_base_class_reordering(const class_diff &d,
+ ostream &out,
+ const string &indent);
} // end namespace comparison
} // end namespace abigail
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index ee23aff6..a0c086d9 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -98,12 +98,16 @@ test-abidiff/test-PR27985-v0.o.abi \
test-abidiff/test-PR27985-v1.c \
test-abidiff/test-PR27985-v1.o \
test-abidiff/test-PR27985-v1.o.abi \
+test-abidiff/test-PR27616-squished-v0.abi \
+test-abidiff/test-PR27616-squished-v1.abi \
+test-abidiff/test-PR27616-v0.xml \
+test-abidiff/test-PR27616-v1.xml \
+\
test-abidiff-exit/test-PR28316-v0.cc \
test-abidiff-exit/test-PR28316-v1.cc \
test-abidiff-exit/test-PR28316-v0.o \
test-abidiff-exit/test-PR28316-v1.o \
test-abidiff-exit/test-PR28316-report.txt \
-\
test-abidiff-exit/test1-voffset-change-report0.txt \
test-abidiff-exit/test1-voffset-change-report1.txt \
test-abidiff-exit/test1-voffset-change.abignore \
@@ -218,10 +222,12 @@ test-abidiff-exit/test-crc-v1.abi \
test-abidiff-exit/test-missing-alias-report.txt \
test-abidiff-exit/test-missing-alias.abi \
test-abidiff-exit/test-missing-alias.suppr \
-test-abidiff/test-PR27616-squished-v0.abi \
-test-abidiff/test-PR27616-squished-v1.abi \
-test-abidiff/test-PR27616-v0.xml \
-test-abidiff/test-PR27616-v1.xml \
+test-abidiff-exit/test-PR29144-report-2.txt \
+test-abidiff-exit/test-PR29144-report.txt \
+test-abidiff-exit/test-PR29144-v0.cc \
+test-abidiff-exit/test-PR29144-v0.o \
+test-abidiff-exit/test-PR29144-v1.cc \
+test-abidiff-exit/test-PR29144-v1.o \
\
test-diff-dwarf/test0-v0.cc \
test-diff-dwarf/test0-v0.o \
diff --git a/tests/data/test-abidiff-exit/test-PR29144-report-2.txt b/tests/data/test-abidiff-exit/test-PR29144-report-2.txt
new file mode 100644
index 00000000..3aff12d4
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-PR29144-report-2.txt
@@ -0,0 +1,8 @@
+Leaf changes summary: 1 artifact changed
+Changed leaf types summary: 1 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+'struct D at 1.cc:13:1' changed:
+ type size hasn't changed
+ base classes of 'struct D' are re-ordered from: 'A, B, C' to: 'B, A, C'
diff --git a/tests/data/test-abidiff-exit/test-PR29144-report.txt b/tests/data/test-abidiff-exit/test-PR29144-report.txt
new file mode 100644
index 00000000..88c78fdf
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-PR29144-report.txt
@@ -0,0 +1,10 @@
+Functions changes summary: 0 Removed, 0 Changed, 0 Added function
+Variables changes summary: 0 Removed, 1 Changed, 0 Added variable
+
+1 Changed variable:
+
+ [C] 'D order' was changed at 2.cc:17:1:
+ type of variable changed:
+ type size hasn't changed
+ base classes of 'struct D' are re-ordered from: 'A, B, C' to: 'B, A, C'
+
diff --git a/tests/data/test-abidiff-exit/test-PR29144-v0.cc b/tests/data/test-abidiff-exit/test-PR29144-v0.cc
new file mode 100644
index 00000000..8805cfd9
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-PR29144-v0.cc
@@ -0,0 +1,17 @@
+struct A {
+ int x;
+};
+
+struct B {
+ int y;
+};
+
+struct C {
+ int z;
+};
+
+struct D: A, B, C {
+ int d;
+};
+
+D order;
diff --git a/tests/data/test-abidiff-exit/test-PR29144-v0.o b/tests/data/test-abidiff-exit/test-PR29144-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..2964a6802a82dbcbcaad22cb5a6be6de85804576
GIT binary patch
literal 2560
zcmbVNOK;Oa5T3P@l42T~M&i|qxClgf#A%|?mZFwGfPz4XhY%;IoMf9C^1^XZ3kNE}
z0SR$HLL4}A;K+?Dzk?Hhfink~S?>mSYXo8>@63EV-|Toin>@RC?UKP5C}FS&zdeNl
zRFV-}m=(yv7`#I}2`oiWf|f~Il(2J<U`6zdbudU{a3bk-0$}GMk-SPf8!!~^uqq^m
z?-TR{c1q|1jE+1<$UKM(m~*%!OroWX2Baxy7YyMFEMpRN*d#{?%Q7=%S@V<ZjCFc4
z#j==q1k98Kc3jEMS(tJHLL-r3N77l#%#G(p_hOlc!Lk-D6JKno3}$-X!YtO6?AiUs
zK1@G>k0Fi`Z!WCpQbH#QDL9UrG4>-pLW&xT4;neqV}s?4-C-I*?)nYx!_xITcJ;)G
z;;daPIYp;vm!?al=~L6Q_S7xjurIrT+{hP?+4**`*5M0H-r>GiN7wc1s|)LMGx?da
zoo_;6wcF-}Mz`_6D>S{J(DG_-&1<?|tFYp&^M0Y{2fQEj3pZ|+P8Z8%C_44}f3gG>
zma5fxdkSCT*gw=SX2!;sr}07#HkEx5b0iu<?FxO-FNwt|+E2jupK*I;IGNkF%y4vK
z@ZoJ30Vd$U#E6MQs0$5c1{q35l_8oGerST!`0PaccQn9=j3=CWBZeIHSOh05ye9yA
zG+erHAc9Bx`XYiyt~R`pIQ4Ufv`MM+sh-Hh!XX($IK|9J|D=XDrLXE$@hypi)8A+Z
zZVh$dhk8|60bdWmX}Ez4POaYu$LB5AY4F-wbJ^>xba7AEZms6?$6K~t-|aN{R!Yn3
za7Do*lu7Wi@ax@no8xbk)9D7>!Kuvm1GoNA`pwRo)APGM?gtyH`2W}ANTpt@{;EtB
zWoTNz2a~u@Saf!XQlTYAbpBC{p&9%We=xT)m+sT|)N@j3g%*_ep&G1uEX@hcWn~Z%
z8k@49j}oVIDNfxN`Vl5sz(>zX_*Ys`?uBZw>ajO)N5?35R)RJ;5z%AmzbLN$DFRVc
zom4;FC%yg#VtP)5;I0JKeIkrRk6lJA?)-{8|FU2K(E0SMrk~$KjB-xn6P4HUpxaRp
zZFRoVPbH@FzsZ0{f8oFS1OKKG5(|Ifzm<F~3^pZS$5q`5|9}R47d>3@_4iW!Q+&#&
rb7}9B1iz4jlEEkL`_nhk?;geXB&h$h==D?HIQ~od{XbK(i0J%ZNgcXU
literal 0
HcmV?d00001
diff --git a/tests/data/test-abidiff-exit/test-PR29144-v1.cc b/tests/data/test-abidiff-exit/test-PR29144-v1.cc
new file mode 100644
index 00000000..eb476828
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-PR29144-v1.cc
@@ -0,0 +1,18 @@
+struct A {
+ int x;
+};
+
+struct B {
+ int y;
+};
+
+struct C {
+ int z;
+};
+
+struct D: B, A, C {
+ int d;
+};
+
+D order;
+
diff --git a/tests/data/test-abidiff-exit/test-PR29144-v1.o b/tests/data/test-abidiff-exit/test-PR29144-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..03eb7a91c2c5768af8fab0e3eca907ddd481d8f2
GIT binary patch
literal 2560
zcmbVNOK;Oa5T3P@l42T~M&i|qxClgf#A%|?mZFwGfPz4XhY%;IoMf9C^1^XZ3kNE}
z0SR$HLL4}A;K+?Dzk?Hhfink~S?>mSYXo8>@63EV-|Toin>@RC?UKP5C}FS&zdeNl
zRFV-}m=(yv7`#I}2`oiWf|f~Il(2J<U`6zdbudU{a3bk-0$}GMk-SPf8!!~^uqq^m
z?-TR{c1q|1jE+1<$ehCk%!9ZjOroWX2Baxy7YyMFEMpRN*d#{?%Q7=%S@V<ZjCFc4
z#j==q1k98Kc3jEMS(tJHLL-r3N77l#%#G(p_hOlc!Lk-D6JKno3}$-X!YtO6?AiUs
zK1@G>k0Fi`Z!WCpQbH#QDL9UrG4>-pLW&xT4;neqV}s?4-C-I*?)nYx!_xITcJ;)G
z;;daPIYp;vm!?al=~L6Q_S7xjurIrT+{hP?+4**`*5M0H-r>GiN7wc1s|)LMGx?da
zoo_;6wcF-}Mz`_6D>S{J(DG_-&1<?|tFYp&^M0Y{2fQEj3pZ|+P8Z8%C^_}|f3gG>
zma5fxdkSCT*gw=SX2!;sr}07#HkEx5b0iu<?FxO-FNwt|+E2jupK*I;IGNkF%y4vK
z@ZoJ30Vd$U#E6MQs0$5c1{q35l_8oGerST!`0PaccQn9=j3=CWBZeIHSOh05ye9yA
zG+erHAc9Bx`XYiyt~R`pIQ4Ufv`MM+sh-Hh!XX($IK|9J|D=XDrLXE$@hypi)8A+Z
zZVh$dhk8|60bdWmX}Ez4POaYu$LB5AY4F-wbJ^>xba7AEZms6?$6K~t-|aN{R!Yn3
za7Do*lu7Wi@ax@no8xbk)9D7>!Kuvm1GoNA`pwRo)APGM?gtyH`2W}ANTpt@{;EtB
zWoTNz2a~u@Saf!XQlTYAbpBC{p&9%We=xT)m+sT|)N@j3g%*_ep&G1uEX@hcWn~Z%
z8k@49j}oVIDNfxN`Vl5sz(>zX_*Ys`?uBZw>ajO)N5?35R)RJ;5z%AmzbLN$DFRVc
zom4;FC%yg#VtP)5;I0JKeIkrRk6lJA?)-{8|FU2K(E0SMrk~$KjB-xn6P4HUpxaRp
zZFRoVPbH@FzsZ0{f8oFS1OKKG5(|Ifzm<F~3^pZS$5q`5|9}R47d>3@_4iW!Q+&#&
rb7}9B1iz4jlEEkL`_nhk?;geXB&h$h==D?HIQ~od{XbK(i0J%ZXQaAT
literal 0
HcmV?d00001
diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 15acfed9..4932d949 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -427,6 +427,28 @@ InOutSpec in_out_specs[] =
"data/test-abidiff-exit/test-PR28316-report.txt",
"output/test-abidiff-exit/test-PR28316-report.txt"
},
+ {
+ "data/test-abidiff-exit/test-PR29144-v0.o",
+ "data/test-abidiff-exit/test-PR29144-v1.o",
+ "",
+ "",
+ "",
+ "--no-default-suppression --harmless",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+ "data/test-abidiff-exit/test-PR29144-report.txt",
+ "output/test-abidiff-exit/test-PR29144-report.txt"
+ },
+ {
+ "data/test-abidiff-exit/test-PR29144-v0.o",
+ "data/test-abidiff-exit/test-PR29144-v1.o",
+ "",
+ "",
+ "",
+ "--leaf-changes-only --no-default-suppression --harmless",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+ "data/test-abidiff-exit/test-PR29144-report-2.txt",
+ "output/test-abidiff-exit/test-PR29144-report-2.txt"
+ },
{0, 0, 0 ,0, 0, 0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
};
--
2.36.1
--
Dodji
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-06-07 9:59 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-07 9:59 [PATCH] Bug 29144 - abidiff doesn't report base class re-ordering Dodji Seketeli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).