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

Sun Jun 12 12:05:04 PDT 2022

royjacobson requested changes to this revision.
royjacobson added a reviewer: clang-language-wg.
royjacobson added a comment.
This revision now requires changes to proceed.

Hi Javier, thank you for submitting this patch!

As far as I could tell, this patch implements the CWG2171 defect report from 2016: https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2171. That means that you'll have to add a test inside clang/test/CXX/drs/dr21xx.cpp to make sure it shows on the DRs status page.

This change is also a large ABI break, which means it has to be considered carefully. I think the best approach here would be to revert to the previous behavior when the -fclang-abi-compat flag is used for clang <= 14, but I'm not sure about this. I would like the approval of someone with more ABI experience here.

The code itself and the tests look good! If you'll need help with the changes I mentioned please let me know.

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593