Re: [PATCH 1/3] abidiff: Remove blank line after typedef changes.

[Matthias Maennich <maennich@google.com>]

On Sun, Mar 29, 2020 at 06:01:19PM +0100, Giuliano Procida wrote:
>This patch removes perhaps the last remaining cause of double blank
>lines in output.
>
>The state variable emit_nl was being set to true just after emitting a
>new line which resulted in a blank line after every local typedef
>change report.
>
>	* include/abg-reporter.h
>	(default_reporter::report_local_typedef_changes): Change
>	return type to void.
>	* src/abg-default-reporter.cc:
>	(default_reporter::report_local_typedef_changes): Change
>	return type to void, remove emit_nl state variable and logic.
>	* tests/data/test-abidiff/test-PR18791-report0.txt: Remove
>	blank lines.
>	* tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt:
>	Ditto.
>	* tests/data/test-diff-suppr/test39-opaque-type-report-0.txt:
>	Ditto.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> include/abg-reporter.h                         |  2 +-
> src/abg-default-reporter.cc                    | 18 +++---------------
> .../data/test-abidiff/test-PR18791-report0.txt |  1 -
> .../test42-PR21296-clanggcc-report0.txt        |  3 ---
> .../test39-opaque-type-report-0.txt            |  1 -
> 5 files changed, 4 insertions(+), 21 deletions(-)
>
>diff --git a/include/abg-reporter.h b/include/abg-reporter.h
>index b1ae88d1..6254be6a 100644
>--- a/include/abg-reporter.h
>+++ b/include/abg-reporter.h
>@@ -170,7 +170,7 @@ public:
>   report(const enum_diff& d, ostream& out,
> 	 const string& indent = "") const;
>
>-  bool
>+  void
>   report_local_typedef_changes(const typedef_diff &d,
> 			       ostream& out,
> 			       const string& indent) const;
>diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
>index d2ac4a58..ff7ed67e 100644
>--- a/src/abg-default-reporter.cc
>+++ b/src/abg-default-reporter.cc
>@@ -183,18 +183,14 @@ default_reporter::report(const enum_diff& d, ostream& out,
> /// @param out the output stream to report to.
> ///
> /// @param indent the white space string to use for indentation.
>-///
>-/// @return true iff the caller needs to emit a newline to the output
>-/// stream before emitting anything else.
>-bool
>+void
> default_reporter::report_local_typedef_changes(const typedef_diff &d,
> 					       ostream& out,
> 					       const string& indent) const
> {
>   if (!d.to_be_reported())
>-    return false;
>+    return;
>
>-  bool emit_nl = false;
>   typedef_decl_sptr f = d.first_typedef_decl(), s = d.second_typedef_decl();
>
>   maybe_report_diff_for_member(f, s, d.context(), out, indent);
>@@ -211,10 +207,7 @@ default_reporter::report_local_typedef_changes(const typedef_diff &d,
> 	  << s->get_qualified_name();
>       report_loc_info(s, *d.context(), out);
>       out << "\n";
>-      emit_nl = true;
>     }
>-
>-  return emit_nl;
> }
>
> /// Reports the difference between the two subjects of the diff in a
>@@ -235,7 +228,7 @@ default_reporter::report(const typedef_diff& d,
>
>   typedef_decl_sptr f = d.first_typedef_decl(), s = d.second_typedef_decl();
>
>-  bool emit_nl = report_local_typedef_changes(d, out, indent);
>+  report_local_typedef_changes(d, out, indent);
>
>   diff_sptr dif = d.underlying_type_diff();
>   if (dif && dif->has_changes())
>@@ -250,7 +243,6 @@ default_reporter::report(const typedef_diff& d,
> 	  report_loc_info(dif->first_subject(), *d.context(), out);
> 	  out << " changed:\n";
> 	  dif->report(out, indent + "  ");
>-	  emit_nl = false;
> 	}
>       else
> 	{
>@@ -274,14 +266,10 @@ default_reporter::report(const typedef_diff& d,
> 	      dif->report(out, indent + "  ");
> 	      if (c & REDUNDANT_CATEGORY)
> 		dif->set_category(c | REDUNDANT_CATEGORY);
>-	      emit_nl = false;
> 	    }
> 	}
>     }
>
>-  if (emit_nl)
>-    out << "\n";
>-
>   d.reported_once(true);
> }
>
>diff --git a/tests/data/test-abidiff/test-PR18791-report0.txt b/tests/data/test-abidiff/test-PR18791-report0.txt
>index 87322587..93faf599 100644
>--- a/tests/data/test-abidiff/test-PR18791-report0.txt
>+++ b/tests/data/test-abidiff/test-PR18791-report0.txt
>@@ -74,7 +74,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>     return type changed:
>       underlying type 'typedef std::list<sigc::slot_base, std::allocator<sigc::slot_base> >::iterator' changed:
>         typedef name changed from std::list<sigc::slot_base, std::allocator<sigc::slot_base> >::iterator to std::__cxx11::list<sigc::slot_base, std::allocator<sigc::slot_base> >::iterator
>-
>     parameter 1 of type 'const sigc::slot_base&' has sub-type changes:
>       in referenced type 'const sigc::slot_base':
>         unqualified underlying type 'class sigc::slot_base' changed, as reported earlier
>diff --git a/tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt b/tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt
>index ff9e2841..087cfd4e 100644
>--- a/tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt
>+++ b/tests/data/test-diff-dwarf/test42-PR21296-clanggcc-report0.txt
>@@ -43,7 +43,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>     'function std::__add_c_ref<STR&&>::type std::__get_helper<0ul, STR&&>(const std::_Tuple_impl<0ul, STR&&>&) {_ZSt12__get_helperILm0EO3STRJEENSt11__add_c_refIT0_E4typeERKSt11_Tuple_implIXT_EJS3_DpT1_EE}' now becomes 'function std::__add_c_ref<STR &&>::type std::__get_helper<0, STR &&>(const std::_Tuple_impl<0, STR &&>&) {_ZSt12__get_helperILm0EO3STRJEENSt11__add_c_refIT0_E4typeERKSt11_Tuple_implIXT_EJS3_DpT1_EE}'
>     return type changed:
>       typedef name changed from std::__add_c_ref<STR&&>::type to std::__add_c_ref<STR &&>::type
>-
>     parameter 1 of type 'const std::_Tuple_impl<0ul, STR&&>&' changed:
>       in referenced type 'const std::_Tuple_impl<0ul, STR&&>':
>         'const std::_Tuple_impl<0ul, STR&&>' changed to 'const std::_Tuple_impl<0, STR &&>'
>@@ -52,7 +51,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>     'function std::__add_c_ref<STR&&>::type std::get<0ul, STR&&>(const std::tuple<STR&&>&) {_ZSt3getILm0EJO3STREENSt11__add_c_refINSt13tuple_elementIXT_ESt5tupleIJDpT0_EEE4typeEE4typeERKS7_}' now becomes 'function std::__add_c_ref<STR &&>::type std::get<0, STR &&>(const std::tuple<STR &&>&) {_ZSt3getILm0EJO3STREENSt11__add_c_refINSt13tuple_elementIXT_ESt5tupleIJDpT0_EEE4typeEE4typeERKS7_}'
>     return type changed:
>       typedef name changed from std::__add_c_ref<STR&&>::type to std::__add_c_ref<STR &&>::type
>-
>     parameter 1 of type 'const std::tuple<STR&&>&' changed:
>       in referenced type 'const std::tuple<STR&&>':
>         'const std::tuple<STR&&>' changed to 'const std::tuple<STR &&>'
>@@ -63,7 +61,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>       in referenced type 'typedef std::remove_reference<STR&>::type':
>         typedef name changed from std::remove_reference<STR&>::type to std::remove_reference<STR &>::type
>
>-
>   [C] 'method void std::tuple<STR&&>::tuple<STR>(STR&&)' has some indirect sub-type changes:
>     Please note that the symbol of this function is _ZNSt5tupleIJO3STREEC2IJS0_ESt9true_typeEEDpOT_
>      and it aliases symbol: _ZNSt5tupleIJO3STREEC1IJS0_ESt9true_typeEEDpOT_
>diff --git a/tests/data/test-diff-suppr/test39-opaque-type-report-0.txt b/tests/data/test-diff-suppr/test39-opaque-type-report-0.txt
>index 7993fc08..b87ba2ae 100644
>--- a/tests/data/test-diff-suppr/test39-opaque-type-report-0.txt
>+++ b/tests/data/test-diff-suppr/test39-opaque-type-report-0.txt
>@@ -8,4 +8,3 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>       in pointed to type 'typedef StructType' at test39-header-v1.h:2:1:
>         typedef name changed from StructType to AnotherStructType at test39-header-v1.h:2:1
>
>-
>-- 
>2.26.0.rc2.310.g2932bb562d-goog
>