[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

In D119017#3400347 , 
@devin.jeanpierre wrote:

> I don't have easy access to a Windows machine at work so am struggling with 
> the reproduction instructions.

Well, after much procrastinating, and an hour of fiddling with installing 
Visual Studio, I tried on Linux with the following RUN line:

  // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -triple i686-pc-win32

No joy. So I guess I really do need to reproduce this on Windows!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119017/new/

https://reviews.llvm.org/D119017

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123059: re-roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".""

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre created this revision.
devin.jeanpierre added a reviewer: gribozavr2.
Herald added subscribers: dexonsmith, pengfei.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
devin.jeanpierre requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This reverts commit b0bc93da926a943cdc2d8b04f8dcbe23a774520c 
.

Changes: `s/_WIN32/_WIN64/g`.

The calling convention kind is specific to 64-bit windows.
It's even in the name: `CCK_MicrosoftWin64`.

After this, the test passes with both `-triple i686-pc-win32` and `-triple 
x86_64-pc-win32`. Phew!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123059

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
+
+struct NontrivialDtor {
+  ~NontrivialDtor() {}
+};
+static_assert(!__is_trivially_relocatable(Non

[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

In D119017#3392961 , @Manna wrote:

> LIT test (SemaCXX/attr-trivial-abi.cpp) is failing for x86 build of clang. 
> The same failures are happening with our downstream X86 clang build.
> [...]
> The test might need to be updated so that shouldn't run with x86 build. 
> @devin.jeanpierre, could you please look at the test failures? Thanks

A couple questions -- was that failure introduced by this change, or a 
different change? Also, what target triple, so that I can reproduce? (New to 
clang development...)

(I am hoping that I'm not expected to roll back this change after it's been in 
for a month.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119017/new/

https://reviews.llvm.org/D119017

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

In D119017#3392993 , @Manna wrote:

>>> lf so, what target triple, so that I can reproduce? (New to clang 
>>> development...)
>
> You can reproduce the test failure if you checkout 32 bit clang compiler.

Please bear with me, how do I do this? :(


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119017/new/

https://reviews.llvm.org/D119017

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

Please revert it, I don't have easy access to a Windows machine at work so am 
struggling with the reproduction instructions.

This has failed twice, and this time with a one month delay! Is there any way 
to run and confirm all tests pass before I try to merge again?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119017/new/

https://reviews.llvm.org/D119017

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

W! Thank you for submitting, and thanks everyone for the kind and helpful 
reviews. 😀


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

Oops, sorry about that. What is the correct way to test/reproduce the change? 
Do I / can I submit builds to the buildbot manually for testing?

Also, should I be rolling back this change, or no? Not sure of the protocol 
here, this is my first change to Clang.

P.S. that breakage is disturbing -- I would not have expected this sort of 
failure on any platform, and I'm not sure if it reflects a bug on PS4. (It 
seems to be that structs are passed in registers, even if the member variables 
of that struct cannot be.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

Thank you @gribozavr2 ! I'll hopefully have a roll-forward ready for you either 
today or tomorrow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

The core difference of behavior is this, in the logic for setting 
`canPassInRegisters`:

  // Clang <= 4 used the pre-C++11 rule, which ignores move operations.
  // The PS4 platform ABI follows the behavior of Clang 3.2.
  if (CCK == TargetInfo::CCK_ClangABI4OrPS4)
return !D->hasNonTrivialDestructorForCall() &&
   !D->hasNonTrivialCopyConstructorForCall();

This is noticeably different from the non-PS4 behavior: not only does it ignore 
the move constructor, but it also does not require that a copy constructor or 
destructor even exist. And so all of the broken tests are structs where the 
copy constructor is deleted and the destructor is either trivial or deleted. On 
non-`CCK_ClangABI4OrPS4`, it is not passable in registers, but on 
`CCK_ClangABI4OrPS4` it is.

Related, but separate: DR1734, which is the same thing, but for 
`std::is_trivially_copyable`.

I believe the fix here, like for Windows, is to special-case this platform in 
the test. (Or delete these tests.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre created this revision.
devin.jeanpierre added a reviewer: gribozavr2.
Herald added a reviewer: aaron.ballman.
Herald added a subscriber: dexonsmith.
devin.jeanpierre requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This reverts commit 852afed5e0200b70047c28ccf4424a17089d17b0 
.

Changes since D114732 :

On PS4, we reverse the expectation that classes whose constructor is deleted 
are not trivially relocatable. Because, at the moment, only classes which are 
passed in registers are trivially relocatable, and PS4 allows passing in 
registers if the copy constructor is deleted, the original assertions were 
broken on PS4.

(This is kinda similar to DR1734.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119017

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomple

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devinj.jeanpierre created this revision.
devinj.jeanpierre added reviewers: rsmith, aaron.ballman, rjmccall, ahatanak.
Herald added a subscriber: dexonsmith.
devinj.jeanpierre requested review of this revision.
Herald added a project: clang.

This change enables library code to skip paired move-construction and
destruction for `trivial_abi` types, as if they were trivially-movable and
trivially-destructible. This offers an extension to the performance fix offered
by `trivial_abi`: rather than only offering trivial-type-like performance for
pass-by-value, it also offers it for library code that moves values but not as
arguments.

For example, if we use `memcpy` for trivially relocatable types inside of
vector reallocation, and mark `unique_ptr` as `trivial_abi` (via
`_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI` / `_LIBCPP_ABI_UNSTABLE` / etc.),
this would speed up `vector::push_back` by 40% on my benchmarks.
(Though note that in this case, the compiler could have done this anyway, but
happens not to due to the inlining horizon.)

If accepted, I intend to follow up with exactly such changes to library code,
including and especially `std::vector`, making them use a trivial relocation
operation on trivially relocatable types.

D50119  and P1144 
:

This change is very similar to D50119 , which 
was rejected from Clang. (That
change was an implementation of P1144 , which 
is not yet part of the C++
standard.)

The intent of this change, rather than trying to pick a winning proposal for
trivial relocation operations, is to extend the behavior of `trivial_abi` in a
way that could be made compatible with any such proposal. If P1144 
 or any
similar proposal were accepted, then `trivial_abi`,
`__is_trivially_relocatable`, and everything else in this change would be
redefined in terms of that.

Safety:

It's worth pointing out, specifically, that `trivial_abi` already implies
trivial relocatability in a narrow sense: a `trivial_abi` type, when passed by
value, has its constructor run in one location, and its destructor run in
another, after the type has been trivially relocated (through registers).

Trivial relocatability optimizations could change the number of paired
constructor/destructor calls, but this seems unlikely to matter for
`trivial_abi` types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114732

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnr

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devinj.jeanpierre added a comment.

Just a heads up, I think this is my first change to clang or llvm, and I'd 
appreciate any feedback you have on the code, review process, etc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

Wow, thanks for the quick response! I really appreciate it.

In D114732#3159247 , @Quuxplusone 
wrote:

> - (Major point of contention) To me, `[[trivial_abi]]` does not imply 
> `[[trivially_relocatable]]` at all. I believe @rjmccall disagreed a few years 
> ago: basically I said "The two attributes are orthogonal," with practical 
> demonstration, and basically he said "yes, in practice you're right, but 
> philosophically that's a bug and we do actually //intend// `[[trivial_abi]]` 
> to imply `[[trivially_relocatable]]` even though the compiler doesn't enforce 
> it" (but then AFAIK nothing was ever done about that).

I agree with you that `[[clang::trivial_abi]]` could be taken not to imply 
`[[trivially_relocatable]]`. However, I also think we can choose for it to 
imply it -- the meaning of `[[clang::trivial_abi]]` is up to us! It does not 
seem very **useful** to have a type which is `trivial_abi`, but not trivially 
relocatable: such a type can be relocated when being passed by value, but is 
leaving performance on the floor everywhere else. It might be that such a type 
really depends on the number of paired constructor/destructor calls, but this 
seems unlikely to be a realistic concern, especially since the number of paired 
constructor/destructor calls can change as a result of implementation changes 
in e.g. `std::vector`'s reallocation algorithm anyway.

As you noted, "nothing was ever done about that". This change seeks to address 
that problem. :B

> Therefore, I support something close to this patch, but I strongly believe 
> that you need to split up the "trivially relocatable" attribute itself from 
> "trivial abi". They are represented orthogonally in the AST, and they should 
> be toggleable orthogonally by the end-user. The end-user might want a 
> trivial-abi type that is not trivially-relocatable, or a 
> trivially-relocatable type that is not trivial-abi.

Leaving aside differences of opinion on how important it is, a version of the 
standards proposal could maintain linear independence, though not 
orthogonality: we could imagine writing `struct [[clang::trivial_abi]] 
[[trivially_relocatable(false]] Foo { ... };` and the like to achieve every 
combination.

I do agree that it's not true that every trivially relocatable type should be 
trivial for calls. This can produce bad performance outcomes in the case of 
e.g. non-inline functions which accept the trivial-for-calls value by 
pointer/reference, such as methods. Calling those can force the value to be put 
back in the stack, adding additional (trivial) copies where a non-trivial type 
would've had none. And so, even with this change, we would still want to be 
able to specify trivial relocatability separately, without implying 
trivial-for-calls.

A bigger problem is that my perception from reading that review thread was that 
any `[[trivially_relocatable]]` attribute proposal was a non-starter: this is 
the subject of a standards proposal which has not yet been accepted and clang 
doesn't want to "pick a winner". So my hope is that rather than picking a 
winner there, we extend the behavior of `trivial_abi`, in a way that is 
compatible with and desirable to have in combination with any accepted 
standards proposal. Hopefully, by being a bit less ambitious and complete, and 
reducing scope only to changes in behavior for `trivial_abi`, the resulting 
change to clang will be more acceptable.

This does mean that some performance is left on the floor -- any types which 
can't be marked `trivial_abi` will not get the benefit of trivial 
relocatability -- but that's what the standards proposal can aim to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre updated this revision to Diff 401035.
devin.jeanpierre added a comment.

Update to pass on Windows (untested right now).

On Windows, unlike Itanium ABI, the only special member functions used are the 
copy constructor and the destructor. If the copy constructor is deleted, then 
the type is not considered trivial for calls, even if it has a move constructor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
+
+struct NontrivialDtor {
+  ~NontrivialDtor() {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialDtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialDtor[]), "");
+
+struct NontrivialCopyCtor {
+  NontrivialCopyCtor(const NontrivialCopyCtor&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor), "");
+static_a

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

OK, while I'm struggling to set up a new Windows machine so I can make sure 
this works on Windows...  @Quuxplusone, after this is merged, do you want to 
rebase D67524  on top of this, or should I? I 
can review it -- I think when I looked at it, I only had two ideas for changes:

1. May need to guard all of these optimizations on the allocator being the 
standard allocator, since otherwise the change in the number of 
constructor/destructor calls would be observable to more than just the 
annotated type.

2. changing `std::swap` to correctly handle potentially-overlapping-objects. My 
thought is we could test that there's no reusable tail padding.

First draft: `has_unique_object_representations` is conservative -- on the 
Itanium ABI, "POD for the purposes of layout" types can have padding which 
isn't reused when it's a potentially overlapping subobject.

Second draft: check by hand:

  
  struct TestStruct {
[[no_unique_address]] T x;
// not sure if this needs to be a bitfield or anything like that, but the 
idea is this.
char extra_byte;
  };
  bool has_padding = sizeof(TestStruct) == sizeof(T);
  

Third draft: we can improve on this further still by iterating over different 
numbers of characters to determine the exact size of the padding, and memcpy 
only the non-padding parts.

---

(D63620  could in some form also be ported 
over, but it needs to be guarded behind ABI stability, since 
`[[clang::trivial_abi]]` is an ABI breaking change. For example, the same way 
it was done for unique_ptr 
, with the same 
benefits.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre updated this revision to Diff 401358.
devin.jeanpierre added a comment.

Clarify Windows comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
+
+struct NontrivialDtor {
+  ~NontrivialDtor() {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialDtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialDtor[]), "");
+
+struct NontrivialCopyCtor {
+  NontrivialCopyCtor(const NontrivialCopyCtor&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor[]), "");
+
+struct NontrivialMoveCtor {
+  NontrivialMoveCtor(NontrivialMoveCtor&&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialMoveCtor), "");
+static_assert(!__is_trivially_relocatable(Nontriv

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

CI test finished successfully before windows setup did 😢. My workplace's 
Windows VMs are a bit hosed at the moment...

In D114732#3253416 , @Quuxplusone 
wrote:

> In D114732#3253142 , 
> @devin.jeanpierre wrote:
>
>> OK, while I'm struggling to set up a new Windows machine so I can make sure 
>> this works on Windows...  @Quuxplusone, after this is merged, do you want to 
>> rebase D67524  on top of this, or should I? 
>> I can review it -- I think when I looked at it, I only had two ideas for 
>> changes:
>>
>> 1. May need to guard all of these optimizations on the allocator being the 
>> standard allocator, since otherwise the change in the number of 
>> constructor/destructor calls would be observable to more than just the 
>> annotated type.
>
> My intent is that the type-traits `__has_trivial_construct` and 
> `__has_trivial_destroy` already handle that. The standard `std::allocator` 
> has a trivial `construct` and `destroy`, but so do lots of user-defined 
> allocators.

Fair, I'm sorry for missing that detail.

>> 2. changing `std::swap` to correctly handle potentially-overlapping-objects. 
>> My thought is we could test that there's no reusable tail padding.
>>
>> First draft: `has_unique_object_representations` is conservative -- on the 
>> Itanium ABI, "POD for the purposes of layout" types can have padding which 
>> isn't reused when it's a potentially overlapping subobject.
>
> I'd //like// something that triggers successfully even when there are 
> internal padding bytes, e.g. `struct TR { int x; std::string y; }`. (But 
> anything is better than nothing. I'm going to keep maintaining my branch for 
> the foreseeable future, so I can keep doing my thing no matter what 
> half-measure makes it into trunk.)

Well, we must do something. In particular, consider this code:

  // class to make it non-POD for the purpose of layout
  class X { int64_t a; int8_t b;};
  class Y : public X {int8_t c;};
  
  Y y1 = ...;
  Y y2 = ;
  X& x1 = y1;
  X& x2 = y2;
  
  std::swap(x1, x2);

Here, at least in some implementations, `c` will live inside the tail padding 
of `X`. Without any special guarding, if `swap` used a `memcpy` with 
`sizeof(x1)` etc., then it would overwrite the padding bytes, which include 
`z`. I don't think swap should do this.

>> Second draft: check by hand:
>>
>>   
>>   struct TestStruct {
>> [[no_unique_address]] T x;
>> // not sure if this needs to be a bitfield or anything like that, but 
>> the idea is this.
>> char extra_byte;
>>   };
>>   bool has_padding = sizeof(TestStruct) == sizeof(T);
>
> If this works, then either my mental model of `[[no_unique_address]]` is 
> wrong or my mental model of tail padding is wrong. I would expect more like
>
>   template struct DerivedFrom : public T { char extra_byte; };
>   template requires is_final_v struct DerivedFrom { char 
> x[sizeof(T)+1]; }; // definitely no objects in the tail padding if it's final
>   bool has_padding = sizeof(DerivedFrom) == sizeof(T);
>
> If `T` is final, then it can't have stuff in its tail padding, I think: 
> https://godbolt.org/z/69sjf15MY

Unfortunately, even if `T` is final, it can have things in its tail padding: 
`[[no_unique_address]]` allows all the same optimizations as a base class, 
including reuse of tail padding and ECBO. See e.g. the cppreference page 
.

The reason your godbolt link shows them having the same size is that the `A` 
struct is "POD for the purpose of layout". In the Itanium ABI, POD types are 
laid out as normal, but their tail padding isn't reused in any circumstances 
(not for subclasses either -- so try e.g. removing final and using inheritance, 
and the assertion still passes: https://godbolt.org/z/MWrMoEbvb). If you make 
that a `class` instead of a `struct`, it isn't POD for the purpose of layout 
(due to private members), and the assertion fails now that the tail padding can 
be reused: https://godbolt.org/z/5edfqrK47

However, I think you're right that this isn't enough -- I don't believe it's 
guaranteed that inheritance and `[[no_unique_address]]` use padding in the 
exact same way, so you probably need to try both just to be safe.

> Anyway, that seems like a reasonable path in the name of getting something 
> merged. My //personal// druthers is that the Standard should just admit that 
> `std::swap` was never meant to work on polymorphic objects, so therefore it 
> can assume it's never swapping `Derived` objects via `Base&`, so therefore it 
> can assume tail padding is not an issue, so therefore no metaprogramming is 
> needed and it can just always do the efficient thing (which is therefore what 
> I've implemented in D67524 ). But I know 
> that's unlikely to ever happen.

I don't think it m

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

Sorry, I missed your other comments. Let me know if there's anything else I 
didn't address.




Comment at: clang/test/SemaCXX/attr-trivial-abi.cpp:146
+#ifdef _WIN32
+static_assert(!__is_trivially_relocatable(CopyDeleted), "");
+#else

Quuxplusone wrote:
> I think you meant `s/CopyDeleted/S20/` here.
Ugh, copy-paste. Thanks, done.



Comment at: clang/test/SemaCXX/type-traits.cpp:2862
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+

Quuxplusone wrote:
> It's mildly surprising that an incomplete type can be trivially relocatable; 
> but it doesn't really matter, since nobody can be depending on this answer. 
> (`int[]` is not movable or destructible, so it's not relocatable — never mind 
> //trivially// relocatable.)
I think it is consistent with the other type traits in C++ -- for example, this 
assert would also pass:

```
static_assert(std::is_trivially_copyable_v, "");
```

The important thing is that this line fails to compile:

```
struct Incomplete;
bool unused = __is_trivially_relocatable(Incomplete);
```



Comment at: clang/test/SemaObjCXX/objc-weak-type-traits.mm:213-214
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);

Quuxplusone wrote:
> IIUC (which I probably don't): Both `__strong id` and `__weak id` //are// 
> trivially relocatable in the p1144 sense, but only `__strong id` is 
> trivial-for-purposes-of-ABI, and that's why only `__strong id` is being 
> caught here. Yes/no?
`__weak` can't be trivially relocatable in any proposal, because their address 
is registered in a runtime table, so that they can be nulled out when the 
object is deleted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre updated this revision to Diff 401379.
devin.jeanpierre added a comment.

Fix copy-paste error.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
+
+struct NontrivialDtor {
+  ~NontrivialDtor() {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialDtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialDtor[]), "");
+
+struct NontrivialCopyCtor {
+  NontrivialCopyCtor(const NontrivialCopyCtor&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor[]), "");
+
+struct NontrivialMoveCtor {
+  NontrivialMoveCtor(NontrivialMoveCtor&&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialMoveCtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialM

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

In D114732#3256046 , 
@devin.jeanpierre wrote:

> Sorry, I missed your other comments. Let me know if there's anything else I 
> didn't address.

I just noticed that there's a "Done" checkbox next to each comment, and it 
isn't automatically checked if I reply to it -- I suppose I should be checking 
these so that it's easier to keep track of what's left to do? Sorry, this is my 
first time using phabricator!




Comment at: clang/test/SemaCXX/attr-trivial-abi.cpp:9-11
+// On Windows, trivial relocatability depends only on a trivial copy 
constructor existing.
+// In this case, it is implicitly deleted. Similar concerns apply to later 
tests.
+static_assert(!__is_trivially_relocatable(a), "");

Quuxplusone wrote:
> devin.jeanpierre wrote:
> > Quuxplusone wrote:
> > > (1) Why should Windows be different from everyone else here?
> > > (2) AFAIK, a user-defined move ctor means the copy ctor is //not 
> > > implicitly declared//, but it's not //deleted//; so I think this comment 
> > > is slightly wrong.
> > > (2) AFAIK, a user-defined move ctor means the copy ctor is not implicitly 
> > > declared, but it's not deleted; so I think this comment is slightly wrong.
> > Sorry, I get that confused a lot. :)
> > 
> > > (1) Why should Windows be different from everyone else here?
> > In this change so far, an object is only considered trivially relocatable 
> > if it's trivial for calls, and Windows is non-conforming wrt what it 
> > considers trivial for calls: it won't consider something trivial for calls 
> > if it isn't trivially copyable.
> > 
> > Code: https://clang.llvm.org/doxygen/SemaDeclCXX_8cpp_source.html#l06558
> > 
> > It surprised me too. This is a good motivator IMO to support something like 
> > your proposal, and have types be trivially relocatable that aren't trivial 
> > for calls. This allows, as you mention elsewhere, for optimizations that 
> > aren't ABI-breaking, and for e.g. non-conforming platforms like this to 
> > still take advantage of trivial relocatability.
> > In this change so far, an object is only considered trivially relocatable 
> > if it's trivial for calls, and Windows [has a calling convention that's 
> > different from the Itanium C++ ABI's calling convention]: it won't consider 
> > something trivial for calls if it isn't trivially copyable.
> 
> Ah, I see, and I agree with your logic here. It's unintuitive but only for 
> the same reasons we've already hashed over: we're introducing an 
> `__is_trivially_relocatable(T)` that gives zero false-positives, but (for 
> now) frequent false-negatives, and this just happens to be one //additional// 
> source of false negatives on Win32 specifically. (And I keep tripping over it 
> because this frequent-false-negative `__is_trivially_relocatable(T)` is named 
> the same as D50119's "perfect" discriminator.)
> 
> Everyone's on board with the idea that we're promising to preserve the true 
> positives forever, but at the same time we're expecting that we might 
> //someday// fix the false negatives, right? I.e., nobody's expecting 
> `!__is_trivially_relocatable(a)` to remain true on Win32 forever? (I'm 
> pretty sure we're all on board with this.)
> Everyone's on board with the idea that we're promising to preserve the true 
> positives forever, but at the same time we're expecting that we might someday 
> fix the false negatives, right? I.e., nobody's expecting 
> !__is_trivially_relocatable(a) to remain true on Win32 forever? (I'm 
> pretty sure we're all on board with this.)

+1

My hope is that future changes can fix the false negatives -- either with 
explicit annotations (like `[[trivially_relocatable]`), or with improved 
automatic inference, etc.

(For example, we could imagine marking types as trivially relocatable if they 
have *a* trivial (for calls) copy or move constructor and a trivial (for calls) 
destructor, even if they also have a nontrivial move or copy destructor. This 
would be a superset of trivial-for-calls types on all platforms, and especially 
Windows.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre updated this revision to Diff 401419.
devin.jeanpierre added a comment.

pull/rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
+
+struct NontrivialDtor {
+  ~NontrivialDtor() {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialDtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialDtor[]), "");
+
+struct NontrivialCopyCtor {
+  NontrivialCopyCtor(const NontrivialCopyCtor&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor[]), "");
+
+struct NontrivialMoveCtor {
+  NontrivialMoveCtor(NontrivialMoveCtor&&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialMoveCtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialMoveCtor[])

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

@rsmith I've pulled+rebased again to avoid the (looks like pre-existing) 
failure. Copy-pasting the wording on 
https://llvm.org/docs/MyFirstTypoFix.html#commit-by-proxy: I don’t have commit 
access, can you land this patch for me? Please use “Devin Jeanpierre 
jeanpierr...@google.com” to commit the change.

(I normally use the jeanpierreda email alias for external stuff, so that my 
internal username doesn't leak.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120086: Explicitly document that `__is_trivially_relocatable(T)` implies that `T` is implicit-lifetime.

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre created this revision.
devin.jeanpierre added reviewers: rsmith, rjmccall.
devin.jeanpierre requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Previous change: D114732 

The wording here might need some tweaking. What I want to guarantee is that 
everything valid for an implicit-lifetime type is also valid for these types. 
Either they're added to the set of implicit-lifetime types as an extension, or 
the operations are valid for additional non-implicit-lifetime types -- 
something like that.

Something to this effect is required for it to be defined behavior to `memcpy` 
to a new location, and then use the type at that new location. And so, this 
guarantee is necessary for `__is_trivially_relocatable` to be actually useful 
in practice. See e.g. D119385  for how this 
plays out.

AIUI, Clang is already fine with this and needs no code changes. Indeed, we see 
many types which are not what the standard would call implicit-lifetime, yet 
which use memcpy for copying: `std::vector` only checks for trivial move, and 
does not pair this with requiring trivial destroy. (See e.g. 
`__construct_forward_with_exception_guarantees`)

(Specifically: I believe that Clang is already fine with treating any 
non-polymorphic type as if it were implicit-lifetime, but I am not sure how to 
check this belief. There's not much in the way of explicit references to 
"implicit-lifetime" types in the codebase. Had a brief out of band conversation 
with rsmith, but may of course have misunderstood the conclusions here.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120086

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1372,8 +1372,9 @@
 * ``__is_trivially_relocatable`` (Clang): Returns true if moving an object
   of the given type, and then destroying the source object, is known to be
   functionally equivalent to copying the underlying bytes and then dropping the
-  source object on the floor. This is true of trivial types and types which
-  were made trivially relocatable via the ``clang::trivial_abi`` attribute.
+  source object on the floor. (Note: any such type is, therefore, 
implicit-lifetime.)
+  This is true of trivial types and types which were made trivially relocatable
+  via the ``clang::trivial_abi`` attribute.
 * ``__is_union`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_unsigned`` (C++, Embarcadero):
   Returns false for enumeration types. Note, before Clang 13, returned true for


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1372,8 +1372,9 @@
 * ``__is_trivially_relocatable`` (Clang): Returns true if moving an object
   of the given type, and then destroying the source object, is known to be
   functionally equivalent to copying the underlying bytes and then dropping the
-  source object on the floor. This is true of trivial types and types which
-  were made trivially relocatable via the ``clang::trivial_abi`` attribute.
+  source object on the floor. (Note: any such type is, therefore, implicit-lifetime.)
+  This is true of trivial types and types which were made trivially relocatable
+  via the ``clang::trivial_abi`` attribute.
 * ``__is_union`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_unsigned`` (C++, Embarcadero):
   Returns false for enumeration types. Note, before Clang 13, returned true for
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120086: Explicitly document that `__is_trivially_relocatable(T)` implies that `T` is implicit-lifetime.

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre updated this revision to Diff 409794.
devin.jeanpierre added a comment.

Rebase off of D119385 . Sorry, I'm bad at git.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120086/new/

https://reviews.llvm.org/D120086

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1372,8 +1372,9 @@
 * ``__is_trivially_relocatable`` (Clang): Returns true if moving an object
   of the given type, and then destroying the source object, is known to be
   functionally equivalent to copying the underlying bytes and then dropping the
-  source object on the floor. This is true of trivial types and types which
-  were made trivially relocatable via the ``clang::trivial_abi`` attribute.
+  source object on the floor. (Note: any such type is, therefore, 
implicit-lifetime.)
+  This is true of trivial types and types which were made trivially relocatable
+  via the ``clang::trivial_abi`` attribute.
 * ``__is_union`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_unsigned`` (C++, Embarcadero):
   Returns false for enumeration types. Note, before Clang 13, returned true for


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1372,8 +1372,9 @@
 * ``__is_trivially_relocatable`` (Clang): Returns true if moving an object
   of the given type, and then destroying the source object, is known to be
   functionally equivalent to copying the underlying bytes and then dropping the
-  source object on the floor. This is true of trivial types and types which
-  were made trivially relocatable via the ``clang::trivial_abi`` attribute.
+  source object on the floor. (Note: any such type is, therefore, implicit-lifetime.)
+  This is true of trivial types and types which were made trivially relocatable
+  via the ``clang::trivial_abi`` attribute.
 * ``__is_union`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_unsigned`` (C++, Embarcadero):
   Returns false for enumeration types. Note, before Clang 13, returned true for
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre updated this revision to Diff 392214.
devin.jeanpierre added a comment.

Suggested changes from code review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_FALSE(__is_trivially_relocatable, __strong id); // Ideally, this would be TRAIT_IS_TRUE.
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
+
+struct NontrivialDtor {
+  ~NontrivialDtor() {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialDtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialDtor[]), "");
+
+struct NontrivialCopyCtor {
+  NontrivialCopyCtor(const NontrivialCopyCtor&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor[]), "");
+
+struct NontrivialMoveCtor {
+  NontrivialMoveCtor(NontrivialMoveCtor&&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialMoveCtor), "")

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

(Sorry for delayed reply -- I made the mistake of signing up to phabricator 
with my personal email, which I don't check very well, apparently!)




Comment at: clang/include/clang/Basic/AttrDocs.td:3211-3215
+The compiler will pass and return a trivially relocatable type using the C ABI
+for the underlying type, even when the type would otherwise be considered
+non-trivially-relocatable. If a type is trivially relocatable, has a
+non-trivial destructor, and is passed as an argument by value, the convention
+is that the callee will destroy the object before returning.

rsmith wrote:
> I think this documentation change has mixed together two different things. 
> The revised wording says that `[[trivial_abi]]` implies trivially-relocatable 
> and that trivially-relocatable implies passing using the C ABI, which is 
> wrong: the second implication does not hold. What we should say is that 
> `[[trivial_abi]]` (if we're not in the "has no effect" case described below) 
> implies both that the type is trivially-relocatable, and that it is passed 
> using the C ABI (that is, that we trivially relocate in argument passing).
> 
> Instead of the wording changes you have here, perhaps we could leave the old 
> wording alone and add a paragraph that says that a type with the attribute is 
> assumed to be trivially-relocatable for the purpose of 
> `__is_trivially_relocatable`, but that Clang doesn't yet take advantage of 
> this fact anywhere other than argument passing.
Yeah, this was too aggressive a wording change, and might easily change later 
when `[[trivially_relocatable]]` is added.

I did drop the "Clang doesn't yet take advantage of this fact anywhere other 
than argument passing.", because I feel like -- does that sort of sentiment 
belong instead in the docs for `__is_trivially_relocatable`? Maybe I should 
change `__is_trivially_relocatable` to note that this is never taken advantage 
of anywhere except in by-value argument passing (when the type is trivial for 
the purpose of calls) and library code.



Comment at: clang/lib/AST/Type.cpp:2500
+return BaseElementType.isTriviallyCopyableType(Context) &&
+   !BaseElementType.isDestructedType();
+  }

rsmith wrote:
> You can simplify this a little by dropping this `isDestructedType()` check; 
> trivially copyable implies trivially destructible.
> 
> It would be a little more precise to check for 
> `!isNonTrivialToPrimitiveDestructiveMove() && !isDestructedType()` here; 
> that'd treat `__autoreleasing` pointers as trivially-relocatable, but not 
> `__strong` pointers (because "destructive move" there actually means 
> "non-destructive move" and needs to leave the object in a valid but 
> unspecified state, which means nulling out a moved-from `__strong` pointer). 
> I think getting this "fully" right would require a bit more plumbing to 
> properly handle the case of a `struct` containing a `__strong` pointer (which 
> is trivially relocatable but not trivially anything else).
Oooh. `isNonTrivialToPrimitiveDestructiveMove` is perfect.

Actually, a struct containing a `__strong` pointer is already handled -- Clang 
[already](https://github.com/llvm/llvm-project/blob/603c1a62f8595f70c3e96ecbec8976d411c0cc08/clang/lib/AST/DeclCXX.cpp#L1037-L1059)
 ensures they're passed in registers, and does all the appropriate logic. It's 
just `__strong` pointers themselves that I'd need to handle the same way here. 
Unfortunately, I really, *really* don't understand the source code dealing with 
`__strong` pointers in structs. (In particular, I don't understand the 
difference between GC::Strong and ObjCLifetime::OCL_Strong, plus I'm uncomfy 
with the global language opts.)

It's tempting to use the return value of 
`isNonTrivialToPrimitiveDestructiveMove` and check for `Strong` -- but now 
that's some *third* thing, and not mirroring the logic of when it's in a struct 
as a data member.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre updated this revision to Diff 392606.
devin.jeanpierre added a comment.

Use `PCK_ARCStrong` to check for ObjC strong pointers, marking them as 
trivially relocatable as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
+
+struct NontrivialDtor {
+  ~NontrivialDtor() {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialDtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialDtor[]), "");
+
+struct NontrivialCopyCtor {
+  NontrivialCopyCtor(const NontrivialCopyCtor&) {}
+};
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor), "");
+static_assert(!__is_trivially_relocatable(NontrivialCopyCtor[]), "");
+
+struct NontrivialMoveCtor {
+  NontrivialMoveCtor(NontrivialMoveCtor&&) {}
+};
+static_assert(!__is_trivially_relocatable

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

(Sorry, I think I'm doing threading wrong here due to lack of experience with 
phabricator. The reply buttons are grayed out!)

> 1. All trivial-abi types are in fact trivially relocated, specifically when 
> they are passed to functions.
> 2. Therefore, all trivial-abi types are "trivially relocatable" in general.

This sounds mostly good so far. For example:

  class [[clang::trivial_abi]] MyType { ... };
  
  MyType Create() {
MyType x;
/* do stuff */
return x;
  }
  
  void Consume(MyType) {}
  int main() {
Consume(Create());
  }

Here, you can place `x` into any valid state for a `MyType`, and it will be 
trivially relocated in the call to `Consume`, without any intermediate steps, 
thanks to guaranteed copy elision. If it is not safe to trivially relocate in 
general, then this is not a sound use of `trivial_abi`.

So I think some version of 2 is correct: **safe** use of 
`[[clang::trivial_abi]]` requires that it be safe to relocate for all publicly 
reachable object states.

(This isn't //watertight//: perhaps a non-publicly reachable state, only 
achieved temporarily internally, is not trivially relocatable. Then such a type 
would still be safe to use via its public API, but not be trivially relocatable 
in general, and very dangerous to use within its private API. By adding the 
optimizations, we'd add more footguns to such a type inside these sections of 
code.)

> 3. Therefore, all trivial-abi types are safe to optimize in all the ways 
> depicted in D67524 .

This is not //quite// right. It might be safe to relocate, in that adding a new 
relocation operation does not change program behavior, but it doesn't quite 
follow that it's safe to stop calling the move constructor and destructor. So 
e.g. some type which relies on exactly paired counts of move constructors / 
destructors might get upset.

Also, there is that parenthetical above: if a type can sometimes reach states 
//internally// that are not trivially relocatable, and in addition it calls 
routines like `swap` etc. while in such a state, then a relocation optimization 
would be unsound.

Nonetheless, my hope is that these can be considered contrived or unsound cases 
which we will not support for `trivial_abi`.




Comment at: clang/lib/AST/Type.cpp:2500
+return BaseElementType.isTriviallyCopyableType(Context) &&
+   !BaseElementType.isDestructedType();
+  }

rsmith wrote:
> devin.jeanpierre wrote:
> > rsmith wrote:
> > > You can simplify this a little by dropping this `isDestructedType()` 
> > > check; trivially copyable implies trivially destructible.
> > > 
> > > It would be a little more precise to check for 
> > > `!isNonTrivialToPrimitiveDestructiveMove() && !isDestructedType()` here; 
> > > that'd treat `__autoreleasing` pointers as trivially-relocatable, but not 
> > > `__strong` pointers (because "destructive move" there actually means 
> > > "non-destructive move" and needs to leave the object in a valid but 
> > > unspecified state, which means nulling out a moved-from `__strong` 
> > > pointer). I think getting this "fully" right would require a bit more 
> > > plumbing to properly handle the case of a `struct` containing a 
> > > `__strong` pointer (which is trivially relocatable but not trivially 
> > > anything else).
> > Oooh. `isNonTrivialToPrimitiveDestructiveMove` is perfect.
> > 
> > Actually, a struct containing a `__strong` pointer is already handled -- 
> > Clang 
> > [already](https://github.com/llvm/llvm-project/blob/603c1a62f8595f70c3e96ecbec8976d411c0cc08/clang/lib/AST/DeclCXX.cpp#L1037-L1059)
> >  ensures they're passed in registers, and does all the appropriate logic. 
> > It's just `__strong` pointers themselves that I'd need to handle the same 
> > way here. Unfortunately, I really, *really* don't understand the source 
> > code dealing with `__strong` pointers in structs. (In particular, I don't 
> > understand the difference between GC::Strong and ObjCLifetime::OCL_Strong, 
> > plus I'm uncomfy with the global language opts.)
> > 
> > It's tempting to use the return value of 
> > `isNonTrivialToPrimitiveDestructiveMove` and check for `Strong` -- but now 
> > that's some *third* thing, and not mirroring the logic of when it's in a 
> > struct as a data member.
> The existing handling only covers Objective-C++ and not (non-C++) Objective-C 
> (the C and C++ codepaths for computing struct properties are entirely 
> different, because the C++ side of things is primarily working out properties 
> of special member functions). It looks like your test coverage doesn't cover 
> the non-C++ side of this.
I //think// this is fine. The keyword `__is_trivially_relocatable` only works 
in (Objective) C++ source files (KEYCXX), and so in order for it to be 
callable, the struct must have been defined in a C++ source file to begin with, 
right? At least, assuming ObjC works the same as C --

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

In D114732#3178312 , @rjmccall wrote:

> Trivial relocation doesn't imply that types have to be safe against being 
> suddenly relocated during the middle of operations while they're not in a 
> safe internal state.  That is not a consideration.

In fact, I want to walk back that concern altogether. As a simple example, I 
think `swap()` is allowed to be defined as so:

  template 
  void swap(T& lhs, T& rhs) {
  ([&](T tmp) {lhs = std::move(rhs); rhs = 
std::move(tmp);})(std::move(lhs));
  }

It's really hard to argue that a type can sensibly call out to library code 
while in a state that is non-relocatable, despite self-labeling as 
`[[clang::trivial_abi]]`.  If the library is allowed to move the value, it's 
allowed to move it into a parameter, causing a relocation! So I actually regret 
pointing them out, or thinking that they might be a concern. :)

(Demonstration code: https://godbolt.org/z/j88Kqd49a)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits