From: "gprocida at google dot com" <sourceware-bugzilla@sourceware.org>
To: libabigail@sourceware.org
Subject: [Bug default/28319] abidw - regression in treatment of anonymous enums in structs
Date: Tue, 01 Mar 2022 14:56:55 +0000 [thread overview]
Message-ID: <bug-28319-9487-e2AVUg9wD4@http.sourceware.org/bugzilla/> (raw)
In-Reply-To: <bug-28319-9487@http.sourceware.org/bugzilla/>
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.
next prev parent reply other threads:[~2022-03-01 14:56 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-08 14:28 [Bug default/28319] New: " gprocida at google dot com
2021-09-08 14:45 ` [Bug default/28319] " gprocida at google dot com
2021-09-08 15:23 ` dodji at redhat dot com
2021-09-08 16:00 ` woodard at redhat dot com
2021-09-21 15:27 ` dodji at redhat dot com
2021-10-08 10:53 ` gprocida at google dot com
2022-01-11 11:30 ` gprocida at google dot com
2022-01-11 12:36 ` gprocida at google dot com
2022-01-11 17:01 ` dodji at redhat dot com
2022-01-11 18:16 ` gprocida at google dot com
2022-01-12 11:25 ` gprocida at google dot com
2022-01-12 11:59 ` gprocida at google dot com
2022-01-13 11:01 ` gprocida at google dot com
2022-01-13 13:35 ` dodji at redhat dot com
2022-01-13 14:26 ` gprocida at google dot com
2022-01-14 12:38 ` dodji at redhat dot com
2022-01-24 17:06 ` gprocida at google dot com
2022-03-01 11:22 ` Dodji Seketeli
2022-03-01 11:22 ` dodji at seketeli dot org
2022-03-01 14:56 ` gprocida at google dot com [this message]
2022-03-01 14:59 ` gprocida at google dot com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bug-28319-9487-e2AVUg9wD4@http.sourceware.org/bugzilla/ \
--to=sourceware-bugzilla@sourceware.org \
--cc=libabigail@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).