address more feedback

Author: cor3ntin

clang/docs/LanguageExtensions.rst

@@ -1828,11 +1828,11 @@ The following type trait primitives are supported by Clang. Those traits marked
   source object on the floor. This is true of trivial types and types which
   were made trivially relocatable via the ``clang::trivial_abi`` attribute.
 * ``__builtin_is_cpp_trivially_relocatable`` (C++): Returns true if an object
-  is trivially relocatable, as defined by the C++26 standard.
+  is trivially relocatable, as defined by the C++26 standard [meta.unary.prop].
   Note that the caller code should ensure that if the object is polymorphic,
   the dynamic type is of the most derived type.
 * ``__builtin_is_replaceable`` (C++): Returns true if an object
-  is replaceable, as defined by the C++26 standard.
+  is replaceable, as defined by the C++26 standard [meta.unary.prop].
 * ``__is_trivially_equality_comparable`` (Clang): Returns true if comparing two
   objects of the provided type is known to be equivalent to comparing their
   object representations. Note that types containing padding bytes are never

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -1058,7 +1058,7 @@ def ext_ms_abstract_keyword : ExtWarn<
   InGroup<MicrosoftAbstract>;
 
 def ext_relocatable_keyword : ExtWarn<
-  "'%select{trivially_relocatable|replaceable}0_if_eligible' "
+  "'%select{trivially_relocatable_if_eligible|replaceable_if_eligible}0' "
   "keyword is a C++2c extension">,
   InGroup<CXX26>;
 def warn_relocatable_keyword  : Warning<

clang/include/clang/Parse/Parser.h

@@ -3174,12 +3174,12 @@ class Parser : public CodeCompletionHandler {
 
   bool isCXX2CTriviallyRelocatableKeyword(Token Tok) const;
   bool isCXX2CTriviallyRelocatableKeyword() const;
-  void ParseOptionalCXX2CTriviallyRelocatableSpecifier(
-      TriviallyRelocatableSpecifier &TRS);
+  void
+  ParseCXX2CTriviallyRelocatableSpecifier(TriviallyRelocatableSpecifier &TRS);
 
   bool isCXX2CReplaceableKeyword(Token Tok) const;
   bool isCXX2CReplaceableKeyword() const;
-  void ParseOptionalCXX2CReplaceableSpecifier(ReplaceableSpecifier &MRS);
+  void ParseCXX2CReplaceableSpecifier(ReplaceableSpecifier &MRS);
 
   bool isClassCompatibleKeyword(Token Tok) const;
   bool isClassCompatibleKeyword() const;

clang/lib/Parse/ParseDeclCXX.cpp

@@ -2707,7 +2707,7 @@ bool Parser::isCXX2CTriviallyRelocatableKeyword() const {
   return isCXX2CTriviallyRelocatableKeyword(Tok);
 }
 
-void Parser::ParseOptionalCXX2CTriviallyRelocatableSpecifier(
+void Parser::ParseCXX2CTriviallyRelocatableSpecifier(
     TriviallyRelocatableSpecifier &TRS) {
   assert(isCXX2CTriviallyRelocatableKeyword() &&
          "expected a trivially_relocatable specifier");
@@ -2734,7 +2734,7 @@ bool Parser::isCXX2CReplaceableKeyword() const {
   return isCXX2CReplaceableKeyword(Tok);
 }
 
-void Parser::ParseOptionalCXX2CReplaceableSpecifier(ReplaceableSpecifier &MRS) {
+void Parser::ParseCXX2CReplaceableSpecifier(ReplaceableSpecifier &MRS) {
   assert(isCXX2CReplaceableKeyword() &&
          "expected a replaceable_if_eligible specifier");
 
@@ -3881,20 +3881,20 @@ void Parser::ParseCXXMemberSpecification(SourceLocation RecordLoc,
             auto Skipped = Tok;
             ConsumeToken();
             Diag(Skipped, diag::err_duplicate_class_relocation_specifier)
-                << 0 << TriviallyRelocatable.getLocation();
+                << /*trivial_relocatable*/ 0
+                << TriviallyRelocatable.getLocation();
           } else {
-            ParseOptionalCXX2CTriviallyRelocatableSpecifier(
-                TriviallyRelocatable);
+            ParseCXX2CTriviallyRelocatableSpecifier(TriviallyRelocatable);
           }
           continue;
         } else if (isCXX2CReplaceableKeyword(Tok)) {
           if (Replacable.isSet()) {
             auto Skipped = Tok;
             ConsumeToken();
             Diag(Skipped, diag::err_duplicate_class_relocation_specifier)
-                << 1 << Replacable.getLocation();
+                << /*replaceable*/ 1 << Replacable.getLocation();
           } else {
-            ParseOptionalCXX2CReplaceableSpecifier(Replacable);
+            ParseCXX2CReplaceableSpecifier(Replacable);
           }
           continue;
         } else {

clang/lib/Sema/SemaChecking.cpp

@@ -1896,7 +1896,8 @@ static ExprResult BuiltinTriviallyRelocate(Sema &S, CallExpr *TheCall) {
     return ExprError();
 
   if (T.isConstQualified() ||
-      !T.isCppTriviallyRelocatableType(S.getASTContext())) {
+      !T.isCppTriviallyRelocatableType(S.getASTContext()) ||
+      T->isIncompleteArrayType()) {
     S.Diag(TheCall->getArg(0)->getExprLoc(),
            diag::err_builtin_trivially_relocate_invalid_arg_type)
         << (T.isConstQualified() ? /*non-const*/ 1 : /*relocatable*/ 2);

clang/test/SemaCXX/cxx2c-trivially-relocatable.cpp

@@ -1,55 +1,79 @@
 // RUN: %clang_cc1 -std=c++2c -verify %s
 
 class Trivial {};
+static_assert(__builtin_is_cpp_trivially_relocatable(Trivial));
 struct NonRelocatable {
     ~NonRelocatable();
 };
 static NonRelocatable NonRelocatable_g;
 
 class A trivially_relocatable_if_eligible {};
+static_assert(__builtin_is_cpp_trivially_relocatable(A));
+
+
 class B trivially_relocatable_if_eligible : Trivial{};
+static_assert(__builtin_is_cpp_trivially_relocatable(B));
+
 class C trivially_relocatable_if_eligible {
     int a;
     void* b;
     int c[3];
     Trivial d[3];
     NonRelocatable& e = NonRelocatable_g;
 };
+static_assert(__builtin_is_cpp_trivially_relocatable(C));
+
+
 class D trivially_relocatable_if_eligible : Trivial {};
+static_assert(__builtin_is_cpp_trivially_relocatable(D));
+
+
 class E trivially_relocatable_if_eligible : virtual Trivial {};
+static_assert(!__builtin_is_cpp_trivially_relocatable(E));
+
 
 class F trivially_relocatable_if_eligible : NonRelocatable {};
+static_assert(!__builtin_is_cpp_trivially_relocatable(F));
+
+class G trivially_relocatable_if_eligible {
+    G(G&&);
+};
+static_assert(__builtin_is_cpp_trivially_relocatable(G));
+
+class H trivially_relocatable_if_eligible {
+    ~H();
+};
+static_assert(__builtin_is_cpp_trivially_relocatable(H));
 
 class I trivially_relocatable_if_eligible {
     NonRelocatable a;
     NonRelocatable b[1];
     const NonRelocatable c;
     const NonRelocatable d[1];
 };
+static_assert(!__builtin_is_cpp_trivially_relocatable(I));
+
 
 class J trivially_relocatable_if_eligible:  virtual Trivial, NonRelocatable {
     NonRelocatable a;
 };
+static_assert(!__builtin_is_cpp_trivially_relocatable(J));
 
-class G trivially_relocatable_if_eligible {
-    G(G&&);
-};
-
-class H trivially_relocatable_if_eligible {
-    ~H();
-};
 
 struct Incomplete; // expected-note {{forward declaration of 'Incomplete'}}
 static_assert(__builtin_is_cpp_trivially_relocatable(Incomplete));  // expected-error {{incomplete type 'Incomplete' used in type trait expression}}
-static_assert(__builtin_is_cpp_trivially_relocatable(Trivial));
-static_assert(__builtin_is_cpp_trivially_relocatable(G));
-static_assert(__builtin_is_cpp_trivially_relocatable(H));
 static_assert(__builtin_is_cpp_trivially_relocatable(int));
 static_assert(__builtin_is_cpp_trivially_relocatable(void*));
 static_assert(!__builtin_is_cpp_trivially_relocatable(int&));
 static_assert(!__builtin_is_cpp_trivially_relocatable(Trivial&));
 static_assert(__builtin_is_cpp_trivially_relocatable(const Trivial));
 static_assert(__builtin_is_cpp_trivially_relocatable(Trivial[1]));
+static_assert(__builtin_is_cpp_trivially_relocatable(Trivial[]));
+
+struct WithConst {
+    const int i;
+};
+static_assert(__builtin_is_cpp_trivially_relocatable(WithConst));
 
 struct UserDtr {
     ~UserDtr();
@@ -115,9 +139,15 @@ struct DeletedMoveAssign {
     DeletedMoveAssign& operator=(DeletedMoveAssign&&) = delete;
 };
 
+struct DeletedDtr {
+    ~DeletedDtr() = delete;
+};
+
 static_assert(!__builtin_is_cpp_trivially_relocatable(DeletedMove));
 static_assert(!__builtin_is_cpp_trivially_relocatable(DeletedCopy));
 static_assert(!__builtin_is_cpp_trivially_relocatable(DeletedMoveAssign));
+static_assert(!__builtin_is_cpp_trivially_relocatable(DeletedDtr));
+
 
 union U {
     G g;
@@ -174,6 +204,7 @@ static_assert(__builtin_is_replaceable(DefaultedMoveAssign));
 static_assert(!__builtin_is_replaceable(DeletedMove));
 static_assert(!__builtin_is_replaceable(DeletedCopy));
 static_assert(!__builtin_is_replaceable(DeletedMoveAssign));
+static_assert(!__builtin_is_replaceable(DeletedDtr));
 
 static_assert(!__builtin_is_replaceable(UserProvidedMove));
 static_assert(!__builtin_is_replaceable(UserProvidedCopy));
@@ -276,3 +307,16 @@ void test__builtin_trivially_relocate() {
     __builtin_trivially_relocate((int*)0, (int*)0, 0);
     __builtin_trivially_relocate((R*)0, (R*)0, 0);
 }
+
+void test__builtin_trivially_relocate(auto&& src, auto&&dest, auto size) {
+    __builtin_trivially_relocate(src, dest, size); // #reloc1
+}
+
+void do_test__builtin_trivially_relocate() {
+    struct S{ ~S();};
+    struct R {};
+    test__builtin_trivially_relocate((R*)0, (R*)0, 0);
+    test__builtin_trivially_relocate((S*)0, (S*)0, 0);
+    // expected-note@-1 {{'test__builtin_trivially_relocate<S *, S *, int>' requested here}}
+    // expected-error@#reloc1 {{first argument to '__builtin_trivially_relocate' must be relocatable}}
+}