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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

This is great, thanks for the changes! I added two follow-up inline comments.

Another two other small details: Please add documentation about this to LangOptions.h (We try to document there the changes for each ABI version), and please add a release note about this change (in clang/docs/ReleaseNotes.rst). The "ABI Changes in Clang" section seems appropriate.

Do you have LLVM commit access? If not, I can commit it for you - let me know the name+email you want the commit to be under in that case.



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:9778
+
+    const bool ClangABICompat14 = Context.getLangOpts().getClangABICompat() <=
+                                  LangOptions::ClangABI::Ver14;
----------------
Please add a comment here about the defect report and what the ClangABICompat14 is about.


================
Comment at: clang/test/CXX/special/class.copy/p12-0x.cpp:31
 };
-using _ = not_trivially_copyable<NonConstCopy>;
 
----------------
Could you add a test that we use the previous behavior (for this and for trivially assignable) when clang-abi-compat=14 is set? Either a new test specifically for this behavior, or you can use the `CLANG_ABI_COMPAT` macro and run the same test with the flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593