From patchwork Fri Mar 27 14:00:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39066 From: gprocida@google.com (Giuliano Procida) Date: Fri, 27 Mar 2020 14:00:15 +0000 Subject: [PATCH 5/7] abidiff: Fix variable declaration formatting. In-Reply-To: <20200327140017.1917-1-gprocida@google.com> References: <20200327140017.1917-1-gprocida@google.com> Message-ID: <20200327140017.1917-6-gprocida@google.com> The represent(var_diff_sptr) function tracks vertical "\n" and horizontal ", " spacing using two state variables: - emitted - the main diff text has been emitted - begin_with_and - main text emitted && new line started, so any further description must be indented and prefixed "and ". - if emitted is true and begin_with_and is false, then further description is prefixed with ", " instead. There was some inconsistent emission of new lines and maintenance of the state variables. This patch documents the variables and makes sure new lines and state variables are kept in sync. Finally, it is theoretically possible to reach the end of the function without emitted becoming true. This patch adds a TODO and makes sure at something is output shoud that ever happen. * src/abg-reporter-priv.cc: (represent) In the var_diff_sptr overload, make sure the state variables begin_with_and and emitted are updated consistently; add a TODO for one case which may result in the end of the function being reached without having emitted a report; add missing new lines following reporting of anonymous member changes; only emit a final new line if begin_with_and hasn't tracked one already; document state variables. * tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt: Add missing blank line after anonymous member change text. * tests/data/test-diff-filter/test44-anonymous-data-member-report-1.txt: Add missing "and " continuation. Signed-off-by: Giuliano Procida --- src/abg-reporter-priv.cc | 42 ++++++++++++++++--- .../test-leaf-peeling-report.txt | 2 - .../test-abidiff-exit/test-leaf2-report.txt | 1 - .../test45-anon-dm-change-report-0.txt | 2 + .../libtest45-basic-type-change-report-1.txt | 3 -- .../test44-anonymous-data-member-report-1.txt | 2 +- 6 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc index f66a7721..bb5b869d 100644 --- a/src/abg-reporter-priv.cc +++ b/src/abg-reporter-priv.cc @@ -412,7 +412,9 @@ represent(const var_diff_sptr &diff, var_decl_sptr o = diff->first_var(); var_decl_sptr n = diff->second_var(); + // Has the main diff text been output? bool emitted = false; + // Are we continuing on a new line? (implies emitted) bool begin_with_and = false; string name1 = o->get_qualified_name(); string name2 = n->get_qualified_name(); @@ -435,6 +437,9 @@ represent(const var_diff_sptr &diff, << indent << " " << tr1 << "\n" << indent << "to:\n" << indent << " " << tr2 << "\n"; + + begin_with_and = true; + emitted = true; } else if (get_type_name(t1) != get_type_name(t2) && is_decl(t1) && is_decl(t2) @@ -451,7 +456,11 @@ represent(const var_diff_sptr &diff, << indent << " This is usually due to " << "an anonymous member type being added or removed from " << "the containing type\n"; + + begin_with_and = true; + emitted = true; } + // TODO: else ...? } else if (filtering::has_anonymous_data_member_change(diff)) { @@ -468,7 +477,7 @@ represent(const var_diff_sptr &diff, *ctxt, out); out << " became data member '" << n->get_pretty_representation() - << "'"; + << "'\n"; } else { @@ -482,8 +491,11 @@ represent(const var_diff_sptr &diff, *ctxt, out); out << " became anonymous data member '" << n->get_pretty_representation() - << "'"; + << "'\n"; } + + begin_with_and = true; + emitted = true; } else if (diff_sptr d = diff->type_diff()) { @@ -522,7 +534,9 @@ represent(const var_diff_sptr &diff, out << indent << " details were reported earlier\n"; else d->report(out, indent + " "); + begin_with_and = true; + emitted = true; } } } @@ -535,12 +549,15 @@ represent(const var_diff_sptr &diff, ; else { - out << indent; if (begin_with_and) { - out << "and "; + out << indent << "and "; begin_with_and = false; } + else if (!emitted) + out << indent; + else + out << ", "; out << "name of '" << name1 << "' changed to '" << name2 << "'"; report_loc_info(n, *ctxt, out); emitted = true; @@ -689,7 +706,20 @@ represent(const var_diff_sptr &diff, out << "now becomes static"; emitted = true; } - if (emitted) + + if (begin_with_and) + // do nothing as begin_with_and implies emitted + ; + else if (!emitted) + // We appear to have fallen off the edge of the map. + out << indent << "'" << pretty_representation + << "' has *some* difference - please report as a bug"; + else + // do nothing + ; + emitted = true; + + if (!begin_with_and) out << "\n"; } @@ -956,7 +986,7 @@ report_mem_header(ostream& out, if (num_filtered) out << " (" << num_filtered << " filtered)"; - out << colon_or_semi_colon << '\n'; + out << colon_or_semi_colon << "\n"; } /// Output the header preceding the the report for diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt index eeffc7b5..c2e5e0fb 100644 --- a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt +++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt @@ -19,7 +19,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable pointer type changed from: 'int*' to: 'int**' - 'struct ops2 at test-leaf-peeling.0.cc:9:1' changed: type size changed from 320 to 640 (in bits) there are data member changes: @@ -31,4 +30,3 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable there are data member changes: type 'void (int&)*' of 'ops3::spong' changed: pointer type changed from: 'void (int&)*' to: 'void (int&&)*' - diff --git a/tests/data/test-abidiff-exit/test-leaf2-report.txt b/tests/data/test-abidiff-exit/test-leaf2-report.txt index 88e18962..1caf2cba 100644 --- a/tests/data/test-abidiff-exit/test-leaf2-report.txt +++ b/tests/data/test-abidiff-exit/test-leaf2-report.txt @@ -8,4 +8,3 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable there are data member changes: type 'void (int)*' of 'ops::munge' changed: pointer type changed from: 'void (int)*' to: 'char (long int, bool)*' - diff --git a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt index f7ad7cbc..12af287b 100644 --- a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt +++ b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt @@ -13,9 +13,11 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 'char S1::m1' offset changed from 64 to 40 (in bits) (by -24 bits) 1 data member change: anonymous data member struct {int m0; char m01;} at offset 0 (in bits) became data member 'int S1::m0' + [C] 'function void foo(S0&)' has some indirect sub-type changes: parameter 1 of type 'S0&' has sub-type changes: in referenced type 'struct S0': type size hasn't changed 1 data member change: data member int S0::m0 at offset 0 (in bits) became anonymous data member 'union {int m0; char m01;}' + diff --git a/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt b/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt index d2cb8097..eecb84a0 100644 --- a/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt +++ b/tests/data/test-diff-filter/libtest45-basic-type-change-report-1.txt @@ -29,7 +29,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable pointer type changed from: 'int*' to: 'char*' - 'struct S1 at test45-basic-type-change-v0.cc:13:1' changed: type size hasn't changed there are data member changes: @@ -37,10 +36,8 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable pointer type changed from: 'int*' to: 'char*' - 'struct S2 at test45-basic-type-change-v0.cc:18:1' changed: type size hasn't changed there are data member changes: type 'int*' of 'S2::m0' changed: pointer type changed from: 'int*' to: 'char*' - diff --git a/tests/data/test-diff-filter/test44-anonymous-data-member-report-1.txt b/tests/data/test-diff-filter/test44-anonymous-data-member-report-1.txt index ca32f5eb..0979891a 100644 --- a/tests/data/test-diff-filter/test44-anonymous-data-member-report-1.txt +++ b/tests/data/test-diff-filter/test44-anonymous-data-member-report-1.txt @@ -10,5 +10,5 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable struct {int b; float c;} to: struct {int b; float c; char e;} - size changed from 64 to 96 (in bits) (by +32 bits) + and size changed from 64 to 96 (in bits) (by +32 bits) 'int S2::d' offset changed from 96 to 128 (in bits) (by +32 bits)