AaronBallman wrote:

> > > > > The MSVC FE team hasn't expressed enthusiasm for adding ugly 
> > > > > spellings. If I learn more I'll relay that info.
> > > > 
> > > > 
> > > > Thank you for checking! Unfortunately, I think that's a reason for 
> > > > Clang to not support it for the `msvc` vendor prefix either.
> > > 
> > > 
> > > What is the alternative?
> > 
> > 
> > Can you get away with not using `[[msvc::no_unique_address]]`? If so, I'd 
> > go that route.
> > No. We need it to have a reasonable ABI.
> > If not, I'd say use the attribute with the non-ugly spelling and woe unto 
> > anyone defining `msvc` as a macro despite that being a perfectly valid 
> > thing for them to do. You could be "kind" and do
> > ```
> > #ifdef msvc
> > #error "Microsoft's vendor specific attribute prefix steals this identifier 
> > from the user's namespace, please file an issue with Microsoft if you see 
> > this diagnostic"
> > #endif
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > or something along those lines.
> 
> This steals both `msvc` and `no_unique_address` and does that on all 
> platforms, not just MSVC, so that's not exactly a thrilling solution. 

You can put guards in to only steal `msvc` on Microsoft's platform; there's no 
need to steal it everywhere. And `no_unique_address`... I was thinking that we 
already supported that with the underscores but it seems we don't: 
https://godbolt.org/z/fn5v19hx7 so that's unfortunate.

> Would defining a namespace like `__clang_msvc__` be an option? 

It's an option, but not one I'm thrilled with. The point to vendor namespaces 
is that the vendor picks them and other vendors can be compatible. Do we want 
EDG to have to support `__clang_msvc__` as a prefix for Clang's compatibility 
with Microsoft? That seems... a bit silly when Microsoft could elect to provide 
a spelling that doesn't steal from the user's namespace.

> libc++ only cares about Clang on MSVC targets, so it's pretty much irrelevant 
> what MSVC does for us. Adding an alias like 
> `[[_Clang::__no_unique_address__]]` would also work if that's more palatable. 
> That'd have to be added to any msvc attributes libc++ cares about of course 
> (though currently that's only `[[msvc::no_unique_address]]`).

That's somewhat more palatable to me, but I'm still wondering whether we're 
making a mountain out of a mole hill with the vendor prefix:
https://sourcegraph.com/search?q=context:global+%5E%23define%5Cs%2Bmsvc%5Cb&patternType=regexp&case=yes&sm=0

That said, the attribute name itself does seem to be an issue:
https://sourcegraph.com/search?q=context:global+%5E%23define%5Cs%2Bno_unique_address%5Cb&patternType=regexp&case=yes&sm=0

So yeah, I think I'd be happier exposing `[[_Clang::__no_unique_address__]]` as 
an alias to the standard attribute, but I am wondering if @StephanTLavavej can 
maybe push back on the MSVC team a little bit regarding support for attribute 
usage in system headers. Both Clang and GCC already provide such a mechanism 
for exactly that reason, and I have to imagine Microsoft would want something 
similar so they can protect their own system headers.

https://github.com/llvm/llvm-project/pull/113765
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to