Fix __has_unique_object_representations with no_unique_address

[gbencze]

Summary

Fix incorrect behavior of __has_unique_object_representations when using the no_unique_address attribute.

Based on the bug report: https://bugs.llvm.org/show_bug.cgi?id=47722

[rsmith]

This looks fine as far as it goes, but it doesn't fix all cases of incorrect behavior of __has_unique_object_representations due to [[no_unique_address]]. Feel free to either to land this as-is and leave the other case to a separate patch, or fix it as a revision of this same change. I would expect we'll want to unify the code paths for base classes and non-static data members, which might be better done all at once.

clang/lib/AST/ASTContext.cpp

2628–2629

We need to do this for non-empty [[no_unique_address]] members of class type too, to handle tail padding reuse in cases such as:

struct A {
    ~A();
    int a;
    char b;
};
struct B {
    [[no_unique_address]] A a;
    char c[3];
};
static_assert(sizeof(B) == 8, "");
static_assert(__has_unique_object_representations(B), "");

[aaron.ballman]

clang/lib/AST/ASTContext.cpp

2638

You can drop the else after this sorta-return if you'd like.

[gbencze]

clang/lib/AST/ASTContext.cpp

2628–2629

You're right, I missed the case of reusing tail padding. I'll try to update this review to include this soon.

But I don't think that this exact example is incorrect now as A is not trivially copyable due to the user defined destructor.
It should return true when the class type member is trivially copyable but non-standard-layout though:

struct A {
private:
    int a;
public:
    char b;
};
struct B {
    [[no_unique_address]] A a;
    char c[3];
};
static_assert(sizeof(B) == 8, "");
static_assert(__has_unique_object_representations(B), "");

[gbencze]

Sorry for the slow update on this.
I fixed the behavior when reusing tail padding as mentioned by @rsmith and took a shot at unifying the code paths for base classes and fields. Let me know your thoughts on the approach!

[gbencze]

clang/test/SemaCXX/has_unique_object_reps_no_unique_addr.cpp

1

Just to be sure: is the specifying the triple here enough to ensure that this always uses the Itanium ABI? I believe MSVC currently ignores the no_unique_address attribute. Or do I need to add some more flags?
Alternatively, the static_asserts could be modified to check sizeof(T) > [expected size with Itanium ABI] || __has_unique_object_representations(T)

[gbencze]

Pinging @rsmith - could you please check the updates to this patch? Thanks!

[gbencze]

Ping @rsmith