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

2022-06-12 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez created this revision.
Herald added a project: All.
Javier-varez requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

>From [class.copy.ctor]:

  A non-template constructor for class X is a copy constructor if its first
  parameter is of type X&, const X&, volatile X& or const volatile X&, and
  either there are no other parameters or else all other parameters have
  default arguments (9.3.4.7).
  
  A copy/move constructor for class X is trivial if it is not user-provided and 
if:
  - class X has no virtual functions (11.7.3) and no virtual base classes 
(11.7.2), and
  - the constructor selected to copy/move each direct base class subobject is 
trivial, and
  - or each non-static data member of X that is of class type (or array 
thereof),
the constructor selected to copy/move that member is trivial;
  
  otherwise the copy/move constructor is non-trivial.

So `T(T&) = default`; should be trivial assuming that the previous
provisions are met.

This works in GCC, but not in Clang at the moment:
https://godbolt.org/z/fTGe71b6P


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127593

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/special/class.copy/p12-0x.cpp
  clang/test/CXX/special/class.copy/p25-0x.cpp


Index: clang/test/CXX/special/class.copy/p25-0x.cpp
===
--- clang/test/CXX/special/class.copy/p25-0x.cpp
+++ clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -31,7 +31,22 @@
 struct NonConstCopy {
   NonConstCopy &operator=(NonConstCopy &) = default;
 };
-using _ = not_trivially_assignable;
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy&, NonConstCopy&), "");
+static_assert(!__is_trivially_assignable(NonConstCopy&, const NonConstCopy &), 
"");
+static_assert(!__is_trivially_assignable(NonConstCopy&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy&, NonConstCopy&&), "");
+static_assert(__is_trivially_assignable(NonConstCopy&&, NonConstCopy&), "");
+static_assert(!__is_trivially_assignable(NonConstCopy&&, const NonConstCopy 
&), "");
+static_assert(!__is_trivially_assignable(NonConstCopy&&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy&&, NonConstCopy&&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers &operator=(const DefaultedSpecialMembers &) = 
default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_assignable;
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/special/class.copy/p12-0x.cpp
===
--- clang/test/CXX/special/class.copy/p12-0x.cpp
+++ clang/test/CXX/special/class.copy/p12-0x.cpp
@@ -28,7 +28,18 @@
 struct NonConstCopy {
   NonConstCopy(NonConstCopy &) = default;
 };
-using _ = not_trivially_copyable;
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy&), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy 
&), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), 
"");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers(const DefaultedSpecialMembers&) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_copyable;
 
 // class X has no virtual functions
 struct VFn {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -9772,11 +9772,9 @@
 
   case CXXCopyConstructor:
   case CXXCopyAssignment: {
-// Trivial copy operations always have const, non-volatile parameter types.
-ConstArg = true;
 const ParmVarDecl *Param0 = MD->getParamDecl(0);
 const ReferenceType *RT = Param0->getType()->getAs();
-if (!RT || RT->getPointeeType().getCVRQualifiers() != Qualifiers::Const) {
+if (!RT) {
   if (Diagnose)
 Diag(Param0->getLocation(), diag::note_nontrivial_param_type)
   << Param0->getSourceRange() << Param0->getType()
@@ -9784,6 +9782,10 @@
Context.getRecordType(RD).withConst());
   return false;
 }
+
+if (RT && ((RT->getPointeeType().getCVRQualifiers() & Qualifiers::Const) 
== Qualifiers::Const)) {
+  ConstArg = true;
+}
 break;
   }
 


Index: clang/test/CXX/special/class.copy/p25-0x.cpp
===
--- clang/test/CXX/special/class.copy/p25-0x.cpp
+++ clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -3

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

2022-06-12 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez updated this revision to Diff 436207.
Javier-varez added a comment.
Herald added a project: clang-tools-extra.

Fix clang-tidy tests and run clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

Files:
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/special/class.copy/p12-0x.cpp
  clang/test/CXX/special/class.copy/p25-0x.cpp

Index: clang/test/CXX/special/class.copy/p25-0x.cpp
===
--- clang/test/CXX/special/class.copy/p25-0x.cpp
+++ clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -31,7 +31,22 @@
 struct NonConstCopy {
   NonConstCopy &operator=(NonConstCopy &) = default;
 };
-using _ = not_trivially_assignable;
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
+static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers &operator=(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_assignable;
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/special/class.copy/p12-0x.cpp
===
--- clang/test/CXX/special/class.copy/p12-0x.cpp
+++ clang/test/CXX/special/class.copy/p12-0x.cpp
@@ -28,7 +28,18 @@
 struct NonConstCopy {
   NonConstCopy(NonConstCopy &) = default;
 };
-using _ = not_trivially_copyable;
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_copyable;
 
 // class X has no virtual functions
 struct VFn {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -9772,11 +9772,9 @@
 
   case CXXCopyConstructor:
   case CXXCopyAssignment: {
-// Trivial copy operations always have const, non-volatile parameter types.
-ConstArg = true;
 const ParmVarDecl *Param0 = MD->getParamDecl(0);
 const ReferenceType *RT = Param0->getType()->getAs();
-if (!RT || RT->getPointeeType().getCVRQualifiers() != Qualifiers::Const) {
+if (!RT) {
   if (Diagnose)
 Diag(Param0->getLocation(), diag::note_nontrivial_param_type)
   << Param0->getSourceRange() << Param0->getType()
@@ -9784,6 +9782,11 @@
Context.getRecordType(RD).withConst());
   return false;
 }
+
+if (RT && ((RT->getPointeeType().getCVRQualifiers() & Qualifiers::Const) ==
+   Qualifiers::Const)) {
+  ConstArg = true;
+}
 break;
   }
 
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
@@ -10,12 +10,12 @@
 };
 
 // This class is non-trivially copyable because the copy-constructor and copy
-// assignment take non-const references.
+// assignment take non-const references and are user-provided.
 struct ModifiesRightSide {
   ModifiesRightSide() = default;
-  ModifiesRightSide(ModifiesRightSide &) = default;
+  ModifiesRightSide(ModifiesRightSide &);
   bool operator<(ModifiesRightSide &) const;
-  ModifiesRightSide &operator=(ModifiesRightSide &) = default;
+  ModifiesRightSide &operator=(ModifiesRightSide &);
 };
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/lis

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

2022-06-13 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez updated this revision to Diff 436516.
Javier-varez added a comment.

- Preseve ABI when clang is invoked with -fclang-abi-compat <= 14.
- Add test for defect report in `clang/test/CXX/drs/dr21xx.cpp`.
- Refactor ConstArg to remove if condition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

Files:
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/drs/dr21xx.cpp
  clang/test/CXX/special/class.copy/p12-0x.cpp
  clang/test/CXX/special/class.copy/p25-0x.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -12840,7 +12840,7 @@
 https://wg21.link/cwg2171";>2171
 CD4
 Triviality of copy constructor with less-qualified parameter
-Unknown
+Clang 15
   
   
 https://wg21.link/cwg2172";>2172
Index: clang/test/CXX/special/class.copy/p25-0x.cpp
===
--- clang/test/CXX/special/class.copy/p25-0x.cpp
+++ clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -31,7 +31,22 @@
 struct NonConstCopy {
   NonConstCopy &operator=(NonConstCopy &) = default;
 };
-using _ = not_trivially_assignable;
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
+static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers &operator=(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_assignable;
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/special/class.copy/p12-0x.cpp
===
--- clang/test/CXX/special/class.copy/p12-0x.cpp
+++ clang/test/CXX/special/class.copy/p12-0x.cpp
@@ -28,7 +28,18 @@
 struct NonConstCopy {
   NonConstCopy(NonConstCopy &) = default;
 };
-using _ = not_trivially_copyable;
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_copyable;
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/drs/dr21xx.cpp
===
--- clang/test/CXX/drs/dr21xx.cpp
+++ clang/test/CXX/drs/dr21xx.cpp
@@ -140,6 +140,33 @@
 #endif
 }
 
+namespace dr2171 { // dr2171: 15
+#if __cplusplus >= 201103L
+
+struct NonConstCopy {
+  NonConstCopy(NonConstCopy &) = default;
+  NonConstCopy &operator=(NonConstCopy &) = default;
+};
+
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
+
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
+static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, Non

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

2022-06-13 Thread Javier Alvarez via Phabricator via cfe-commits 
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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2022-06-14 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez updated this revision to Diff 436871.
Javier-varez marked 2 inline comments as done.
Javier-varez added a comment.

Address review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

Files:
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/drs/dr21xx.cpp
  clang/test/CXX/special/class.copy/p12-0x.cpp
  clang/test/CXX/special/class.copy/p25-0x.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -12840,7 +12840,7 @@
 https://wg21.link/cwg2171";>2171
 CD4
 Triviality of copy constructor with less-qualified parameter
-Unknown
+Clang 15
   
   
 https://wg21.link/cwg2172";>2172
Index: clang/test/CXX/special/class.copy/p25-0x.cpp
===
--- clang/test/CXX/special/class.copy/p25-0x.cpp
+++ clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
 
 // expected-no-diagnostics
 
@@ -31,7 +32,30 @@
 struct NonConstCopy {
   NonConstCopy &operator=(NonConstCopy &) = default;
 };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy assignment operators were not trivial because
+// of dr2171
 using _ = not_trivially_assignable;
+#else
+// In the latest Clang version, all defaulted assignment operators are trivial, even if non-const,
+// because dr2171 is fixed
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
+static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers &operator=(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_assignable;
+#endif
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/special/class.copy/p12-0x.cpp
===
--- clang/test/CXX/special/class.copy/p12-0x.cpp
+++ clang/test/CXX/special/class.copy/p12-0x.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted
+// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
 
 // expected-no-diagnostics
 
@@ -28,7 +29,25 @@
 struct NonConstCopy {
   NonConstCopy(NonConstCopy &) = default;
 };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy constructors were not trivial because of dr2171
 using _ = not_trivially_copyable;
+#else
+// In the latest Clang version, all defaulted constructors are trivial, even if non-const, because
+// dr2171 is fixed.
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_copyable;
+#endif
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/drs/dr21xx.cpp
===
--- clang/test/CXX/drs/dr21xx.cpp
+++ clang/test/CXX/drs/dr21xx.cpp
@@ -140,6 +140,33 @@
 #endif
 }
 
+namespace dr2171 { // dr2171: 15
+#if __cplusplus >= 201103L
+
+struct NonConstCopy {
+  NonConstCopy(NonConstCopy &) = default;
+  NonConstCopy &operator=(NonConstCopy &) = default;
+};
+
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivia

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

2022-06-14 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez added a comment.

Thanks again for having another look at the changes! I do not have commit 
access since this is my first change to LLVM. I would appreciate if you could 
merge it. here's my username and email:
Javier Alvarez 

And let me know if there's anything else you'd like to modify :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2022-06-14 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez updated this revision to Diff 437033.
Javier-varez added a comment.

Rebase on main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

Files:
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-const.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/drs/dr21xx.cpp
  clang/test/CXX/special/class.copy/p12-0x.cpp
  clang/test/CXX/special/class.copy/p25-0x.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -12840,7 +12840,7 @@
 https://wg21.link/cwg2171";>2171
 CD4
 Triviality of copy constructor with less-qualified parameter
-Unknown
+Clang 15
   
   
 https://wg21.link/cwg2172";>2172
Index: clang/test/CXX/special/class.copy/p25-0x.cpp
===
--- clang/test/CXX/special/class.copy/p25-0x.cpp
+++ clang/test/CXX/special/class.copy/p25-0x.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
 
 // expected-no-diagnostics
 
@@ -31,7 +32,30 @@
 struct NonConstCopy {
   NonConstCopy &operator=(NonConstCopy &) = default;
 };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy assignment operators were not trivial because
+// of dr2171
 using _ = not_trivially_assignable;
+#else
+// In the latest Clang version, all defaulted assignment operators are trivial, even if non-const,
+// because dr2171 is fixed
+static_assert(__has_trivial_assign(NonConstCopy), "");
+static_assert(__is_trivially_assignable(NonConstCopy &, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &, NonConstCopy &&), "");
+static_assert(__is_trivially_assignable(NonConstCopy &&, NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, const NonConstCopy &), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy), "");
+static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers &operator=(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers &operator=(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_assignable;
+#endif
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/special/class.copy/p12-0x.cpp
===
--- clang/test/CXX/special/class.copy/p12-0x.cpp
+++ clang/test/CXX/special/class.copy/p12-0x.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted
+// RUN: %clang_cc1 -std=c++11 -verify %s -Wno-defaulted-function-deleted -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
 
 // expected-no-diagnostics
 
@@ -28,7 +29,25 @@
 struct NonConstCopy {
   NonConstCopy(NonConstCopy &) = default;
 };
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+// Up until (and including) Clang 14, non-const copy constructors were not trivial because of dr2171
 using _ = not_trivially_copyable;
+#else
+// In the latest Clang version, all defaulted constructors are trivial, even if non-const, because
+// dr2171 is fixed.
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, const NonConstCopy &), "");
+static_assert(!__is_trivially_constructible(NonConstCopy, NonConstCopy &&), "");
+
+struct DefaultedSpecialMembers {
+  DefaultedSpecialMembers(const DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &) = default;
+  DefaultedSpecialMembers(DefaultedSpecialMembers &&) = default;
+};
+using _ = trivially_copyable;
+#endif
 
 // class X has no virtual functions
 struct VFn {
Index: clang/test/CXX/drs/dr21xx.cpp
===
--- clang/test/CXX/drs/dr21xx.cpp
+++ clang/test/CXX/drs/dr21xx.cpp
@@ -140,6 +140,33 @@
 #endif
 }
 
+namespace dr2171 { // dr2171: 15
+#if __cplusplus >= 201103L
+
+struct NonConstCopy {
+  NonConstCopy(NonConstCopy &) = default;
+  NonConstCopy &operator=(NonConstCopy &) = default;
+};
+
+static_assert(__has_trivial_copy(NonConstCopy), "");
+static_assert(__is_trivially_constructible(NonConstCopy, NonConstCopy &), "");
+s

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

2022-06-16 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez added a comment.

I did try this locally and the built test binaries seem to crash with a 
segmentation fault. I run one of them in lldb to check the error and I got this 
backtrace:

  llvm-project/build on  fix-trivially-copyable-check [?⇕] via △ v3.22.3 took 
3s ❯ lldb 
/Users/javier/Desktop/Projects/llvm-project/build/projects/openmp/runtime/test/worksharing/single/Output/omp_single_copyprivate.c.tmp
  (lldb) target create 
"/Users/javier/Desktop/Projects/llvm-project/build/projects/openmp/runtime/test/worksharing/single/Output/omp_single_copyprivate.c.tmp"
  Current executable set to 
'/Users/javier/Desktop/Projects/llvm-project/build/projects/openmp/runtime/test/worksharing/single/Output/omp_single_copyprivate.c.tmp'
 (arm64).
  (lldb) r
  Process 61339 launched: 
'/Users/javier/Desktop/Projects/llvm-project/build/projects/openmp/runtime/test/worksharing/single/Output/omp_single_copyprivate.c.tmp'
 (arm64)
  Process 61339 stopped
  * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS 
(code=1, address=0x10)
  frame #0: 0x000100362f54 libomp.dylib`::__kmp_get_global_thread_id() 
at kmp_runtime.cpp:201:8
 198
 199  /* dynamically updated stack window for uber threads to avoid 
get_specific
 200 call */
  -> 201  if (!TCR_4(other_threads[i]->th.th_info.ds.ds_stackgrow)) {
 202KMP_FATAL(StackOverflow, i);
 203  }
 204
  Target 0: (omp_single_copyprivate.c.tmp) stopped.
  (lldb) bt
  * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS 
(code=1, address=0x10)
* frame #0: 0x000100362f54 libomp.dylib`::__kmp_get_global_thread_id() 
at kmp_runtime.cpp:201:8
  frame #1: 0x0001003b8c60 libomp.dylib`void 
__kmp_release_template >(flag=0x00016fdff0b8) at 
kmp_wait_release.h:790:39
  frame #2: 0x0001003b8c20 
libomp.dylib`::__kmp_release_64(flag=0x00016fdff0b8) at 
kmp_wait_release.cpp:25:46
  frame #3: 0x0001003798d0 
libomp.dylib`__kmp_reap_thread(thread=0x000101010a40, is_root=0) at 
kmp_runtime.cpp:6146:9
  frame #4: 0x00010037497c libomp.dylib`__kmp_internal_end() at 
kmp_runtime.cpp:6334:7
  frame #5: 0x0001003745ec 
libomp.dylib`::__kmp_internal_end_library(gtid_req=-1) at kmp_runtime.cpp:6498:3
  frame #6: 0x000100374258 libomp.dylib`::__kmp_internal_end_atexit() 
at kmp_runtime.cpp:6115:3
  frame #7: 0x000100374218 libomp.dylib`::__kmp_internal_end_dtor() at 
kmp_runtime.cpp:6083:3
  frame #8: 0x000189359dc4 libsystem_c.dylib`__cxa_finalize_ranges + 464
  frame #9: 0x000189359b68 libsystem_c.dylib`exit + 44
  frame #10: 0x00018947aec4 
libdyld.dylib`dyld4::LibSystemHelpers::exit(int) const + 20
  frame #11: 0x0001000150d4 dyld`start + 592
  (lldb)

Not sure how to proceed from here though... I'm really not familiar with OpenMP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2022-06-16 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez added a comment.

I had the same failures with and without this change. It looks like these tests 
are already broken upstream.

Without this patch:

  Unsupported:  15
  Passed :  50
  Failed : 232

With this patch:

  Unsupported:  15
  Passed :  50
  Failed : 232


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2022-06-16 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez added a comment.

For referente, I run the tests without this change on commit:

  commit 94d1692aa155adf3b69609d142762b8c696e0710 (upstream/main, upstream/HEAD)
  Author: Fangrui Song 
  Date:   Tue Jun 14 21:25:56 2022 -0700
  
  [MC] Remove unused MCStreamer::SwitchSection
  
  switchSection should be used instead.

And the tests with this patch just has this patch on top of the previous commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2022-06-17 Thread Javier Alvarez via Phabricator via cfe-commits 
Javier-varez added a comment.

Thanks for taking care of this Roy and helping me out through the process!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits