[PATCH] D110485: Support [[no_unique_address]] for all targets.

2022-02-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D110485#3308878 , @mstorsjo wrote: > In D110485#3308854 , @mstorsjo > wrote: > >> To pick up the thread here again, `[[no_unique_address]]` is done and >> settled in MSVC, with

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2022-02-09 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D110485#3308854 , @mstorsjo wrote: > To pick up the thread here again, `[[no_unique_address]]` is done and settled > in MSVC, with the slightly surprising semantics: `[[no_unique_address]]` is > accepted, without any

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2022-02-09 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. To pick up the thread here again, `[[no_unique_address]]` is done and settled in MSVC, with the slightly surprising semantics: `[[no_unique_address]]` is accepted, without any warning (in C++20 mode), but it has no effect. (This, not related to LLVM, but because they

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D110485#3033889 , @expnkx wrote: > https://github.com/microsoft/STL/issues/1364 > > As Microsoft said, the reason why things like msvc::no_unique_address exist > are because clang does not support it. In fact, it has

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-30 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D110485#3033889 , @expnkx wrote: > In D110485#3030874 , @erichkeane > wrote: > >> In D110485#3030853 , @expnkx wrote: >> >>> clang simply

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-30 Thread cqwrteur via Phabricator via cfe-commits
expnkx added a comment. In D110485#3030874 , @erichkeane wrote: > In D110485#3030853 , @expnkx wrote: > >> clang simply just does not truly support everything msvc supports. There are >> no things that deals

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D110485#3030853 , @expnkx wrote: > clang simply just does not truly support everything msvc supports. There are > no things that deals [[msvc::]] attributes at all in the clang. In fact the > point of using clang on

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-29 Thread cqwrteur via Phabricator via cfe-commits
expnkx added a comment. clang simply just does not truly support everything msvc supports. There are no things that deals [[msvc::]] attributes at all in the clang. In fact the point of using clang on windows is that we do not want to do the same as Microsoft does. Support

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1690 +def NoUniqueAddress : InheritableAttr { let Spellings = [CXX11<"", "no_unique_address", 201803>]; let Subjects = SubjectList<[NonBitField], ErrorDiag>; The way to do a

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In D110485#3026629 , @rjmccall wrote: > MSVC gets to chose the ABI rules for their platform. It is not Clang policy > to

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-28 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375775. expnkx added a comment. clang format + clang tidy all clang tests pass locally. flang is bugged. Do not know why flang cannot find llvm-jxx CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110485/new/ https://reviews.llvm.org/D110485 Files:

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-28 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375758. expnkx added a comment. fixing clang format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110485/new/ https://reviews.llvm.org/D110485 Files: clang/lib/AST/RecordLayoutBuilder.cpp Index: clang/lib/AST/RecordLayoutBuilder.cpp

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-28 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375720. expnkx added a comment. it should first update the field and then test no_unique_address CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110485/new/ https://reviews.llvm.org/D110485 Files: clang/lib/AST/RecordLayoutBuilder.cpp Index:

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-28 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375690. expnkx added a comment. I cannot find out where to add attribute that starts with msvc::x. I am just applying this "potential" patch to the clang upstream. In the future we can just remove TargetItaniumCXXABI attr in the clang table gen file

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread John McCall via Phabricator via cfe-commits
rjmccall requested changes to this revision. rjmccall added a comment. This revision now requires changes to proceed. MSVC gets to chose the ABI rules for their platform. It is not Clang policy to pick ABI rules and then break them later. If you'd like to contribute an implementation of

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread Ivan Garramona via Phabricator via cfe-commits
ivan171 added a comment. In D110485#3026533 , @rjmccall wrote: > We should not add a standard feature with ABI impact if we aren't certain > we're going to match MSVC's ABI on it. I'm sorry, but this needs to wait > until there's an MSVC release that

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375468. expnkx added a comment. should be diff3 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110485/new/ https://reviews.llvm.org/D110485 Files: clang/include/clang/Basic/Attr.td clang/test/Preprocessor/has_attribute.cpp

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx added a comment. In D110485#3026533 , @rjmccall wrote: > We should not add a standard feature with ABI impact if we aren't certain > we're going to match MSVC's ABI on it. I'm sorry, but this needs to wait > until there's an MSVC release that

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx added a comment. In D110485#3026533 , @rjmccall wrote: > We should not add a standard feature with ABI impact if we aren't certain > we're going to match MSVC's ABI on it. I'm sorry, but this needs to wait > until there's an MSVC release that

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375466. expnkx added a comment. itanium + microsoft test both CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110485/new/ https://reviews.llvm.org/D110485 Files: clang/include/clang/Basic/Attr.td clang/test/SemaCXX/cxx2a-no-unique-address.cpp

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. We should not add a standard feature with ABI impact if we aren't certain we're going to match MSVC's ABI on it. I'm sorry, this needs to wait until MSVC releases a compiler that supports this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110485/new/

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375461. expnkx added a comment. I just find that Preprocessor test has a windows (msvc not gnu) specific test that tests no_unique_address. add that to patch and see whether it is mergable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110485/new/

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx added a comment. ninja: error: unknown target 'check-libc', did you mean 'check-lit'? what's wrong with this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110485/new/ https://reviews.llvm.org/D110485 ___ cfe-commits mailing list

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375431. expnkx added a comment. do not know why it cannot be built. upload and then verify again CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110485/new/ https://reviews.llvm.org/D110485 Files: clang/include/clang/Basic/Attr.td