diff mbox series

[1/2] Ensure change_kind enum values are used consistently.

Message ID 20200318142926.6692-1-gprocida@google.com
State Committed
Headers
Series [1/2] Ensure change_kind enum values are used consistently. |

Commit Message

Giuliano Procida March 18, 2020, 2:29 p.m. UTC 
  The change_kind enumeration (bitset) values are used to track what
kinds of a changes are present at a diff node.

Local type and non-type changes may be present. These are tracked
using 3 bits, with the invariant that LOCAL_TYPE_CHANGE_KIND or
LOCAL_NON_TYPE_CHANGE_KIND both imply LOCAL_CHANGE_KIND. This
invariant has to be maintained in code which doesn't always happen.

This patch fixes a couple of cases where LOCAL_CHANGE_KIND or
LOCAL_TYPE_CHANGE_KIND was missing and changes a few other bits of
code so that it is clear that two bits are being paired together.

	* src/abg-comparison.cc (distinct_diff::has_local_changes):
	Remove unnecessary parentheses around return expression.
	* src/abg-default-reporter.cc (report): In the reference_diff
	overload, replace test against LOCAL_CHANGE_KIND with test
	against ALL_LOCAL_CHANGES_MASK.
	* src/abg-ir.cc (equals): In the array_type_def and class_decl
	overloads, add missing LOCAL_TYPE_CHANGE_KIND. In the
	class_decl overload, also add missing LOCAL_CHANGE_KIND. In
	the enum_type_decl and function_decl::parameter overloads
	clarify pairing of LOCAL*CHANGE_KIND bits.
	(enum_has_non_name_change): Clarify pairing of
	LOCAL*CHANGE_KIND bits.

A follow-up patch will drop LOCAL_CHANGE_KIND as it is now completely
redundant.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-comparison.cc       |  2 +-
 src/abg-default-reporter.cc |  2 +-
 src/abg-ir.cc               | 21 +++++++++------------
 3 files changed, 11 insertions(+), 14 deletions(-)

Comments

Dodji Seketeli March 19, 2020, 10:50 a.m. UTC | #1 
Giuliano Procida <gprocida@google.com> a ?crit:

> The change_kind enumeration (bitset) values are used to track what
> kinds of a changes are present at a diff node.
>
> Local type and non-type changes may be present. These are tracked
> using 3 bits, with the invariant that LOCAL_TYPE_CHANGE_KIND or
> LOCAL_NON_TYPE_CHANGE_KIND both imply LOCAL_CHANGE_KIND. This
> invariant has to be maintained in code which doesn't always happen.
>
> This patch fixes a couple of cases where LOCAL_CHANGE_KIND or
> LOCAL_TYPE_CHANGE_KIND was missing and changes a few other bits of
> code so that it is clear that two bits are being paired together.
>
> 	* src/abg-comparison.cc (distinct_diff::has_local_changes):
> 	Remove unnecessary parentheses around return expression.
> 	* src/abg-default-reporter.cc (report): In the reference_diff
> 	overload, replace test against LOCAL_CHANGE_KIND with test
> 	against ALL_LOCAL_CHANGES_MASK.
> 	* src/abg-ir.cc (equals): In the array_type_def and class_decl
> 	overloads, add missing LOCAL_TYPE_CHANGE_KIND. In the
> 	class_decl overload, also add missing LOCAL_CHANGE_KIND. In
> 	the enum_type_decl and function_decl::parameter overloads
> 	clarify pairing of LOCAL*CHANGE_KIND bits.
> 	(enum_has_non_name_change): Clarify pairing of
> 	LOCAL*CHANGE_KIND bits.

I am applying this to master with just one little nit change: I made the
ChangeLog part of the commit log be the last.

Thanks!

Cheers,
diff mbox series

Patch

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 7b55ee62..96a4a484 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -2713,7 +2713,7 @@  distinct_diff::has_local_changes() const
 {
   // Changes on a distinct_diff are all local.
   if (has_changes())
-    return (LOCAL_CHANGE_KIND | LOCAL_TYPE_CHANGE_KIND);
+    return LOCAL_CHANGE_KIND | LOCAL_TYPE_CHANGE_KIND;
   return NO_CHANGE_KIND;
 }
 
diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index e6aba3e5..a5364a56 100644
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -444,7 +444,7 @@  default_reporter::report(const reference_diff& d, ostream& out,
   enum change_kind k = ir::NO_CHANGE_KIND;
   equals(*d.first_reference(), *d.second_reference(), &k);
 
-  if ((k & LOCAL_CHANGE_KIND) && !(k & SUBTYPE_CHANGE_KIND))
+  if ((k & ALL_LOCAL_CHANGES_MASK) && !(k & SUBTYPE_CHANGE_KIND))
     report_local_reference_type_changes(d, out, indent);
 
   if (k & SUBTYPE_CHANGE_KIND)
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 2cb76ffc..76f57396 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -14179,7 +14179,7 @@  equals(const array_type_def::subrange_type& l,
 	{
 	  if (!types_have_similar_structure(l.get_underlying_type().get(),
 					    r.get_underlying_type().get()))
-	    *k |= LOCAL_CHANGE_KIND;
+	    *k |= LOCAL_CHANGE_KIND | LOCAL_TYPE_CHANGE_KIND;
 	  else
 	    *k |= SUBTYPE_CHANGE_KIND;
 	}
@@ -14906,10 +14906,9 @@  enum_has_non_name_change(const enum_type_decl& l,
       if (k)
 	{
 	  if (!l.decl_base::operator==(r))
-	    *k |= LOCAL_NON_TYPE_CHANGE_KIND;
+	    *k |= LOCAL_CHANGE_KIND | LOCAL_NON_TYPE_CHANGE_KIND;
 	  if (!l.type_base::operator==(r))
-	    *k |=  LOCAL_TYPE_CHANGE_KIND;
-	  *k |= LOCAL_CHANGE_KIND;
+	    *k |= LOCAL_CHANGE_KIND | LOCAL_TYPE_CHANGE_KIND;
 	}
       else
 	{
@@ -14987,10 +14986,9 @@  equals(const enum_type_decl& l, const enum_type_decl& r, change_kind* k)
       if (k)
 	{
 	  if (!l.decl_base::operator==(r))
-	    *k |= LOCAL_NON_TYPE_CHANGE_KIND;
+	    *k |= LOCAL_CHANGE_KIND | LOCAL_NON_TYPE_CHANGE_KIND;
 	  if (!l.type_base::operator==(r))
-	    *k |= LOCAL_TYPE_CHANGE_KIND;
-	  *k |= LOCAL_CHANGE_KIND;
+	    *k |= LOCAL_CHANGE_KIND | LOCAL_TYPE_CHANGE_KIND;
 	}
       else
 	return false;
@@ -17558,11 +17556,10 @@  equals(const function_decl::parameter& l,
       if (k)
 	{
 	  if (l.get_index() != r.get_index())
-	    *k |= LOCAL_NON_TYPE_CHANGE_KIND;
+	    *k |= LOCAL_CHANGE_KIND | LOCAL_NON_TYPE_CHANGE_KIND;
 	  if (l.get_variadic_marker() != r.get_variadic_marker()
 	      || !!l.get_type() != !!r.get_type())
-	    *k |= LOCAL_TYPE_CHANGE_KIND;
-	  *k |= LOCAL_CHANGE_KIND;
+	    *k |= LOCAL_CHANGE_KIND | LOCAL_TYPE_CHANGE_KIND;
 	}
       else
 	return false;
@@ -20337,7 +20334,7 @@  equals(const class_decl& l, const class_decl& r, change_kind* k)
 	    {
 	      if (!types_have_similar_structure((*b0)->get_base_class().get(),
 						(*b1)->get_base_class().get()))
-		*k |= LOCAL_CHANGE_KIND;
+		*k |= LOCAL_CHANGE_KIND | LOCAL_TYPE_CHANGE_KIND;
 	      else
 		*k |= SUBTYPE_CHANGE_KIND;
 	      break;
@@ -20398,7 +20395,7 @@  equals(const class_decl& l, const class_decl& r, change_kind* k)
 	  {
 	    result = false;
 	    if (k)
-	      *k |= LOCAL_NON_TYPE_CHANGE_KIND | LOCAL_NON_TYPE_CHANGE_KIND;
+	      *k |= LOCAL_CHANGE_KIND | LOCAL_NON_TYPE_CHANGE_KIND;
 	    RETURN(result);
 	  }