[1/2] Ensure change_kind enum values are used consistently.
Commit Message
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
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,
@@ -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;
}
@@ -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)
@@ -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);
}