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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D110485#3308878 <https://reviews.llvm.org/D110485#3308878>, @mstorsjo wrote:

> In D110485#3308854 <https://reviews.llvm.org/D110485#3308854>, @mstorsjo wrote:
>
>> To pick up the thread here again, `[[no_unique_address]]` is done and settled in MSVC, with the slightly surprising semantics: `[[no_unique_address]]` is accepted, without any warning (in C++20 mode), but it has no effect. (This, not related to LLVM, but because they had shipped it in earlier versions without having an effect, and changing that later would break things.) `[[msvc::no_unique_address]]` does have an effect though. See https://github.com/microsoft/STL/issues/1364#issuecomment-1034167093 for a more authoritative source on that.
>>
>> So, separately from implementing `[[msvc::no_unique_address]]`, I think we also should also silence the current warning about unknown attribute for the standard `[[no_unique_address]]`, to match MSVC.
>
> Oh, also, according to https://devblogs.microsoft.com/cppblog/msvc-cpp20-and-the-std-cpp20-switch/, the plan is to change `[[no_unique_address]]` to actually have an effect the next time the compiler breaks its C++ ABI at an unknown point in the future. (This shouldn't be an issue for Clang, as we'd have to make a conscious effort to implement the new ABI whenever that happens anyway.)

Thanks for the update! I'll have to think about the appropriate way forward here a bit more. We have a rule with attributes to issue an "attribute ignored" warning for any attribute that isn't accepted by Clang, and we typically extend that to attributes which are accepted by Clang but have no impact in a way the user may be surprised by (e.g., if they write it on the wrong construct, we don't silently eat the attribute, we warn the user that the attribute is being ignored). So my initial thinking is, have an on-by-default diagnostic (grouped separately under the ignored attributes warning flag) about a standard attribute being accepted and purposefully ignored that's only used for `[[no_unique_address]]` in MS compatibility mode (for now, we may get more such attributes in the future). Then users get a different warning than they do today, and they can control it separately from `-Wignored-attributes`. I think still having a diagnostic is useful for some class of users (but not all). People who only care about compatibility with MSVC likely want the warning off by default, while people who care about compatibility outside of MSVC likely want the warning on by default. Given the security implications of ignoring the attribute, I think erring on the side of caution and warning by default is appropriate (this is a standard attribute, not a vendor one, so users are understandably going to expect the attribute to do something if they've used it correctly according to the standard; silencing it is a minor burden that will not impact other kinds of ignored attributes). The fact that MS intends to support the attribute in the future (which breaks ABI) is all the more reason why I think we should warn by default; Clang users shouldn't be caught off guard when MSVC eventually breaks their ABI.


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

https://reviews.llvm.org/D110485