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

Message ID 20200329170121.188147-1-gprocida@google.com
State Committed
Headers
Series [1/3] abidiff: Remove blank line after typedef changes. |

Commit Message

Giuliano Procida March 29, 2020, 5:01 p.m. UTC
  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>
---
 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(-)
  

Comments

Matthias Männich March 29, 2020, 5:30 p.m. UTC | #1
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
>
  
Dodji Seketeli April 1, 2020, 4:21 p.m. UTC | #2
Giuliano Procida <gprocida@google.com> a ?crit:

> 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.

Applied to master, thanks!!

Cheers,
  

Patch

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
 
-