[Bug default/28319] abidw - regression in treatment of anonymous enums in structs

[gprocida at google dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28319

--- Comment #16 from gprocida at google dot com ---
Hello.

On Tue, 1 Mar 2022 at 11:22, dodji at seketeli dot org
<sourceware-bugzilla@sourceware.org> wrote:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28319
>
> --- Comment #15 from dodji at seketeli dot org ---
> gprocida at google dot com via Libabigail <libabigail@sourceware.org> a
> écrit:
>
> > Hi Dodji.
>
> Hey Giuliano,
>
> > git grep use_enum_binary_only_equality
>
> Now, from what I am seeing, that one is not used anywhere anymore.  The
> last use of it was in an obsolete comment.  I removed it in this commit
> https://sourceware.org/pipermail/libabigail/2022q1/004184.html.
>
> [...]
>
>
> > When I tested in comment 12, I looked for changes in abidw output and there was
> > none.
> >
> > There is a regression in abidiff though.
> >
> > The two enums in these small code fragments
> >
> > enum E6 { a6 = 6 } v6;
> >
> > enum E6 { a6 = 6, b6 = 6 } v6;
> >
> > are treated as ABI equivalent ...
>
> Yes, this is as intended.  In the function
> equals(const enum_type_decl& l, const enum_type_decl& r, change_kind* k)
> in abi-ir.cc, you can see the comment:
>
>   // Now compare the enumerators.  Note that the order of declaration
>   // of enumerators should not matter in the comparison.
>   //
>   // Also if an enumerator value is redundant, that shouldn't impact
>   // the comparison.
>   //
>   // In that case, note that the two enums below are considered equal:
>   //
>   // enum foo
>   //     {
>   //       e0 = 0;
>   //       e1 = 1;
>   //       e2 = 2;
>   //     };
>   //
>   //     enum foo
>   //     {
>   //       e0 = 0;
>   //       e1 = 1;
>   //       e2 = 2;
>   //       e_added = 1; // <-- this value is redundant with the value
>   //                    //     of the enumerator e1.
>   //     };
>   //
>   // These two enums are considered equal.
>
> > ... as they share the same set of enumerator values.
>
> It's rather because they have the same enumerator names /and/ values,
> modulo one /additional/ enumerator which value is redundant with the
> existing ones.  So in this newer way of seeing things, the previous
> concept of "binary-only equality" is not used anymore.
>

enum E6 { a6 = 6 } v6;
vs
enum E6 { a6 = 6, b6 = 6 } v6;

Well, the sets of enumerator values are:
{ 6 } vs { 6 }
and the sets of enumerator names are:
{ a6 } vs { a6, b6 }

So they don't have the same enumerator names.

As you point out, the addition of "b6" does not change the meaning of
any other enumerator. I think we understand each other.

It's just a question as to whether this should be considered an ABI
difference. Perhaps it should be a "harmless" difference, though I
would not want to add more complexity to code that is not regularly
exercised in production.

I also discovered recently the concept of C (and this does not exist
in C++) type compatibility.

https://en.cppreference.com/w/c/language/type#Compatible_types
https://www.ibm.com/docs/en/zos/2.1.0?topic=types-compatibility-structures-unions-enumerations-c-only

These documents could be more clear, but I think they imply that these
two are compatible.

enum E { b = 2, a = 1 };
enum E { a = 1, b = 2 };

On first reading, I thought cppreference also implied that these two
are compatible (which would be relevant here), but I don't think they
are:

enum E { a = 1, b = 2 };
enum E { a = 1 };

> [...]
>
> > 2. Just for the record, if you think this is working as intended, please
> > re-close the bug.
>
> Yes, I think it's working as intended.  I'll wait for you to double
> check if you see an obvious issue.  If not, I'll let you close the bug.
> Would that work for you?
>

Sure.

> Thanks.
>
> --
> You are receiving this mail because:
> You reported the bug.

-- 
You are receiving this mail because:
You are on the CC list for the bug.