[libc++] `no_unique_address` is reserved word in pre-C++20 dialects (non-conforming)

[JoeLoser]

As we can see on this Godbolt link, even when compiling in earlier dialects than C++20 (such as C++17), we get a compiler error since no_unique_address is effectively a reserved word even in pre-C++20 dialects.

Some context: Clang supports [[no_unique_address] in all dialects for C++11 and later (I don't know why it doesn't support it in C++03 mode, even as __attribute__((no_unique_address))).

Failing to support a double-underscored no_unique_address means that no_unique_address encroaches into the user's namespace in C++11 through C++17 dialects which is not good.

One resolution would be if Clang could start supporting a __dundered form of no_unique_address that would be safely usable by the implementor, e.g. __attribute__((__no_unique_address__)).

[shafik]

As per documentation: https://clang.llvm.org/docs/AttributeReference.html#no-unique-address this is supported in C++11 forward. I am looking at the PR: https://reviews.llvm.org/D63451 that implemented it and I don't see a rationale for supporting it as a extension in C++11.

CC @zygoloid who implemented and can perhaps talk to why it is supported pre C++20 and attributes code owner @erichkeane

[zygoloid]

I'm not sure I understand the question. It's clearly useful to support this in older versions, and it's conforming to do so. Indeed, the design of C++ attributes is intended precisely to permit compilers supporting attributes in older versions.

There's definitely a libc++ bug here -- it's not conforming for it to use __has_cpp_attribute(msvc::no_unique_address) like this because neither msvc nor no_unique_address are on the list of identifiers that a program shall not define before including a library header.

Now, Clang does support __no_unique_address__ as a synonym for no_unique_address, both in the attribute and in the __has_cpp_attribute check, but using that doesn't help, because this is a check for an MSVC feature, so what matters is what MSVC supports. And it looks like MSVC doesn't support a double-underscore version, so libc++ can't use that.

Presumably the proper fix for libc++ would be to guard this check with something like

#if !defined(msvc) && !defined(no_unique_address)

... but I suppose there's still the possibility that the user will #define no_unique_address or msvc between including the config header and including some other library header, so maybe even that doesn't work?

In any case, I don't see any Clang issue here. Retargeting as a libc++ bug.

[shafik]

@zygoloid maybe I am reading it wrong but it seems like macro.names would reserve the attribute-tokens and by supporting it as a extension we are encroaching on the user namespace early then folks would expect.

[philnik777]

@zygoloid maybe I am reading it wrong but it seems like macro.names would reserve the attribute-tokens and by supporting it as a extension we are encroaching on the user namespace early then folks would expect.

The attribute-token s are the standard attributes, i.e. you can't #define no_unique_address in C++20-and-later, but you are allowed to do that in C++17. This essentially just means that the library is allowed to use the non-_Uglified versions of the attributes when they are in the standard. So you are free to do fun things like this, and as you see the compiler is happy to do that.

I think we should just remove the [[msvc::no_unique_address]] for now, since it's not supported by clang or GCC currently anyways.

[shafik]

I chatted w/ @zygoloid offline and we are saying the same thing, I was just missing some context. So I agree with @zygoloid original comment.

[frederick-vs-ja]

There's definitely a libc++ bug here -- it's not conforming for it to use __has_cpp_attribute(msvc::no_unique_address) like this because neither msvc nor no_unique_address are on the list of identifiers that a program shall not define before including a library header.

MSVC STL resolved this by preprocessor extensions (microsoft/STL#2649). Is the way suitable for libc++?

[philnik777]

There's definitely a libc++ bug here -- it's not conforming for it to use __has_cpp_attribute(msvc::no_unique_address) like this because neither msvc nor no_unique_address are on the list of identifiers that a program shall not define before including a library header.

MSVC STL resolved this by preprocessor extensions (microsoft/STL#2649). Is the way suitable for libc++?

We don't support the Microsoft compiler anyways, so we can just ask the clang folks to add an __msvc__ spelling if [[msvc::no_unique_address]] ever gets implemented.

[zygoloid]

There's definitely a libc++ bug here -- it's not conforming for it to use __has_cpp_attribute(msvc::no_unique_address) like this because neither msvc nor no_unique_address are on the list of identifiers that a program shall not define before including a library header.

MSVC STL resolved this by preprocessor extensions (microsoft/STL#2649). Is the way suitable for libc++?

MSVC's approach is still broken: https://godbolt.org/z/cTYor9Ta6

It's also unsuitable for [[msvc::no_unique_address]] (unless the fallback path gets the same class layout a different way), because falling back on not using the attribute could result in an ABI difference between different TUs in the same program.