[PATCH] D110485: Support [[no_unique_address]] for all targets.

Wed Sep 29 04:28:05 PDT 2021

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

In D110485#3026629 <https://reviews.llvm.org/D110485#3026629>, @rjmccall wrote:

> MSVC gets to chose the ABI rules for their platform.  It is not Clang policy to pick ABI rules and then break them later.
>
> If you'd like to contribute an implementation of `[[msvc::no_unique_address]]` that matches MSVC's ABI,  that would be welcome.  I don't know, maybe that's as simple as making it use the existing implementation; @zygoloid might know better.  But "add an ABI feature ahead of the platform owner and then fix the ABI problems later" is not how we do things.

I concur with John.

In D110485#3028553 <https://reviews.llvm.org/D110485#3028553>, @expnkx wrote:

> I cannot find out where to add attribute that starts with msvc::xxxxx.

To Attr.td; you'd give a new spelling, like: `CXX11<"msvc", "no_unique_address">`

> I am just applying this "potential" patch to the clang upstream. In the future we can just remove TargetItaniumCXXABI attr in the clang table gen file without worrying about too much. So it is a "noop".
>
> So this patch changes nothing but it makes us be convenient to fix it in the future.

This patch is changing behavior and cannot be landed as-is. I don't think we should do anything in this area unless it's an implementation of `[[msvc::no_unique_address]]` that is ABI compatible with the Microsoft implementation.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110485/new/

https://reviews.llvm.org/D110485