[clang] Add GNU spelling for no_unqiue_address attribute

[philnik]

Summary

libc++ wants to switch from __compressed_pair to using no_unique_address. That is currently not possible, because there is no way to get the behaviour of no_unique_address in C++03. Adding the __attribute__ spelling allows libc++ to switch.

[aaron.ballman]

I'm not opposed, but this does muddy the waters about the target-specific nature of the attribute. Currently, the semantic attribute cannot be used outside of Itanium ABI targets, so it doesn't work on Windows (and we don't want it to -- Microsoft hasn't picked their ABI for the standard attribute and we want to avoid ABI breaks). But should we pick an ABI for the GNU attribute on Windows and just assume that Microsoft gets no say in the ABI for that because it's not their vendor extension? Or, if we disallow the GNU spelling on Windows because we want it to match the ABI of the standard attribute, does it actually help libc++ to expose the GNU spelling?

Also, the change is missing test coverage and a release note.

[philnik]

In D122874#3422263, @aaron.ballman wrote:

I'm not opposed, but this does muddy the waters about the target-specific nature of the attribute. Currently, the semantic attribute cannot be used outside of Itanium ABI targets, so it doesn't work on Windows (and we don't want it to -- Microsoft hasn't picked their ABI for the standard attribute and we want to avoid ABI breaks). But should we pick an ABI for the GNU attribute on Windows and just assume that Microsoft gets no say in the ABI for that because it's not their vendor extension? Or, if we disallow the GNU spelling on Windows because we want it to match the ABI of the standard attribute, does it actually help libc++ to expose the GNU spelling?

Also, the change is missing test coverage and a release note.

The best option for libc++ would probably be having the same ABI for __atrribute__((no_unique_address)) and [[msvc::no_unique_address]] on Windows. We already have code that uses [[no_unique_address]]/[[msvc::no_unique_address]] but clang-cl currently supports none of them, so there will be an ABI break if we declare these things stable before clang-cl has implemented one of these (although we don't really care about ABI stability on Windows AFAICT).

[aaron.ballman]

In D122874#3422489, @philnik wrote:

In D122874#3422263, @aaron.ballman wrote:

I'm not opposed, but this does muddy the waters about the target-specific nature of the attribute. Currently, the semantic attribute cannot be used outside of Itanium ABI targets, so it doesn't work on Windows (and we don't want it to -- Microsoft hasn't picked their ABI for the standard attribute and we want to avoid ABI breaks). But should we pick an ABI for the GNU attribute on Windows and just assume that Microsoft gets no say in the ABI for that because it's not their vendor extension? Or, if we disallow the GNU spelling on Windows because we want it to match the ABI of the standard attribute, does it actually help libc++ to expose the GNU spelling?

Also, the change is missing test coverage and a release note.

The best option for libc++ would probably be having the same ABI for __atrribute__((no_unique_address)) and [[msvc::no_unique_address]] on Windows.

And if [[msvc::no_unique_address]] and [[no_unique_address]] (when eventually implemented) in MSVC have a different ABI impact, will we will leave __attribute__((no_unique_address)) alone so it won't break ABI?

We already have code that uses [[no_unique_address]]/[[msvc::no_unique_address]] but clang-cl currently supports none of them, so there will be an ABI break if we declare these things stable before clang-cl has implemented one of these (although we don't really care about ABI stability on Windows AFAICT).

I believe we care about ABI stability, even on Windows. At the very least, I've never heard that the community does not care about ABI stability on Windows.

[philnik]

And if [[msvc::no_unique_address]] and [[no_unique_address]] (when eventually implemented) in MSVC have a different ABI impact, will we will leave __attribute__((no_unique_address)) alone so it won't break ABI?

I think that would depend on how Microsoft breaks the ABI. If there would be different mangling it would probably be best to also break the ABI of __attribute__((no_unique_address)), and I think that would be the most likely scenario.

I believe we care about ABI stability, even on Windows. At the very least, I've never heard that the community does not care about ABI stability on Windows.

Sorry, with 'we' I meant libc++. Because libc++ is never the primary c++ standard library on Windows most programs that use it ship their own version, so ABI stability isn't really an issue there. I think there is also a mingw environment with libc++, but IIRC there ABI stability was also more of a nice-to-have than an absolute need-to-have.

[aaron.ballman]

In D122874#3430672, @philnik wrote:

And if [[msvc::no_unique_address]] and [[no_unique_address]] (when eventually implemented) in MSVC have a different ABI impact, will we will leave __attribute__((no_unique_address)) alone so it won't break ABI?

I think that would depend on how Microsoft breaks the ABI. If there would be different mangling it would probably be best to also break the ABI of __attribute__((no_unique_address)), and I think that would be the most likely scenario.

Hmmm, I'd be more comfortable if this wasn't an ABI break for people using the __attribute__(()) spelling. If we wanted to surprise users with ABI breaks, we'd just implement [[no_unique_address]] on Windows and break users when Microsoft eventually supports the attribute, which is an approach I'm rather strongly opposed to. In this case, it's a bit different because it's a GNU-style spelling and Microsoft doesn't support that spelling at all, so my thinking was that we could define the ABI for it and make it stable because it's our extension. But I can see why you'd want something different in libc++.

So here's a potential path forward (thinking out loud, so this is not fully thought through and may be a bad idea): pick whatever ABI makes sense for __attribute__((no_unique_address)), document that the ABI is currently unstable for this spelling, and if we need to break the ABI then libc++ can use ABI tags on the structure to keep things compatible? And once we get the ABI-stable version, we can update the docs accordingly.

I believe we care about ABI stability, even on Windows. At the very least, I've never heard that the community does not care about ABI stability on Windows.

Sorry, with 'we' I meant libc++. Because libc++ is never the primary c++ standard library on Windows most programs that use it ship their own version, so ABI stability isn't really an issue there. I think there is also a mingw environment with libc++, but IIRC there ABI stability was also more of a nice-to-have than an absolute need-to-have.

Ah, okay, that makes more sense to me. However, this attribute isn't exposed just for libc++ folks; anyone will be able to use it for whatever they want, so my ABI concerns are for the wider audience.

[RKSimon]

Sorry but this is not an area I know enough about to review

[philnik]

Hmmm, I'd be more comfortable if this wasn't an ABI break for people using the __attribute__(()) spelling. If we wanted to surprise users with ABI breaks, we'd just implement [[no_unique_address]] on Windows and break users when Microsoft eventually supports the attribute, which is an approach I'm rather strongly opposed to. In this case, it's a bit different because it's a GNU-style spelling and Microsoft doesn't support that spelling at all, so my thinking was that we could define the ABI for it and make it stable because it's our extension. But I can see why you'd want something different in libc++.

I'm also OK with __attribute__((no_unique_address)) being stable. We have all these fun things behind a macro anyways currently, so nothing would prevent us from having __attribute__((no_unique_address)) in the stable ABI and use [[no_unique_address]] (whenever it will be implemented) in the unstable ABI. Or just remove the macro stuff if it will be the same ABI.

[ldionne]

I actually think that [[no_unique_address]] and __attribute__((no_unique_address)) should be equivalent. Similarly, we should be able to do [[msvc::no_unique_address]] and __attribute__((msvc::no_unique_address)), and both should be equivalent. Which one we pick for use in libc++ is a different issue, and we do it through a macro anyway so that we could easily change it in the future if we wanted.

[aaron.ballman]

In D122874#3442887, @ldionne wrote:

I actually think that [[no_unique_address]] and __attribute__((no_unique_address)) should be equivalent. Similarly, we should be able to do [[msvc::no_unique_address]] and __attribute__((msvc::no_unique_address)), and both should be equivalent.

I don't see how that works if [[no_unique_address]] and [[msvc_no_unique_address]] have different ABIs in practice, and we don't know what's going to happen there.

I agree with you in theory because I keep hoping that Microsoft will add support for [[no_unique_address]] and I really hope it's identical to [[msvc::no_unique_address]] when they do. However, Microsoft has not defined the ABI for [[no_unique_address]] because they don't support it (so neither do we; we don't know HOW to be compatible without a potential ABI break). We discussed supporting [[msvc::no_unique_address]] as part of https://github.com/llvm/llvm-project/issues/49358, and I'd be fine supporting that, but someone has to do the actual legwork to figure out what Microsoft's ABI is for it; thus far, nobody has been willing to do that work and Microsoft has gone silent on the thread once I started asking for details.

Long-term, I think users are going to expect __attribute__((no_unique_address)) to have the same behavior and ABI as [[no_unique_address]] instead of [[msvc::no_unique_address]] if the two attributes end up having an ABI difference.

Which one we pick for use in libc++ is a different issue, and we do it through a macro anyway so that we could easily change it in the future if we wanted.

+1

[philnik]

This is useless until we have some no_unique_address in clang-cl.

[philnik]

I'm dropping this in favour of D151683.

clang/include/clang/Basic/Attr.td

 }
 
 def ArmMveStrictPolymorphism : TypeAttr, TargetSpecificAttr<TargetARM> {
   let Spellings = [Clang<"__clang_arm_mve_strict_polymorphism">];
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
 def NoUniqueAddress : InheritableAttr, TargetSpecificAttr<TargetItaniumCXXABI> {
-  let Spellings = [CXX11<"", "no_unique_address", 201803>];
+  let Spellings = [GNU<"no_unique_address">, CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;
   let Documentation = [NoUniqueAddressDocs];
   let SimpleHandler = 1;
 }
 
 def ReturnsTwice : InheritableAttr {
   let Spellings = [GCC<"returns_twice">];
   let Subjects = SubjectList<[Function]>;