[clang] [Clang] Add __ugly__ spelling for the msvc attribute scope (PR #113765)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 30 06:22:52 PDT 2024


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


More information about the cfe-commits mailing list