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

Thiago Macieira via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 12 15:11:04 PDT 2024


================
@@ -857,8 +881,13 @@ void CXXRecordDecl::addedMember(Decl *D) {
         data().HasDeclaredCopyAssignmentWithConstParam = true;
     }
 
-    if (Method->isMoveAssignmentOperator())
+    if (Method->isMoveAssignmentOperator()) {
       SMKind |= SMF_MoveAssignment;
+    }
+
+    if (Method->isUserProvided() &&
+        (Method->isCopyAssignment() || Method->isMoveAssignment()))
+      data().IsNaturallyTriviallyRelocatable = false;
----------------
thiagomacieira wrote:

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
```c++
   tgt = std::move(src);
```
or as
```c++
    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.

https://github.com/llvm/llvm-project/pull/84621


More information about the cfe-commits mailing list