[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

Mon Jun 13 12:41:24 PDT 2022

Javier-varez added a comment.

Thanks for the review! Indeed this looks like it implements the defect report dr2171. I didn't realize there was a defect report about this, thanks for bringing it up. I am not sure If this behavior should also change for move constructors, but clang currently cannot declare defaulted move constructors with arguments other than `T&&` (so basically they are not trivial anyway because they are simply not allowed to be defaulted) and it seems GCC also does not allow it. In my opinion that should be fine as is, given `const T&&` is not quite sensible.

I updated the change with your suggestions. I used ClangABICompat 14 because 15 is unreleased, so I keep the behavior to preserve the ABI if `-fclang-abi-compat` is called with argument 14 or less.

I also didn't manage to find out why the openmp tests are failing, I'll need to have another look at that.

================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:9786-9789
+    if (RT && ((RT->getPointeeType().getCVRQualifiers() & Qualifiers::Const) ==
+               Qualifiers::Const)) {
+      ConstArg = true;
+    }
----------------
royjacobson wrote:
> I think this should work instead? No need to check for RT since we already returned if !RT.
Ah, good point, lazy refactoring on my side!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593