[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address
Gabor Bencze via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 19 08:24:43 PDT 2020
gbencze added inline comments.
================
Comment at: clang/lib/AST/ASTContext.cpp:2580-2581
if (!isStructEmpty(Base.getType())) {
llvm::Optional<int64_t> Size = structHasUniqueObjectRepresentations(
Context, Base.getType()->castAs<RecordType>()->getDecl());
if (!Size)
----------------
rsmith wrote:
> 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), "");
> ```
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), "");
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89649/new/
https://reviews.llvm.org/D89649
More information about the cfe-commits
mailing list