[5/7] abidiff: Fix variable declaration formatting.

Message ID 20200327140017.1917-6-gprocida@google.com
State Committed
Headers
Series Mostly whitespace changes and fixes. |

Commit Message

Giuliano Procida March 27, 2020, 2 p.m. UTC
  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 <gprocida@google.com>
---
 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(-)
  

Patch

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)