Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Missing [[no_unique_address]] support on Windows #48983

Open Quuxplusone opened 3 years ago

Quuxplusone commented 3 years ago
Bugzilla Link PR50014
Status NEW
Importance P enhancement
Reported by Rob Klein Ikink (rob.klein.ikink@student.nhlstenden.com)
Reported on 2021-04-17 11:15:09 -0700
Last modified on 2021-11-18 16:44:00 -0800
Version trunk
Hardware PC Windows NT
CC aaron@aaronballman.com, blitzrakete@gmail.com, Casey@Carter.net, erik.pilkington@gmail.com, heavenandhell171@gmail.com, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments attachments.zip (683 bytes, application/x-zip-compressed)
Blocks
Blocked by
See also
Created attachment 24764
test.cpp: the source file that triggers the warning. log.txt: console output
from compiling test.cpp.

https://clang.llvm.org/cxx_status.html#cxx20 lists the [[no_unique_address]]
attribute as supported since Clang 9. However, the attribute does not seem to
currently be supported on Windows platforms.

Compiling the following source code on Windows with Clang 12.0.0 with -
std=c++20:

struct X {};

struct Y1 {
    [[no_unique_address]] X x;
    long long a;
};

struct Y2 {
    X x;
    long long a;
};

int main() {}

gives the following warning:

test.cpp:4:4: warning: unknown attribute 'no_unique_address' ignored [-Wunknown-
attributes]
        [[no_unique_address]] X x;
          ^~~~~~~~~~~~~~~~~
1 warning generated.

The size of objects from Y1 are also the same size as those from Y2, indicating
that an unique address is indeed still generated:

std::cout << sizeof(Y1) << ", " << sizeof(Y2);

prints "16, 16" compared to "8, 16" on Linux: https://godbolt.org/z/nfaf3KeP3
Quuxplusone commented 3 years ago

Attached attachments.zip (683 bytes, application/x-zip-compressed): test.cpp: the source file that triggers the warning. log.txt: console output from compiling test.cpp.

Quuxplusone commented 3 years ago

_Bug 51051 has been marked as a duplicate of this bug._

Quuxplusone commented 3 years ago

This is not a bug. Microsoft has not defined the ABI for this attribute on their platform, so Clang needs to wait for Microsoft to define that before we can support [[no_unique_address]] there (otherwise we run into potential ABI incompatibilities).

However, I'm leaving this request open because presumably Microsoft will define the ABI on their platform for this feature, and then Clang can follow suit.

Quuxplusone commented 3 years ago
(In reply to Aaron Ballman from comment #2)
> This is not a bug. Microsoft has not defined the ABI for this attribute on
> their platform, so Clang needs to wait for Microsoft to define that before
> we can support [[no_unique_address]] there (otherwise we run into potential
> ABI incompatibilities).
>
> However, I'm leaving this request open because presumably Microsoft will
> define the ABI on their platform for this feature, and then Clang can follow
> suit.

Microsoft already added support for this as of VS 2019 16.9.
(https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-160)
Quuxplusone commented 3 years ago
(In reply to Rob Klein Ikink from comment #3)
> (In reply to Aaron Ballman from comment #2)
> > This is not a bug. Microsoft has not defined the ABI for this attribute on
> > their platform, so Clang needs to wait for Microsoft to define that before
> > we can support [[no_unique_address]] there (otherwise we run into potential
> > ABI incompatibilities).
> >
> > However, I'm leaving this request open because presumably Microsoft will
> > define the ABI on their platform for this feature, and then Clang can follow
> > suit.
>
> Microsoft already added support for this as of VS 2019 16.9.
> (https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-
> conformance?view=msvc-160)

Alas, but I don't believe the documentation based on how their compiler behaves
in practice (which is not something Clang will be emulating because it looks to
be a buggy implementation). See https://godbolt.org/z/j1PzbK68P, but basically,
__has_cpp_attribute returns false for both no_unique_address and
msvc::no_unique_address, yet the latter actually does the correct thing that
the former is standardized to do while the former is silently ignored.
Quuxplusone commented 2 years ago

It is not intended that MSVC predefines __has_cpp_attribute(msvc::no_unique_address) to 0. (Copy-pasta strikes again!) I'll get it changed to 201803L. It is intentional that MSVC predefines __has_cpp_attribute(no_unique_address) to 0, however - we don't consider the no-op implementation to be worthy of claiming support for the attribute.

As an STL developer, I would greatly appreciate if clang-cl were to implement [[msvc::no_unique_address]]. I'm willing to bribe people with adult beverages =)