Unnamed bit field is being compared in defaulted equality operator

[foonathan]

#include <cassert>
#include <cstring>

struct foo
{
    int member : 8;
    int : 24;

    bool operator==(const foo&) const = default;
};

int main()
{
    // Create two objects with different object representations.
    foo a, b;
    std::memset(&a, 0xFF, sizeof(foo));
    std::memset(&b, 0x99, sizeof(foo));

    // Make all members identical to give them the same value representation.
    a.member = 0;
    b.member = 0;
    // This does not pass?!
    assert(a == b);
}

Demo: https://godbolt.org/z/75ee78Gss

Unnamed bit fields aren't members, so they shouldn't be compared.

[llvmbot]

@llvm/issue-subscribers-clang-codegen

[shafik]

This does look like a bug, let me think a bit more before confirming.

[shafik]

I agree this is a bug, we should not be including this in the comparison. I was wondering if because class.bit p2 says:

... Unnamed bit-fields are not members and cannot be initialized ...

That perhaps clang is assuming some sort of zero init and therefore since it could not change then it would be fair to include them in the comparison but looking at the code I don't see this.

My first take on fixing this would be to modify DefaultedComparisonVisitor member function visitSubobjects to include this check while iterating over the fields:

if (Field->isUnnamedBitfield())
        continue;

My initial test seems to indicate this does the job but I may be missing something. CC @zygoloid @AaronBallman

[AaronBallman]

Based on my reading of:

https://eel.is/c++draft/class.compare.default#5.sentence-1
https://eel.is/c++draft/class.bit#2.sentence-2

I believe you're correct on the approach for the fix, as well.

[shafik]

We have another bug that looks like it is fixed by my proposed fix for this, so I am going to run check-clang for this and if it passes I will put a PR together since this looks like a serious bug.

[shafik]

PR: https://reviews.llvm.org/D146329

[llvmbot]

@llvm/issue-subscribers-clang-frontend