Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][Sema] Track trivial-relocatability as a type trait #84621

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

AMP999
Copy link
Contributor

@AMP999 AMP999 commented Mar 9, 2024

To resolve #69394, this patch separates trivial-relocatability's logic from canPassInRegisters to decide if a type is trivial-relocatable. A type passed in registers doesn't necessarily mean trivial-relocatability of that type(e.g. on Windows) i.e. it gives us an unintended false positive. This change would be beneficial for Abseil since they rely upon these semantics. By these changes now:
User-provided special members prevent natural trivial-relocatabilitiy.
It's important because Abseil and maybe others assume the assignment operator doesn't have an impact on the trivial-relocatability of a type.
In fact, it does have an effect, and with a user-provided assignment operator, the compiler should only accept it as trivial-relocatable if it's implied by the [[clang::trivial_abi]] attribute.
Just because a type can pass in registers doesn't necessarily mean it's trivial-relocatable. The [[clang::trivial_abi]] attribute always implies trivial-relocatability, even if it can't pass in registers. The trait has extensive tests for both old and new behaviors. Test aggregation of both kinds of types as data members; inheritance; virtual member functions and virtual bases; const and reference data members; and reference types.

Fixes #69394

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 9, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 9, 2024

@llvm/pr-subscribers-clang

Author: Amirreza Ashouri (AMP999)

Changes

To resolve llvm#69394, this patch separates trivial-relocatability's logic from canPassInRegisters to decide if a type is trivial-relocatable. A type passed in registers doesn't necessarily mean trivial-relocatability of that type(e.g. on Windows) i.e. it gives us an unintended false positive. This change would be beneficial for Abseil since they rely upon these semantics. By these changes now:
User-provided special members prevent natural trivial-relocatabilitiy.
It's important because Abseil and maybe others assume the assignment operator doesn't have an impact on the trivial-relocatability of a type.
In fact, it does have an effect, and with a user-provided assignment operator, the compiler should only accept it as trivial-relocatable if it's implied by the [[clang::trivial_abi]] attribute.
Just because a type can pass in registers doesn't necessarily mean it's trivial-relocatable. The [[clang::trivial_abi]] attribute always implies trivial-relocatability, even if it can't pass in registers. The trait has extensive tests for both old and new behaviors. Test aggregation of both kinds of types as data members; inheritance; virtual member functions and virtual bases; const and reference data members; and reference types.

Fixes llvm#69394


Patch is 21.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84621.diff

7 Files Affected:

  • (modified) clang/include/clang/AST/CXXRecordDeclDefinitionBits.def (+5)
  • (modified) clang/include/clang/AST/DeclCXX.h (+3)
  • (modified) clang/lib/AST/DeclCXX.cpp (+42-3)
  • (modified) clang/lib/AST/Type.cpp (+4-2)
  • (modified) clang/test/SemaCXX/attr-trivial-abi.cpp (-35)
  • (added) clang/test/SemaCXX/is-trivially-relocatable.cpp (+106)
  • (modified) clang/test/SemaCXX/type-traits.cpp (+68)
diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
index cdf0804680ad0a..36d3cfb7dfe85b 100644
--- a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
+++ b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -189,6 +189,11 @@ FIELD(DeclaredNonTrivialSpecialMembers, 6, MERGE_OR)
 /// SMF_MoveConstructor, and SMF_Destructor are meaningful here.
 FIELD(DeclaredNonTrivialSpecialMembersForCall, 6, MERGE_OR)
 
+/// True when this class's bases and fields are all trivially relocatable
+/// or references, and the class itself has no user-provided special
+/// member functions.
+FIELD(IsNaturallyTriviallyRelocatable, 1, NO_MERGE)
+
 /// True when this class has a destructor with no semantic effect.
 FIELD(HasIrrelevantDestructor, 1, NO_MERGE)
 
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 9cebaff63bb0db..a58126c98597b0 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1386,6 +1386,9 @@ class CXXRecordDecl : public RecordDecl {
         (SMF_CopyConstructor | SMF_MoveConstructor | SMF_Destructor);
   }
 
+  /// Determine whether this class is trivially relocatable
+  bool isTriviallyRelocatable() const;
+
   /// Determine whether declaring a const variable with this type is ok
   /// per core issue 253.
   bool allowConstDefaultInit() const {
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 117e802dae2d9d..ae9de533889f2c 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -95,7 +95,8 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
       DefaultedDestructorIsDeleted(false), HasTrivialSpecialMembers(SMF_All),
       HasTrivialSpecialMembersForCall(SMF_All),
       DeclaredNonTrivialSpecialMembers(0),
-      DeclaredNonTrivialSpecialMembersForCall(0), HasIrrelevantDestructor(true),
+      DeclaredNonTrivialSpecialMembersForCall(0),
+      IsNaturallyTriviallyRelocatable(true), HasIrrelevantDestructor(true),
       HasConstexprNonCopyMoveConstructor(false),
       HasDefaultedDefaultConstructor(false),
       DefaultedDefaultConstructorIsConstexpr(true),
@@ -279,6 +280,10 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
 
       //   An aggregate is a class with [...] no virtual functions.
       data().Aggregate = false;
+
+      // A trivially relocatable class is a class:
+      // -- which has no virtual member functions or virtual base classes
+      data().IsNaturallyTriviallyRelocatable = false;
     }
 
     // C++0x [class]p7:
@@ -293,6 +298,9 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
     if (!hasNonLiteralTypeFieldsOrBases() && !BaseType->isLiteralType(C))
       data().HasNonLiteralTypeFieldsOrBases = true;
 
+    if (Base->isVirtual() || !BaseClassDecl->isTriviallyRelocatable())
+      data().IsNaturallyTriviallyRelocatable = false;
+
     // Now go through all virtual bases of this base and add them.
     for (const auto &VBase : BaseClassDecl->vbases()) {
       // Add this base if it's not already in the list.
@@ -570,6 +578,10 @@ bool CXXRecordDecl::hasAnyDependentBases() const {
   return !forallBases([](const CXXRecordDecl *) { return true; });
 }
 
+bool CXXRecordDecl::isTriviallyRelocatable() const {
+  return (data().IsNaturallyTriviallyRelocatable || hasAttr<TrivialABIAttr>());
+}
+
 bool CXXRecordDecl::isTriviallyCopyable() const {
   // C++0x [class]p5:
   //   A trivially copyable class is a class that:
@@ -758,6 +770,10 @@ void CXXRecordDecl::addedMember(Decl *D) {
       //    -- has no virtual functions
       data().IsStandardLayout = false;
       data().IsCXX11StandardLayout = false;
+
+      // A trivially relocatable class is a class:
+      // -- which has no virtual member functions or virtual base classes
+      data().IsNaturallyTriviallyRelocatable = false;
     }
   }
 
@@ -824,6 +840,14 @@ void CXXRecordDecl::addedMember(Decl *D) {
               ? !Constructor->isImplicit()
               : (Constructor->isUserProvided() || Constructor->isExplicit()))
         data().Aggregate = false;
+
+      // A trivially relocatable class is a class:
+      // -- where no eligible copy constructor, move constructor, copy
+      // assignment operator, move assignment operator, or destructor is
+      // user-provided,
+      if (Constructor->isUserProvided() && (Constructor->isCopyConstructor() ||
+                                            Constructor->isMoveConstructor()))
+        data().IsNaturallyTriviallyRelocatable = false;
     }
   }
 
@@ -853,11 +877,18 @@ void CXXRecordDecl::addedMember(Decl *D) {
           Method->getNonObjectParameter(0)->getType()->getAs<ReferenceType>();
       if (!ParamTy || ParamTy->getPointeeType().isConstQualified())
         data().HasDeclaredCopyAssignmentWithConstParam = true;
+
+      if (Method->isUserProvided())
+        data().IsNaturallyTriviallyRelocatable = false;
     }
 
-    if (Method->isMoveAssignmentOperator())
+    if (Method->isMoveAssignmentOperator()) {
       SMKind |= SMF_MoveAssignment;
 
+      if (Method->isUserProvided())
+        data().IsNaturallyTriviallyRelocatable = false;
+    }
+
     // Keep the list of conversion functions up-to-date.
     if (auto *Conversion = dyn_cast<CXXConversionDecl>(D)) {
       // FIXME: We use the 'unsafe' accessor for the access specifier here,
@@ -1066,6 +1097,12 @@ void CXXRecordDecl::addedMember(Decl *D) {
     } else if (!T.isCXX98PODType(Context))
       data().PlainOldData = false;
 
+    // A trivially relocatable class is a class:
+    // -- all of whose members are either of reference type or of trivially
+    // relocatable type
+    if (!T->isReferenceType() && !T.isTriviallyRelocatableType(Context))
+      data().IsNaturallyTriviallyRelocatable = false;
+
     if (T->isReferenceType()) {
       if (!Field->hasInClassInitializer())
         data().HasUninitializedReferenceMember = true;
@@ -1420,8 +1457,10 @@ void CXXRecordDecl::addedEligibleSpecialMemberFunction(const CXXMethodDecl *MD,
   // See https://github.com/llvm/llvm-project/issues/59206
 
   if (const auto *DD = dyn_cast<CXXDestructorDecl>(MD)) {
-    if (DD->isUserProvided())
+    if (DD->isUserProvided()) {
       data().HasIrrelevantDestructor = false;
+      data().IsNaturallyTriviallyRelocatable = false;
+    }
     // If the destructor is explicitly defaulted and not trivial or not public
     // or if the destructor is deleted, we clear HasIrrelevantDestructor in
     // finishedDefaultedOrDeletedMember.
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 78dcd3f4007a5a..073d3b68f6d730 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2680,8 +2680,10 @@ bool QualType::isTriviallyRelocatableType(const ASTContext &Context) const {
     return false;
   } else if (!BaseElementType->isObjectType()) {
     return false;
-  } else if (const auto *RD = BaseElementType->getAsRecordDecl()) {
-    return RD->canPassInRegisters();
+  } else if (CXXRecordDecl *RD = BaseElementType->getAsCXXRecordDecl()) {
+    return RD->isTriviallyRelocatable();
+  } else if (BaseElementType.isTriviallyCopyableType(Context)) {
+    return true;
   } else {
     switch (isNonTrivialToPrimitiveDestructiveMove()) {
     case PCK_Trivial:
diff --git a/clang/test/SemaCXX/attr-trivial-abi.cpp b/clang/test/SemaCXX/attr-trivial-abi.cpp
index c215f90eb124ce..c1f39047f8af84 100644
--- a/clang/test/SemaCXX/attr-trivial-abi.cpp
+++ b/clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -5,15 +5,7 @@ void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' att
 // Should not crash.
 template <class>
 class __attribute__((trivial_abi)) a { a(a &&); };
-#if defined(_WIN64) && !defined(__MINGW32__)
-// On Windows/MSVC, to be trivial-for-calls, an object must be trivially copyable.
-// (And it is only trivially relocatable, currently, if it is trivial for calls.)
-// In this case, it is suppressed by an explicitly defined move constructor.
-// Similar concerns apply to later tests that have #if defined(_WIN64) && !defined(__MINGW32__)
-static_assert(!__is_trivially_relocatable(a<int>), "");
-#else
 static_assert(__is_trivially_relocatable(a<int>), "");
-#endif
 
 struct [[clang::trivial_abi]] S0 {
   int a;
@@ -39,14 +31,7 @@ struct __attribute__((trivial_abi)) S3_3 { // expected-warning {{'trivial_abi' c
   S3_3(S3_3 &&);
   S3_2 s32;
 };
-#ifdef __ORBIS__
-// The ClangABI4OrPS4 calling convention kind passes classes in registers if the
-// copy constructor is trivial for calls *or deleted*, while other platforms do
-// not accept deleted constructors.
-static_assert(__is_trivially_relocatable(S3_3), "");
-#else
 static_assert(!__is_trivially_relocatable(S3_3), "");
-#endif
 
 // Diagnose invalid trivial_abi even when the type is templated because it has a non-trivial field.
 template <class T>
@@ -118,30 +103,18 @@ struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'tri
   CopyMoveDeleted(const CopyMoveDeleted &) = delete;
   CopyMoveDeleted(CopyMoveDeleted &&) = delete;
 };
-#ifdef __ORBIS__
 static_assert(__is_trivially_relocatable(CopyMoveDeleted), "");
-#else
-static_assert(!__is_trivially_relocatable(CopyMoveDeleted), "");
-#endif
 
 struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
   CopyMoveDeleted a;
 };
-#ifdef __ORBIS__
 static_assert(__is_trivially_relocatable(S18), "");
-#else
-static_assert(!__is_trivially_relocatable(S18), "");
-#endif
 
 struct __attribute__((trivial_abi)) CopyDeleted {
   CopyDeleted(const CopyDeleted &) = delete;
   CopyDeleted(CopyDeleted &&) = default;
 };
-#if defined(_WIN64) && !defined(__MINGW32__)
-static_assert(!__is_trivially_relocatable(CopyDeleted), "");
-#else
 static_assert(__is_trivially_relocatable(CopyDeleted), "");
-#endif
 
 struct __attribute__((trivial_abi)) MoveDeleted {
   MoveDeleted(const MoveDeleted &) = default;
@@ -153,19 +126,11 @@ struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' ca
   CopyDeleted a;
   MoveDeleted b;
 };
-#ifdef __ORBIS__
 static_assert(__is_trivially_relocatable(S19), "");
-#else
-static_assert(!__is_trivially_relocatable(S19), "");
-#endif
 
 // This is fine since the move constructor isn't deleted.
 struct __attribute__((trivial_abi)) S20 {
   int &&a; // a member of rvalue reference type deletes the copy constructor.
 };
-#if defined(_WIN64) && !defined(__MINGW32__)
-static_assert(!__is_trivially_relocatable(S20), "");
-#else
 static_assert(__is_trivially_relocatable(S20), "");
-#endif
 } // namespace deletedCopyMoveConstructor
diff --git a/clang/test/SemaCXX/is-trivially-relocatable.cpp b/clang/test/SemaCXX/is-trivially-relocatable.cpp
new file mode 100644
index 00000000000000..3fd620792b69ce
--- /dev/null
+++ b/clang/test/SemaCXX/is-trivially-relocatable.cpp
@@ -0,0 +1,106 @@
+// RUN: %clang_cc1 -std=c++03 -fsyntax-only -verify %s -triple x86_64-windows-msvc
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s -triple x86_64-windows-msvc
+// RUN: %clang_cc1 -std=c++03 -fsyntax-only -verify %s -triple x86_64-apple-darwin10
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s -triple x86_64-apple-darwin10
+
+// expected-no-diagnostics
+
+#if __cplusplus < 201103L
+#define static_assert(...) __extension__ _Static_assert(__VA_ARGS__, "")
+// cxx98-error@-1 {{variadic macros are a C99 feature}}
+#endif
+
+template <class T>
+struct Agg {
+  T t_;
+};
+
+template <class T>
+struct Der : T {
+};
+
+template <class T>
+struct Mut {
+  mutable T t_;
+};
+
+template <class T>
+struct Non {
+  Non(); // make it a non-aggregate
+  T t_;
+};
+
+struct CompletelyTrivial {
+};
+static_assert(__is_trivially_relocatable(CompletelyTrivial));
+static_assert(__is_trivially_relocatable(Agg<CompletelyTrivial>));
+static_assert(__is_trivially_relocatable(Der<CompletelyTrivial>));
+static_assert(__is_trivially_relocatable(Mut<CompletelyTrivial>));
+static_assert(__is_trivially_relocatable(Non<CompletelyTrivial>));
+
+struct NonTrivialDtor {
+  ~NonTrivialDtor();
+};
+static_assert(!__is_trivially_relocatable(NonTrivialDtor));
+static_assert(!__is_trivially_relocatable(Agg<NonTrivialDtor>));
+static_assert(!__is_trivially_relocatable(Der<NonTrivialDtor>));
+static_assert(!__is_trivially_relocatable(Mut<NonTrivialDtor>));
+static_assert(!__is_trivially_relocatable(Non<NonTrivialDtor>));
+
+struct NonTrivialCopyCtor {
+  NonTrivialCopyCtor(const NonTrivialCopyCtor&);
+};
+static_assert(!__is_trivially_relocatable(NonTrivialCopyCtor));
+static_assert(!__is_trivially_relocatable(Agg<NonTrivialCopyCtor>));
+static_assert(!__is_trivially_relocatable(Der<NonTrivialCopyCtor>));
+static_assert(!__is_trivially_relocatable(Mut<NonTrivialCopyCtor>));
+static_assert(!__is_trivially_relocatable(Non<NonTrivialCopyCtor>));
+
+struct NonTrivialMutableCopyCtor {
+  NonTrivialMutableCopyCtor(NonTrivialMutableCopyCtor&);
+};
+static_assert(!__is_trivially_relocatable(NonTrivialMutableCopyCtor));
+static_assert(!__is_trivially_relocatable(Agg<NonTrivialMutableCopyCtor>));
+static_assert(!__is_trivially_relocatable(Der<NonTrivialMutableCopyCtor>));
+static_assert(!__is_trivially_relocatable(Mut<NonTrivialMutableCopyCtor>));
+static_assert(!__is_trivially_relocatable(Non<NonTrivialMutableCopyCtor>));
+
+#if __cplusplus >= 201103L
+struct NonTrivialMoveCtor {
+  NonTrivialMoveCtor(NonTrivialMoveCtor&&);
+};
+static_assert(!__is_trivially_relocatable(NonTrivialMoveCtor));
+static_assert(!__is_trivially_relocatable(Agg<NonTrivialMoveCtor>));
+static_assert(!__is_trivially_relocatable(Der<NonTrivialMoveCtor>));
+static_assert(!__is_trivially_relocatable(Mut<NonTrivialMoveCtor>));
+static_assert(!__is_trivially_relocatable(Non<NonTrivialMoveCtor>));
+#endif
+
+struct NonTrivialCopyAssign {
+  NonTrivialCopyAssign& operator=(const NonTrivialCopyAssign&);
+};
+static_assert(!__is_trivially_relocatable(NonTrivialCopyAssign));
+static_assert(!__is_trivially_relocatable(Agg<NonTrivialCopyAssign>));
+static_assert(!__is_trivially_relocatable(Der<NonTrivialCopyAssign>));
+static_assert(!__is_trivially_relocatable(Mut<NonTrivialCopyAssign>));
+static_assert(!__is_trivially_relocatable(Non<NonTrivialCopyAssign>));
+
+struct NonTrivialMutableCopyAssign {
+  NonTrivialMutableCopyAssign& operator=(NonTrivialMutableCopyAssign&);
+};
+static_assert(!__is_trivially_relocatable(NonTrivialMutableCopyAssign));
+static_assert(!__is_trivially_relocatable(Agg<NonTrivialMutableCopyAssign>));
+static_assert(!__is_trivially_relocatable(Der<NonTrivialMutableCopyAssign>));
+static_assert(!__is_trivially_relocatable(Mut<NonTrivialMutableCopyAssign>));
+static_assert(!__is_trivially_relocatable(Non<NonTrivialMutableCopyAssign>));
+
+#if __cplusplus >= 201103L
+struct NonTrivialMoveAssign {
+  NonTrivialMoveAssign& operator=(NonTrivialMoveAssign&&);
+};
+static_assert(!__is_trivially_relocatable(NonTrivialMoveAssign));
+static_assert(!__is_trivially_relocatable(Agg<NonTrivialMoveAssign>));
+static_assert(!__is_trivially_relocatable(Der<NonTrivialMoveAssign>));
+static_assert(!__is_trivially_relocatable(Mut<NonTrivialMoveAssign>));
+static_assert(!__is_trivially_relocatable(Non<NonTrivialMoveAssign>));
+#endif
diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp
index 5659594577111e..f425ec22608e6c 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -3104,18 +3104,43 @@ namespace is_trivially_relocatable {
 static_assert(!__is_trivially_relocatable(void), "");
 static_assert(__is_trivially_relocatable(int), "");
 static_assert(__is_trivially_relocatable(int[]), "");
+static_assert(__is_trivially_relocatable(const int), "");
+static_assert(__is_trivially_relocatable(volatile int), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<int>), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<int[2]>), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<const int>), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<const int[2]>), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<volatile int>), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<volatile int[2]>), "");
+static_assert(!__is_trivially_relocatable(int&), "");
+static_assert(!__is_trivially_relocatable(const int&), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<int&>), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<const int&>), "");
+
+static_assert(!__is_trivially_relocatable(Polymorph), "");
+static_assert(!__is_trivially_relocatable(InheritPolymorph), "");
+static_assert(!__is_trivially_relocatable(AggregateTemplate<Polymorph>), "");
+
+static_assert(__is_trivially_relocatable(HasPrivateBase), "");
+static_assert(__is_trivially_relocatable(HasProtectedBase), "");
+static_assert(!__is_trivially_relocatable(HasVirtBase), "");
 
 enum Enum {};
 static_assert(__is_trivially_relocatable(Enum), "");
 static_assert(__is_trivially_relocatable(Enum[]), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<Enum>), "");
 
 union Union {int x;};
 static_assert(__is_trivially_relocatable(Union), "");
 static_assert(__is_trivially_relocatable(Union[]), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<Union>), "");
 
 struct Trivial {};
 static_assert(__is_trivially_relocatable(Trivial), "");
 static_assert(__is_trivially_relocatable(Trivial[]), "");
+static_assert(__is_trivially_relocatable(const Trivial), "");
+static_assert(__is_trivially_relocatable(volatile Trivial), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<Trivial>), "");
 
 struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
 bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
@@ -3125,30 +3150,43 @@ struct NontrivialDtor {
 };
 static_assert(!__is_trivially_relocatable(NontrivialDtor), "");
 static_assert(!__is_trivially_relocatable(NontrivialDtor[]), "");
+static_assert(!__is_trivially_relocatable(const NontrivialDtor), "");
+static_assert(!__is_trivially_relocatable(volatile NontrivialDtor), "");
+static_assert(!__is_trivially_relocatable(AggregateTemplate<NontrivialDtor>), "");
 
 struct NontrivialCopyCtor {
   NontrivialCopyCtor(const NontrivialCopyCtor&) {}
 };
 static_assert(!__is_trivially_relocatable(NontrivialCopyCtor), "");
 static_assert(!__is_trivially_relocatable(NontrivialCopyCtor[]), "");
+static_assert(!__is_trivially_relocatable(AggregateTemplate<NontrivialCopyCtor>), "");
 
 struct NontrivialMoveCtor {
   NontrivialMoveCtor(NontrivialMoveCtor&&) {}
 };
 static_assert(!__is_trivially_relocatable(NontrivialMoveCtor), "");
 static_assert(!__is_trivially_relocatable(NontrivialMoveCtor[]), "");
+static_assert(!__is_trivially_relocatable(AggregateTemplate<NontrivialMoveCtor>), "");
 
 struct [[clang::trivial_abi]] TrivialAbiNontrivialDtor {
   ~TrivialAbiNontrivialDtor() {}
 };
 static_assert(__is_trivially_relocatable(TrivialAbiNontrivialDtor), "");
 static_assert(__is_trivially_relocatable(TrivialAbiNontrivialDtor[]), "");
+static_assert(__is_trivially_relocatable(const TrivialAbiNontrivialDtor), "");
+static_assert(__is_trivially_relocatable(volatile TrivialAbiNontrivialDtor), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<TrivialAbiNontrivialDtor>), "");
+static_assert(__is_trivially_relocatable(NonAggregateTemplate<TrivialAbiNontrivialDtor>), "");
 
 struct [[clang::trivial_abi]] TrivialAbiNontrivialCopyCtor {
   TrivialAbiNontrivialCopyCtor(const TrivialAbiNontrivialCopyCtor&) {}
 };
 static_assert(__is_trivially_relocatable(TrivialAbiNontrivialCopyCtor), "");
 static_assert(__is_trivially_relocatable(TrivialAbiNontrivialCopyCtor[]), "");
+static_assert(__is_trivially_relocatable(const TrivialAbiNontrivialCopyCtor), "");
+static_assert(__is_trivially_relocatable(volatile TrivialAbiNontrivialCopyCtor), "");
+static_assert(__is_trivially_relocatable(AggregateTemplate<TrivialAbiNontrivialCopyCtor>), "");
+static_assert(__is_trivially_relocatable(NonAggregateTemplate<TrivialAbiNontrivialCopyCtor>), "");
 
 // A more complete set of tests for the behavior of trivial_abi can be found in
 // clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -3157,6 +3195,36 @@ struct [[c...
[truncated]

@AMP999 AMP999 force-pushed the pr10-trivial-relocatability-as-a-type-trait branch from 191a98e to 7150230 Compare March 9, 2024 11:16
@AMP999 AMP999 force-pushed the pr10-trivial-relocatability-as-a-type-trait branch from 7150230 to 2603e17 Compare March 9, 2024 14:30
Quuxplusone added a commit to Quuxplusone/folly that referenced this pull request Mar 11, 2024
Clang 18's `__is_trivially_relocatable(T)` isn't yet compatible with Folly's
use of the term; false negatives are fine, but Clang dangerously gives
false positives. However, after llvm/llvm-project#84621 ,
the builtin will no longer give any (known) false positives, which means
it will be safe for Folly to start using the builtin as of Clang 19+.
To resolve llvm#69394, this patch separates trivial-relocatability's logic from `canPassInRegisters` to decide if a type is trivial-relocatable. A type passed in registers doesn't necessarily mean trivial-relocatability of that type(e.g. on Windows) i.e. it gives us an unintended false positive. This change would be beneficial for Abseil since they rely upon these semantics.
By these changes now:
User-provided special members prevent natural trivial-relocatabilitiy.
    It's important because Abseil and maybe others assume the assignment operator doesn't have an impact on the trivial-relocatability of a type.
    In fact, it does have an effect, and with a user-provided assignment operator, the compiler should only accept it as trivial-relocatable if it's implied by the `[[clang::trivial_abi]]` attribute.
Just because a type can pass in registers doesn't necessarily mean it's trivial-relocatable.
The `[[clang::trivial_abi]]` attribute always implies trivial-relocatability, even if it can't pass in registers.
The trait has extensive tests for both old and new behaviors. Test aggregation of
both kinds of types as data members; inheritance; virtual member functions
and virtual bases; const and reference data members; and reference types.

Fixes llvm#69394
@AMP999 AMP999 force-pushed the pr10-trivial-relocatability-as-a-type-trait branch from e01413f to a7ff71d Compare March 12, 2024 10:57
Copy link

github-actions bot commented Mar 12, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@AMP999 AMP999 force-pushed the pr10-trivial-relocatability-as-a-type-trait branch from 6e07a06 to da761ed Compare March 12, 2024 13:30
clang/lib/AST/DeclCXX.cpp Outdated Show resolved Hide resolved
Comment on lines 888 to 890
if (Method->isUserProvided() &&
(Method->isCopyAssignment() || Method->isMoveAssignment()))
data().IsNaturallyTriviallyRelocatable = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we stick to the 2786r4 definition, this is overly restrictive (ie assignment does not affect relocatability)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some solicited comments, from Qt maintainer.

I don't have a strong opinion on whether the presence of an assignment operator should affect relocatability. Only "stupid types" will have a trivial copy/move constructor and a non-trivial assignment operator that does something different. Those types are inherently broken, because a move can be implemented as either

   tgt = std::move(src);

or as

    std::destroy_at(&tgt);
    std::construct_at(&tgt, std::move(src));

(provided &tgt != &src)

It is the container implementer's choice of which those two to use and we have both being chosen in different frameworks, sometimes even both inside the same framework. Therefore, any type that has visibly different behaviour for the two implementations above is "stupid" and has bigger problems than its relocatability.

That means it's a reasonable choice to say the presence of the assignment operator does not affect the relocatability. But then it's also an equally valid choice to say it does, with the benefit that one could find out that a type is "stupid" before a release.

I do have a weak "leans to" choice though: for the first implementation of trivial relocatability, we should err on the side of being more restrictive. We can always be less so in the future, but going the other direction is more problematic.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also been asked to comment. My personal opinion is that relocability is strictly tied to the Rule Of Five. The moment a user decides to opt into the RO5 by e.g. writing a custom assignment operator, they should lose trivial relocability, unless they do something extra to gain it back (different proposals disagree on the syntax but ultimately they achieve the same). (We may then argue regarding all the subtleties about user-declared vs. user-defaulted vs. user-defined, but I don't really have a strong opinion about that.)

Copy link
Contributor

@cor3ntin cor3ntin Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern here is to make sure we are consistent with WG21.
EWG approved a design that does not consider assignment, and Clang should follow suite (ie we should avoid needing 2 type traits, one that is standard conforming, and one that isn't).

Either way we have some time to adapt that, as this paper should (finger crossed) be approved in june, before we need to ship clang 18.

I generally agree with @thiagomacieira (Hi!) that this is mostly a question for "stupid" types, but I think the good mental model is that relocation is an optimization of copy. IE a type that does not follow the the rule of 5 is "stupid" whether it's relocatable or not.

So my advice here is do as P2786 does, and revisit should that paper fail to be adopted before the next clang release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only "stupid types" will have a trivial copy/move constructor and a non-trivial assignment operator that does something different.

One such type is std::tuple<int&>. As far as I can tell, the way the different papers handle this type is:

  • P1144: not trivially relocatable (because the copy/move assignment operator is user-provided)
  • P2786: trivially relocatable (because we only care about move construction, and that one is defaulted)

I don't actually know what goes wrong (if anything) if you make tuple<int&> trivially relocatable, but at least that's a nice concrete example to think about.

/// True when this class's bases and fields are all trivially relocatable
/// or references, and the class itself has no user-provided special
/// member functions.
FIELD(IsNaturallyTriviallyRelocatable, 1, NO_MERGE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you are trying to distinguish from the abi_tag but naturally is not saying much.

Either IsTriviallyRelocatable or, IsImplicitlyTriviallyRelocatable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still like to see a rename here

Comment on lines 845 to 852

// A trivially relocatable class is a class:
// -- where no eligible copy constructor, move constructor, copy
// assignment operator, move assignment operator, or destructor is
// user-provided,
if (Constructor->isUserProvided() && (Constructor->isCopyConstructor() ||
Constructor->isMoveConstructor()))
data().IsNaturallyTriviallyRelocatable = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you get that definition from?
In P2786R4, C is trivial if C(someC()) does not invoke a user-defined constructor, which is more corect vis a vis constraints. (So that would have to be done from outside the class, Maybe in Sema)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a user-defined copy constructor, then it's not trivially copyable anymore. Same goes for moves. So P1144 is more in line with what the standard has chosen in the past. IMO it's better to keep the same principle and not hide things to the developer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider, for example

template <typename T>
struct S {
    S(const S&) = default;
    S(const S&) requires false = delete;
};

https://compiler-explorer.com/z/5s5rja6d3

Quuxplusone added a commit to Quuxplusone/folly that referenced this pull request Mar 12, 2024
Clang 18's `__is_trivially_relocatable(T)` isn't yet compatible with Folly's
use of the term; false negatives are fine, but Clang dangerously gives
false positives. However, after llvm/llvm-project#84621 ,
the builtin will no longer give any (known) false positives, which means
it will be safe for Folly to start using the builtin as of Clang 19+.
Copy link
Member

@sdkrystian sdkrystian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit, but otherwise looks good.

@alandefreitas
Copy link

My Boost libraries would benefit a lot from this change. 🙂

@@ -2680,8 +2680,8 @@ bool QualType::isTriviallyRelocatableType(const ASTContext &Context) const {
return false;
} else if (!BaseElementType->isObjectType()) {
return false;
} else if (const auto *RD = BaseElementType->getAsRecordDecl()) {
return RD->canPassInRegisters();
} else if (CXXRecordDecl *RD = BaseElementType->getAsCXXRecordDecl()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The const auto * was more our coding style.

@DanielLiamAnderson
Copy link

This would be really useful for ParlayLib. We implement our own is_trivially_relocatable which is not nearly as reliable as having the compiler do it. I support this PR's semantics; types with non-defaulted assignment operators should report false, not true.

@sjanel
Copy link

sjanel commented Mar 18, 2024

The amc library defining drop-in replacement for vector-like containers handling trivially relocatability optimizations would love to see this kind of improvement, as currently the users need to manually tell the compiler which types are OK to consider as trivially relocatable. I would be glad to test it!

@isidorostsa
Copy link

Currently, HPX implements the P1144 interface. We have our own implementation of the is_trivially_relocatable trait, but it cannot recursively figure out if a type is relocatable based on its components. It would be great for us to let the compiler handle that, as it would lead to optimizations for many more types.

@danakj
Copy link
Contributor

danakj commented Mar 22, 2024

The subspace library also provides mechanisms for querying and marking types trivially relocatable, and relies on the compiler's __is_trivially_relocatable as a signal as well. It then optimizes containers and operations internally based on its answer, and allows client code to do the same. Getting __is_trivially_relocatable correct here would be very appreciated.

@arturbac
Copy link

arturbac commented Mar 27, 2024

My small_vectors are waiting for that feature for a long time, already have trait defined but cant implement and I've TODO in code.

@AMP999
Copy link
Contributor Author

AMP999 commented Apr 4, 2024

The semantics in this PR are the semantics relied upon by the libraries whose maintainers have commented in support of this PR. These semantics have been codified into P1144; that's the paper I'm quoting in the code comments. @cor3ntin could you clarify what you mean by "do as P2786 does" and "stupid types"? Are you talking about types like pair<K&, V&>? If that kind of "stupid type" is reported as trivially relocatable, then fbvector and q_rotate would mis-compile operations on it, so Folly and Qt would remain unable to use Clang's builtin.

@cor3ntin
Copy link
Contributor

cor3ntin commented Apr 4, 2024

The semantics in this PR are the semantics relied upon by the libraries whose maintainers have commented in support of this PR. These semantics have been codified into P1144; that's the paper I'm quoting in the code comments.

P1144 is not the direction the committee is taking though (cplusplus/papers#1463 (comment))

@cor3ntin could you clarify what you mean by "do as P2786 does" and "stupid types"? Are you talking about types like pair<K&, V&>? If that kind of "stupid type" is reported as trivially relocatable, then fbvector and q_rotate would mis-compile operations on it

Can you expand on that? Are you talking about the issues described here? https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2959r0.html

, so Folly and Qt would remain unable to use Clang's builtin.

I don't think this accurate, If a library requires additional constraint, they can add them (in addition to checking the type trait), whereas if the type trait is too constrained for some use cases, it would indeed not be usable

@cor3ntin
Copy link
Contributor

cor3ntin commented Apr 5, 2024

The semantics in this PR are the semantics relied upon by the libraries whose maintainers have commented in support of this PR. These semantics have been codified into P1144; that's the paper I'm quoting in the code comments. @cor3ntin could you clarify what you mean by "do as P2786 does" and "stupid types"? Are you talking about types like pair<K&, V&>? If that kind of "stupid type" is reported as trivially relocatable, then fbvector and q_rotate would mis-compile operations on it, so Folly and Qt would remain unable to use Clang's builtin.

I apologize, this wasn't a useful/constructive description. I meant types that are copyable and not assignable, which as several people pointed out, does indeed include all sort of reference wrapper. These types are still copyable and any restriction to make them not relocatable would be extremely artificial (and an additional requirements can be put on a library wrapper to achieve that design if anyone wanted it)

@AaronBallman
Copy link
Collaborator

Based on the poll results from Tokyo, P2786 is the direction EWG has taken for this functionality, so we should be focusing our efforts on supporting that direction. If WG21 changes direction again, we can shift again, (this builtin exists to support the standard's type trait and thus is experimental until the standard makes a final decision). I don't think we should put efforts into supporting a P1144-based approach.

@dangelog
Copy link

dangelog commented Apr 6, 2024

I perfectly understand that Clang wants to implement the semantics that are being adopted by standard C++, so it wants to reserve __is_trivially_relocatable to match those semantics.

The amount of comments in this PR is however a strong signal that standard C++ is going directly against widely established practices. Qt won't need P2786 but P1144, because it uses trivial relocation not only ① for vector reallocation (that is: move construction + destruction via trivial relocation) but also ② for vector erase/insert (that is: move assignment (+destruction) via trivial relocation). It would not make sense for a type with a user-declared assignment operator to be considered TR for Qt.

Upstream C++ seems to be fine with just providing ① (via P2786), but that leaves a huge number of optimizations on the plate: vector insert, erase, swaps and swap-based algorithms, etc.; which is a total shame.

I perfectly understand that some types (like the aforementioned "reference wrappers", pmr::string, etc.) only allow ① semantics. If upstream cares about them so much and doesn't want to lose the opportunity of optimizing vector reallocation for them, why haven't they landed two type traits? A generic "is trivial relocatable" (that captures construction and assignment) and a more specific one (that only captures construction; a subset of the first). It would make perfect sense to me to capture different semantics via different traits. Not only that, but P2786 has squatted the "trivial relocatable" name for the trait, which really belongs to the more general trait.

Oh well, sorry for the rant :-)

I don't think this accurate, If a library requires additional constraint, they can add them (in addition to checking the type trait),

But this isn't useful at all; every library has already invented its own constraints and type traits. The point of having a language-based solution is that the "is TR" property gets automatically propagated when you compose types. No library solution can achieve that, especially lacking reflection.

I don't think we should put efforts into supporting a P1144-based approach.

Do you mean that you would reject a proposal that adds Clang-specific trait (with another name) that implements P1144's semantics?

@cor3ntin
Copy link
Contributor

cor3ntin commented Apr 6, 2024

But this isn't useful at all; every library has already invented its own constraints and type traits. The point of having a language-based solution is that the "is TR" property gets automatically propagated when you compose types. No library solution can achieve that, especially lacking reflection.

I am not sure i understand, assignability is already a transitive property for defaults operators? so the scenario you are concerned about is triviality copyable types with a user defined assignment operators?

This would for example not impact reference wrappers which are not assignable.

The larger point is that there are 2 concerns here, one is whether it is safe to relocate a type at the language level, which this type trait is intended to provide, and one is how a library uses relocatability for optimization.

A library may decide it doesn't want to relocate types that are not assignable for some operations (which btw doesn't need to apply recursively to subobjects) , and it can do so today.

I would in fact imagine that the logic would be 1/checking for assignability in all cases 2/checking for relocatability for optimization.

It would be useful to illustrate with code examples scenarios in which that wouldn't work.

And again, https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2959r0.html goes into some of these questions and might be worth checking out.

@cor3ntin
Copy link
Contributor

cor3ntin commented Apr 6, 2024

Do you mean that you would reject a proposal that adds Clang-specific trait (with another name) that implements P1144's semantics?

A proposal that explores trivial swapability for example, with the understanding it would be a subset of trivially relocatable types might be worth exploring as a separate feature. Hopefully this would go through careful design and RFC.

@AaronBallman
Copy link
Collaborator

Do you mean that you would reject a proposal that adds Clang-specific trait (with another name) that implements P1144's semantics?

So long as it checks all the boxes in our usual criteria for extensions, I'd see no reason to oppose additional type traits. However, the push back right now comes from point #4: the idea was proposed to a standards body and that idea was rejected. So long as whatever new idea comes along has some chance of standardization, then it would be reasonable to add. If the trait was also supported in GCC, that would also help build a case for including it in Clang.

@rnk
Copy link
Collaborator

rnk commented Apr 16, 2024

So, I was completely unaware that trivial relocatability had been picked up at all by WG21. Since the beginning of trivial_abi, I we were solidly in the vendor-extension space trying to build non-standard but practical solutions to real world problems, like the fact that we couldn't pass unique_ptr in registers. We (users & implementers) have all collectively identified trivial relocatability as an important optimization for vector-like containers, and we're all looking for a solution to that. It feels like this attempt to standardize this type trait is getting in the way of our collective ability to add vendor extensions that were not necessarily intended to become standards-track proposals.

@AaronBallman
Copy link
Collaborator

So, I was completely unaware that trivial relocatability had been picked up at all by WG21. Since the beginning of trivial_abi, I we were solidly in the vendor-extension space trying to build non-standard but practical solutions to real world problems, like the fact that we couldn't pass unique_ptr in registers. We (users & implementers) have all collectively identified trivial relocatability as an important optimization for vector-like containers, and we're all looking for a solution to that. It feels like this attempt to standardize this type trait is getting in the way of our collective ability to add vendor extensions that were not necessarily intended to become standards-track proposals.

The primary purpose of our documented requirement to put extensions like this in front of a standards body is because we trust the standards organizations to be more thorough about the features they add than we as implementers typically achieve. So this isn't the standardization process getting in the way of us adding extensions, it's our process being followed correctly. Our type traits are intended as the underlying implementation to be used for the standard library, so changing __is_trivially_relocatable to not follow what WG21 is doing is not the approach we should take.

I don't think anyone is arguing against solving problems the standards body isn't solving with their solution. It's more that we're saying to start from first principles with a new feature that's not the one being standardized by WG21 (pick a new name, address issues raised by WG21 discussions, post an RFC describing why this is beneficial/necessary, etc). However, because WG21 has not finalized the feature, we should be careful before adding a new type trait to the compiler because WG21 does change their opinion occasionally. @rnk, Google is still a member of WG21, so perhaps it would be worth attending the upcoming meeting in St Louis to advocate for the design approach you prefer (if successful, then we don't need to add a secondary trait at all).

@QrczakMK
Copy link

I think that if Clang chooses to support both variants of the type trait, it should provide both variants of [[clang::trivial_abi]], so that a type like std::unique_ptr<T> is trivially relocatable including assignment (and can be annotated as such), while a type like std::tuple<int&> is trivially relocatable only for construction (and can be annotated as such if its move constructor cannot be trivial for some reason).

Riegeli would benefit from the type trait, with no difference in how it treats types with non-trivial or not-relocating or missing assignment operator.

It provides a type-erasing holder called AnyDependency. When not requested to reserve inline storage, it stores a type inline if it fits in a pointer and is trivially relocatable, otherwise allocates it on the heap. This constraint is in turn used to optimize move constructor and assignment to skip a call through a function pointer, as well as to mark AnyDependency itself trivially relocatable (even though it is not trivially movable: it marks the moved-from object as empty).

The optimization of storing the type inline should be applicable not only to trivially copyable types but also to std::unique_ptr (if tagged with __attribute__((trivial_abi))) and to types with std::unique_ptr as a sole member.

AnyDependency does not use the assignment operator of the stored type (assigning a new stored value always destroys and recreates the stored value, because its type may change and it is not worth the effort to detect when it happens to not change). It is rarely used with types which are move constructible but not move assignable (or with an atypical semantics of assignment). Hence either variant of the semantics of the type trait would work equally well here.

@AMP999
Copy link
Contributor Author

AMP999 commented Apr 16, 2024

We (users & implementers) have all collectively identified trivial relocatability as an important optimization for vector-like containers, and we're all looking for a solution to that. It feels like this attempt to standardize this type trait is getting in the way of our collective ability to add vendor extensions that were not necessarily intended to become standards-track proposals.

Since people are asking for these semantics in Clang's vendor extension, how would people feel about proceeding in this direction for now, and adding a second type trait, named something like __is_cpp_trivially_relocatable(T), for the P2786 version?

@AaronBallman
Copy link
Collaborator

AaronBallman commented Apr 16, 2024

We (users & implementers) have all collectively identified trivial relocatability as an important optimization for vector-like containers, and we're all looking for a solution to that. It feels like this attempt to standardize this type trait is getting in the way of our collective ability to add vendor extensions that were not necessarily intended to become standards-track proposals.

Since people are asking for these semantics in Clang's vendor extension, how would people feel about proceeding in this direction for now, and adding a second type trait, named something like __is_cpp_trivially_relocatable(T), for the P2786 version?

Our naming convention is generally __is_xxx mapping to std::is_xxx, so this trait should not be changed to have different semantics from what C++ uses for the type trait. Until WG21 finalizes the discussion of this type trait, I do not think we should do anything to __is_trivially_relocatable and it's not clear we'll need a new type trait. I think this PR is in a holding pattern until we know what the standard solution will be -- once we have that information, we can make an informed choice as to how to move forward. (If the committee drops the ball, we can cross that bridge if we come to it.)

facebook-github-bot pushed a commit to facebook/folly that referenced this pull request Jun 17, 2024
)

Summary:
Well, llvm/llvm-project#84621 isn't going anywhere in Clang, therefore #2159 will presumably never be mergeable (in the foreseeable future, anyway).

So here's an alternative approach, for your consideration: If [P1144's feature-test macro](https://open-std.org/jtc1/sc22/wg21/docs/papers/2024/p1144r10.html#wording) is set, then use `std::is_trivially_relocatable<T>` in place of `std::is_trivially_copyable<T>`.
This will produce improved codegen for e.g. `fbvector<std::vector<int>>::erase`, when compiled on a compiler supporting P1144 semantics. https://godbolt.org/z/s6sa313h3

Compare https://github.com/charles-salvia/std_error/blob/3abc272/all_in_one.hpp#L180-L196

Pull Request resolved: #2216

Reviewed By: Gownta

Differential Revision: D58147899

Pulled By: Orvid

fbshipit-source-id: 671ed21eeb6080f583e6966e776d243c99c9c17f
facebook-github-bot pushed a commit to facebook/hhvm that referenced this pull request Jun 17, 2024
Summary:
Well, llvm/llvm-project#84621 isn't going anywhere in Clang, therefore facebook/folly#2159 will presumably never be mergeable (in the foreseeable future, anyway).

So here's an alternative approach, for your consideration: If [P1144's feature-test macro](https://open-std.org/jtc1/sc22/wg21/docs/papers/2024/p1144r10.html#wording) is set, then use `std::is_trivially_relocatable<T>` in place of `std::is_trivially_copyable<T>`.
This will produce improved codegen for e.g. `fbvector<std::vector<int>>::erase`, when compiled on a compiler supporting P1144 semantics. https://godbolt.org/z/s6sa313h3

Compare https://github.com/charles-salvia/std_error/blob/3abc272/all_in_one.hpp#L180-L196

X-link: facebook/folly#2216

Reviewed By: Gownta

Differential Revision: D58147899

Pulled By: Orvid

fbshipit-source-id: 671ed21eeb6080f583e6966e776d243c99c9c17f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang] __is_trivially_relocatable isn't correct on Windows