[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb1c960fc6dc2: [Clang] Implement P0848 (Conditionally Trivial 
Special Member Functions) (authored by royjacobson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs-2.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,8 +927,16 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
-  
+
+  
+Clang 16 (Partial)
+Because of other concepts implementation deficits, the __cpp_concepts macro is not yet set to 202002L.
+Also, the related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly, deleted
+special member functions are treated as eligible even though they shouldn't be.
+  
+
+
   
 https://wg21.link/p1616r1;>P1616R1
 Clang 10
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 455721.
royjacobson added a comment.

Formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs-2.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,8 +927,16 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
-  
+
+  
+Clang 16 (Partial)
+Because of other concepts implementation deficits, the __cpp_concepts macro is not yet set to 202002L.
+Also, the related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly, deleted
+special member functions are treated as eligible even though they shouldn't be.
+  
+
+
   
 https://wg21.link/p1616r1;>P1616R1
 Clang 10
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 455696.
royjacobson added a comment.

We call LookupSpecialMember before we compute eligibility, so the assert fails. 
Remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs-2.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,8 +927,16 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
-  
+
+  
+Clang 16 (Partial)
+Because of other concepts implementation deficits, the __cpp_concepts macro is not yet set to 202002L.
+Also, the related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly, deleted
+special member functions are treated as eligible even though they shouldn't be.
+  
+
+
   
 https://wg21.link/p1616r1;>P1616R1
 Clang 10
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 455689.
royjacobson added a comment.

Rename file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs-2.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,8 +927,16 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
-  
+
+  
+Clang 16 (Partial)
+Because of other concepts implementation deficits, the __cpp_concepts macro is not yet set to 202002L.
+Also, the related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly, deleted
+special member functions are treated as eligible even though they shouldn't be.
+  
+
+
   
 https://wg21.link/p1616r1;>P1616R1
 Clang 10
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/AST/conditionally-trivial-smfs-1.cpp:1
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-linux -ast-dump=json %s | 
FileCheck %s --check-prefixes=CHECK,LIN
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-win32 -ast-dump=json %s | 
FileCheck %s

can you rename that file back to clang/test/AST/conditionally-trivial-smfs.cpp 
? Thanks!



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

I added handling of FunctionTemplateDecls in SemaDecl because they can be 
default constructors. This caused errors with std::pair for example - it has 
two enable_ifs constructors to control explicit/implicit construction.

I will try committing again if the pre-commit CI finishes green. Running `nm` 
on the provided reproducer from FuchsiaOS resulted in the names list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 455686.
royjacobson added a comment.

Default constructors can be template functions. Update patch to handle them 
correctly, and add
asserts to LookupSpecialMember that can catch similar problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs-1.cpp
  clang/test/AST/conditionally-trivial-smfs-2.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,8 +927,16 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
-  
+
+  
+Clang 16 (Partial)
+Because of other concepts implementation deficits, the __cpp_concepts macro is not yet set to 202002L.
+Also, the related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly, deleted
+special member functions are treated as eligible even though they shouldn't be.
+  
+
+
   
 https://wg21.link/p1616r1;>P1616R1
 Clang 10
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Haowei Wu via Phabricator via cfe-commits]

haowei added subscribers: haowei, phosek, mcgrathr.
haowei added a comment.

Hi,

We are seeing build failures on Fuchsia build bots after this change was 
landed. and we suspect this patch causes clang to miscompile certain code. I 
filed bug https://github.com/llvm/llvm-project/issues/57351 and included a 
reproducer. Could you take a look? If it is confirmed to be a bug and take long 
time to fix, could you revert this change please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG717161509914: [Clang] Implement P0848 (Conditionally Trivial 
Special Member Functions) (authored by royjacobson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,8 +927,16 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
-  
+
+  
+Clang 16 (Partial)
+Because of other concepts implementation deficits, the __cpp_concepts macro is not yet set to 202002L.
+Also, the related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly, deleted
+special member functions are treated as eligible even though they shouldn't be.
+  
+
+
   
 https://wg21.link/p1616r1;>P1616R1
 Clang 10
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 454913.
royjacobson added a comment.

Rebase on main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,8 +927,16 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
-  
+
+  
+Clang 16 (Partial)
+Because of other concepts implementation deficits, the __cpp_concepts macro is not yet set to 202002L.
+Also, the related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly, deleted
+special member functions are treated as eligible even though they shouldn't be.
+  
+
+
   
 https://wg21.link/p1616r1;>P1616R1
 Clang 10
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, thank you for this!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin accepted this revision.
cor3ntin added a comment.
This revision is now accepted and ready to land.

I would wait for @aaron.ballman or @erichkeane to sign off on it, but this 
looks good to me. 
Thanks again for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 454577.
royjacobson added a comment.

Clarify ambiguous doc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,8 +927,16 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
-  
+
+  
+Clang 16 (Partial)
+Because of other concepts implementation deficits, the __cpp_concepts macro is not yet set to 202002L.
+Also, the related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly, deleted
+special member functions are treated as eligible even though they shouldn't be.
+  
+
+
   
 https://wg21.link/p1616r1;>P1616R1
 Clang 10
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

erichkeane wrote:
> royjacobson wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > cor3ntin wrote:
> > > > > royjacobson wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Does any of the not-yet-implemented bits (including from the DRs) 
> > > > > > > impact the ability to use conditionally trivial special member 
> > > > > > > functions? If so, we might want to be careful about aggressively 
> > > > > > > bumping this value. (It's more palatable for us to come back and 
> > > > > > > bump the value later than it is for us to claim we implement 
> > > > > > > something fully when we know we don't -- the goal of the feature 
> > > > > > > test macros is so that users don't have to resort to compiler 
> > > > > > > version checks, which is what users have to use when they fall 
> > > > > > > into that "not fully implemented" space.)
> > > > > > I don't think they're very significant, and the benefits of 
> > > > > > enabling it seem large enough for me - for example, std::expected 
> > > > > > works with libstdc++ and passes their unit tests but is gated by 
> > > > > > this macro.
> > > > > > 
> > > > > > We still have a non-trivial amount of concept bugs to go over, but 
> > > > > > I support enabling this.
> > > > > > 
> > > > > I think it's better to be conservative, It's the lesser of two not 
> > > > > great options.
> > > > > I'm hoping we can get to fix the issues in the clang 16 cycle , but 
> > > > > in the meantime we should not claim conformance if we are not, in 
> > > > > fact, conforming.
> > > > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > > > support enabling this.
> > > > 
> > > > I think we should specify the updated macro value only after we think 
> > > > concepts have no known major issues with them. (Unknown issues happen 
> > > > and there's not much we can do about them, this is more that we 
> > > > shouldn't specify that we conform to a particular feature test macro 
> > > > when we knowingly still don't have it right yet.)
> > > Yeah, I don't think we can set this until we can at least compile a basic 
> > > libstdc++ ranges use, which the deferred instantiation still holds up.  
> > > That would be my 'bare minimum' test for whether we can set that.
> > But we're already defining the `__cpp_concepts` macro, even with the 
> > current known bugs. The version bump, introduced by 
> > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2493r0.html is 
> > specifically about the conditionally trivial SMFs paper.
> > 
> > We can decide that we want the version bump to mean 'no more known concept 
> > bugs in clang' instead. But that would extend the meaning of the macro and 
> > would be confusing to users who want to rely on the documented, intended 
> > meaning of the version.
> > 
> > Also I think telling library writers 'this feature works now, but we didn't 
> > set its feature test macro' will make them use more compiler version 
> > checks, not less.
> The feature test macros aren't supposed to mean "I support the feature from 
> the paper that updated it".  They are intended to mean "I support the feature 
> from the paper that updated it AND everything before it".
> 
> I don't believe we need to be bug-free, but something as extreme as "we can't 
> compile a large number of uses of concepts because we don't implement a 
> central design consideration" (which, btw, was clarified in Core after the 
> 2019 date IIRC) means we shouldn't be updating this.
> But we're already defining the __cpp_concepts macro, even with the current 
> known bugs.

FWIW, I consider that a bug. We should have never defined this macro in the 
first place -- we're not there yet on the base feature.

> I don't believe we need to be bug-free, but something as extreme as "we can't 
> compile a large number of uses of concepts because we don't implement a 
> central design consideration" (which, btw, was clarified in Core after the 
> 2019 date IIRC) means we shouldn't be updating this.

+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

erichkeane wrote:
> royjacobson wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > cor3ntin wrote:
> > > > > royjacobson wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Does any of the not-yet-implemented bits (including from the DRs) 
> > > > > > > impact the ability to use conditionally trivial special member 
> > > > > > > functions? If so, we might want to be careful about aggressively 
> > > > > > > bumping this value. (It's more palatable for us to come back and 
> > > > > > > bump the value later than it is for us to claim we implement 
> > > > > > > something fully when we know we don't -- the goal of the feature 
> > > > > > > test macros is so that users don't have to resort to compiler 
> > > > > > > version checks, which is what users have to use when they fall 
> > > > > > > into that "not fully implemented" space.)
> > > > > > I don't think they're very significant, and the benefits of 
> > > > > > enabling it seem large enough for me - for example, std::expected 
> > > > > > works with libstdc++ and passes their unit tests but is gated by 
> > > > > > this macro.
> > > > > > 
> > > > > > We still have a non-trivial amount of concept bugs to go over, but 
> > > > > > I support enabling this.
> > > > > > 
> > > > > I think it's better to be conservative, It's the lesser of two not 
> > > > > great options.
> > > > > I'm hoping we can get to fix the issues in the clang 16 cycle , but 
> > > > > in the meantime we should not claim conformance if we are not, in 
> > > > > fact, conforming.
> > > > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > > > support enabling this.
> > > > 
> > > > I think we should specify the updated macro value only after we think 
> > > > concepts have no known major issues with them. (Unknown issues happen 
> > > > and there's not much we can do about them, this is more that we 
> > > > shouldn't specify that we conform to a particular feature test macro 
> > > > when we knowingly still don't have it right yet.)
> > > Yeah, I don't think we can set this until we can at least compile a basic 
> > > libstdc++ ranges use, which the deferred instantiation still holds up.  
> > > That would be my 'bare minimum' test for whether we can set that.
> > But we're already defining the `__cpp_concepts` macro, even with the 
> > current known bugs. The version bump, introduced by 
> > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2493r0.html is 
> > specifically about the conditionally trivial SMFs paper.
> > 
> > We can decide that we want the version bump to mean 'no more known concept 
> > bugs in clang' instead. But that would extend the meaning of the macro and 
> > would be confusing to users who want to rely on the documented, intended 
> > meaning of the version.
> > 
> > Also I think telling library writers 'this feature works now, but we didn't 
> > set its feature test macro' will make them use more compiler version 
> > checks, not less.
> The feature test macros aren't supposed to mean "I support the feature from 
> the paper that updated it".  They are intended to mean "I support the feature 
> from the paper that updated it AND everything before it".
> 
> I don't believe we need to be bug-free, but something as extreme as "we can't 
> compile a large number of uses of concepts because we don't implement a 
> central design consideration" (which, btw, was clarified in Core after the 
> 2019 date IIRC) means we shouldn't be updating this.
It seems like I'm in the minority here, so I removed the version change and 
added a comment with a reference to this discussion to the code.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 454567.
royjacobson added a comment.

Remove the __cpp_concepts version bump.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,8 +927,16 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
-  
+
+  
+Clang 16 (Partial)
+Because of current implementation deficits, the __cpp_concepts macro is not yet set to 202002L.
+Also, the related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly, deleted
+special member functions are treated as eligible even though they shouldn't be.
+  
+
+
   
 https://wg21.link/p1616r1;>P1616R1
 Clang 10
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+   

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

royjacobson wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > cor3ntin wrote:
> > > > royjacobson wrote:
> > > > > aaron.ballman wrote:
> > > > > > Does any of the not-yet-implemented bits (including from the DRs) 
> > > > > > impact the ability to use conditionally trivial special member 
> > > > > > functions? If so, we might want to be careful about aggressively 
> > > > > > bumping this value. (It's more palatable for us to come back and 
> > > > > > bump the value later than it is for us to claim we implement 
> > > > > > something fully when we know we don't -- the goal of the feature 
> > > > > > test macros is so that users don't have to resort to compiler 
> > > > > > version checks, which is what users have to use when they fall into 
> > > > > > that "not fully implemented" space.)
> > > > > I don't think they're very significant, and the benefits of enabling 
> > > > > it seem large enough for me - for example, std::expected works with 
> > > > > libstdc++ and passes their unit tests but is gated by this macro.
> > > > > 
> > > > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > > > support enabling this.
> > > > > 
> > > > I think it's better to be conservative, It's the lesser of two not 
> > > > great options.
> > > > I'm hoping we can get to fix the issues in the clang 16 cycle , but in 
> > > > the meantime we should not claim conformance if we are not, in fact, 
> > > > conforming.
> > > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > > support enabling this.
> > > 
> > > I think we should specify the updated macro value only after we think 
> > > concepts have no known major issues with them. (Unknown issues happen and 
> > > there's not much we can do about them, this is more that we shouldn't 
> > > specify that we conform to a particular feature test macro when we 
> > > knowingly still don't have it right yet.)
> > Yeah, I don't think we can set this until we can at least compile a basic 
> > libstdc++ ranges use, which the deferred instantiation still holds up.  
> > That would be my 'bare minimum' test for whether we can set that.
> But we're already defining the `__cpp_concepts` macro, even with the current 
> known bugs. The version bump, introduced by 
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2493r0.html is 
> specifically about the conditionally trivial SMFs paper.
> 
> We can decide that we want the version bump to mean 'no more known concept 
> bugs in clang' instead. But that would extend the meaning of the macro and 
> would be confusing to users who want to rely on the documented, intended 
> meaning of the version.
> 
> Also I think telling library writers 'this feature works now, but we didn't 
> set its feature test macro' will make them use more compiler version checks, 
> not less.
The feature test macros aren't supposed to mean "I support the feature from the 
paper that updated it".  They are intended to mean "I support the feature from 
the paper that updated it AND everything before it".

I don't believe we need to be bug-free, but something as extreme as "we can't 
compile a large number of uses of concepts because we don't implement a central 
design consideration" (which, btw, was clarified in Core after the 2019 date 
IIRC) means we shouldn't be updating this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

erichkeane wrote:
> aaron.ballman wrote:
> > cor3ntin wrote:
> > > royjacobson wrote:
> > > > aaron.ballman wrote:
> > > > > Does any of the not-yet-implemented bits (including from the DRs) 
> > > > > impact the ability to use conditionally trivial special member 
> > > > > functions? If so, we might want to be careful about aggressively 
> > > > > bumping this value. (It's more palatable for us to come back and bump 
> > > > > the value later than it is for us to claim we implement something 
> > > > > fully when we know we don't -- the goal of the feature test macros is 
> > > > > so that users don't have to resort to compiler version checks, which 
> > > > > is what users have to use when they fall into that "not fully 
> > > > > implemented" space.)
> > > > I don't think they're very significant, and the benefits of enabling it 
> > > > seem large enough for me - for example, std::expected works with 
> > > > libstdc++ and passes their unit tests but is gated by this macro.
> > > > 
> > > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > > support enabling this.
> > > > 
> > > I think it's better to be conservative, It's the lesser of two not great 
> > > options.
> > > I'm hoping we can get to fix the issues in the clang 16 cycle , but in 
> > > the meantime we should not claim conformance if we are not, in fact, 
> > > conforming.
> > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > support enabling this.
> > 
> > I think we should specify the updated macro value only after we think 
> > concepts have no known major issues with them. (Unknown issues happen and 
> > there's not much we can do about them, this is more that we shouldn't 
> > specify that we conform to a particular feature test macro when we 
> > knowingly still don't have it right yet.)
> Yeah, I don't think we can set this until we can at least compile a basic 
> libstdc++ ranges use, which the deferred instantiation still holds up.  That 
> would be my 'bare minimum' test for whether we can set that.
But we're already defining the `__cpp_concepts` macro, even with the current 
known bugs. The version bump, introduced by 
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2493r0.html is 
specifically about the conditionally trivial SMFs paper.

We can decide that we want the version bump to mean 'no more known concept bugs 
in clang' instead. But that would extend the meaning of the macro and would be 
confusing to users who want to rely on the documented, intended meaning of the 
version.

Also I think telling library writers 'this feature works now, but we didn't set 
its feature test macro' will make them use more compiler version checks, not 
less.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

aaron.ballman wrote:
> cor3ntin wrote:
> > royjacobson wrote:
> > > aaron.ballman wrote:
> > > > Does any of the not-yet-implemented bits (including from the DRs) 
> > > > impact the ability to use conditionally trivial special member 
> > > > functions? If so, we might want to be careful about aggressively 
> > > > bumping this value. (It's more palatable for us to come back and bump 
> > > > the value later than it is for us to claim we implement something fully 
> > > > when we know we don't -- the goal of the feature test macros is so that 
> > > > users don't have to resort to compiler version checks, which is what 
> > > > users have to use when they fall into that "not fully implemented" 
> > > > space.)
> > > I don't think they're very significant, and the benefits of enabling it 
> > > seem large enough for me - for example, std::expected works with 
> > > libstdc++ and passes their unit tests but is gated by this macro.
> > > 
> > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > support enabling this.
> > > 
> > I think it's better to be conservative, It's the lesser of two not great 
> > options.
> > I'm hoping we can get to fix the issues in the clang 16 cycle , but in the 
> > meantime we should not claim conformance if we are not, in fact, conforming.
> > We still have a non-trivial amount of concept bugs to go over, but I 
> > support enabling this.
> 
> I think we should specify the updated macro value only after we think 
> concepts have no known major issues with them. (Unknown issues happen and 
> there's not much we can do about them, this is more that we shouldn't specify 
> that we conform to a particular feature test macro when we knowingly still 
> don't have it right yet.)
Yeah, I don't think we can set this until we can at least compile a basic 
libstdc++ ranges use, which the deferred instantiation still holds up.  That 
would be my 'bare minimum' test for whether we can set that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

cor3ntin wrote:
> royjacobson wrote:
> > aaron.ballman wrote:
> > > Does any of the not-yet-implemented bits (including from the DRs) impact 
> > > the ability to use conditionally trivial special member functions? If so, 
> > > we might want to be careful about aggressively bumping this value. (It's 
> > > more palatable for us to come back and bump the value later than it is 
> > > for us to claim we implement something fully when we know we don't -- the 
> > > goal of the feature test macros is so that users don't have to resort to 
> > > compiler version checks, which is what users have to use when they fall 
> > > into that "not fully implemented" space.)
> > I don't think they're very significant, and the benefits of enabling it 
> > seem large enough for me - for example, std::expected works with libstdc++ 
> > and passes their unit tests but is gated by this macro.
> > 
> > We still have a non-trivial amount of concept bugs to go over, but I 
> > support enabling this.
> > 
> I think it's better to be conservative, It's the lesser of two not great 
> options.
> I'm hoping we can get to fix the issues in the clang 16 cycle , but in the 
> meantime we should not claim conformance if we are not, in fact, conforming.
> We still have a non-trivial amount of concept bugs to go over, but I support 
> enabling this.

I think we should specify the updated macro value only after we think concepts 
have no known major issues with them. (Unknown issues happen and there's not 
much we can do about them, this is more that we shouldn't specify that we 
conform to a particular feature test macro when we knowingly still don't have 
it right yet.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

royjacobson wrote:
> aaron.ballman wrote:
> > Does any of the not-yet-implemented bits (including from the DRs) impact 
> > the ability to use conditionally trivial special member functions? If so, 
> > we might want to be careful about aggressively bumping this value. (It's 
> > more palatable for us to come back and bump the value later than it is for 
> > us to claim we implement something fully when we know we don't -- the goal 
> > of the feature test macros is so that users don't have to resort to 
> > compiler version checks, which is what users have to use when they fall 
> > into that "not fully implemented" space.)
> I don't think they're very significant, and the benefits of enabling it seem 
> large enough for me - for example, std::expected works with libstdc++ and 
> passes their unit tests but is gated by this macro.
> 
> We still have a non-trivial amount of concept bugs to go over, but I support 
> enabling this.
> 
I think it's better to be conservative, It's the lesser of two not great 
options.
I'm hoping we can get to fix the issues in the clang 16 cycle , but in the 
meantime we should not claim conformance if we are not, in fact, conforming.



Comment at: clang/www/cxx_status.html:930
 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 16 (12)
   

royjacobson wrote:
> aaron.ballman wrote:
> > FWIW, the way we've started handling this in recent history is to use 
> > "partial" and a details tag instead of a footnote, as in: 
> > https://github.com/llvm/llvm-project/blob/main/clang/www/cxx_status.html#L915.
> It felt a bit too long of an explanation to put in the tiny table box, but I 
> don't feel very strongly about it either way.
I agree with Aaron. 
I think it's better to be consistent, the column resize when the details are 
expanded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson marked 11 inline comments as done.
royjacobson added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

aaron.ballman wrote:
> Does any of the not-yet-implemented bits (including from the DRs) impact the 
> ability to use conditionally trivial special member functions? If so, we 
> might want to be careful about aggressively bumping this value. (It's more 
> palatable for us to come back and bump the value later than it is for us to 
> claim we implement something fully when we know we don't -- the goal of the 
> feature test macros is so that users don't have to resort to compiler version 
> checks, which is what users have to use when they fall into that "not fully 
> implemented" space.)
I don't think they're very significant, and the benefits of enabling it seem 
large enough for me - for example, std::expected works with libstdc++ and 
passes their unit tests but is gated by this macro.

We still have a non-trivial amount of concept bugs to go over, but I support 
enabling this.




Comment at: clang/www/cxx_status.html:930
 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 16 (12)
   

aaron.ballman wrote:
> FWIW, the way we've started handling this in recent history is to use 
> "partial" and a details tag instead of a footnote, as in: 
> https://github.com/llvm/llvm-project/blob/main/clang/www/cxx_status.html#L915.
It felt a bit too long of an explanation to put in the tiny table box, but I 
don't feel very strongly about it either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 454338.
royjacobson added a comment.

Address Aaron's comments, add the consteval test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 16 (12)
   
   
 https://wg21.link/p1616r1;>P1616R1
@@ -1274,6 +1274,10 @@
 (11): Prior to Clang 8, this feature is not enabled by
 -std=c++20, but can be enabled with -fchar8_t.
 
+(12): The related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly deleted special
+member functions are treated as eligible even though they shouldn't be.
+
 
 
 
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added a comment.

Can you please also add `consteval` related test case (mentioned in 
https://github.com/llvm/llvm-project/issues/57046#issuecomment-1211665079) to 
cxx2a-consteval.cpp 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/DeclCXX.cpp:1443-1447
+if (D->isTrivial()) {
+  data().HasTrivialSpecialMembers |= SMKind;
+}
+else
+  data().DeclaredNonTrivialSpecialMembers |= SMKind;





Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

Does any of the not-yet-implemented bits (including from the DRs) impact the 
ability to use conditionally trivial special member functions? If so, we might 
want to be careful about aggressively bumping this value. (It's more palatable 
for us to come back and bump the value later than it is for us to claim we 
implement something fully when we know we don't -- the goal of the feature test 
macros is so that users don't have to resort to compiler version checks, which 
is what users have to use when they fall into that "not fully implemented" 
space.)



Comment at: clang/lib/Sema/SemaDecl.cpp:17980
+///   [CWG2595], if any, are satisfied is more constrained.
+void SetEligibleMethods(Sema , CXXRecordDecl* Record,
+ArrayRef Methods,

You may want to run the patch through clang-format before you land, just to be 
sure 
(https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting).



Comment at: clang/lib/Sema/SemaDecl.cpp:17870
+///   parameter type and the same cv-qualifiers and ref-qualifier, if any.
+bool AreSpecialMemberFunctionsSameKind(CXXMethodDecl *M1, CXXMethodDecl *M2,
+ Sema::CXXSpecialMember CSM) {

royjacobson wrote:
> cor3ntin wrote:
> > 
> It's in an anonymous namespace. Is it a Clang convention to use static 
> functions instead? I saw both used in Sema, I think
Yeah, we prefer putting types into an anonymous namespace but making functions 
static: https://llvm.org/docs/CodingStandards.html#anonymous-namespaces



Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:344
+// CHECK-NEXT:  },
\ No newline at end of file


Please add a newline to the end of the file.



Comment at: clang/www/cxx_status.html:930
 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 16 (12)
   

FWIW, the way we've started handling this in recent history is to use "partial" 
and a details tag instead of a footnote, as in: 
https://github.com/llvm/llvm-project/blob/main/clang/www/cxx_status.html#L915.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

@aaron.ballman I think this looks good, you want to double check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:494-497
+- Implemented "Conditionally Trivial Special Member Functions" (`P0848 
`_).
+  Note: The handling of deleted functions is not yet compliant, as Clang
+  does not implement `DR1496 
`_
+  and `DR1734 
`_.

cor3ntin wrote:
> royjacobson wrote:
> > h-vetinari wrote:
> > > Is that lack of compliance worth a note in `cxx_status`?
> > I'm not very opinionated about this, but I tend to not significant enough. 
> > I mean, 7 years later and only MSVC have even bothered to implement them.
> We might has well, I think it's a good way to not lose track of it.
Added as a footnote.



Comment at: clang/test/SemaCXX/constrained-special-member-functions.cpp:200
+// FIXME: We should not throw an error, instead SFINAE should make the 
constraint
+// silently unsatisfied. See [temp.constr.constr]p5
+template 

cor3ntin wrote:
> Have you been able to investigate that?
No, I haven't looked at this - we already have a bunch of those all over the 
code, see https://github.com/llvm/llvm-project/issues/48857

So I don't think this is about this patch specifically - I just call 
Sema::CheckFunctionConstraints and inherit its current behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 453712.
royjacobson added a comment.

Add disclaimer to cxx_status about the standing DRs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 16 (12)
   
   
 https://wg21.link/p1616r1;>P1616R1
@@ -1274,6 +1274,10 @@
 (11): Prior to Clang 8, this feature is not enabled by
 -std=c++20, but can be enabled with -fchar8_t.
 
+(12): The related defect reports https://wg21.link/cwg1496;>DR1496 and
+https://wg21.link/cwg1734;>DR1734 are not yet implemented. Accordingly deleted special
+member functions are treated as eligible even though they shouldn't be.
+
 
 
 
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,210 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

This looks good to me.

I *think* landing things without doing silent sfinae for incomplete types is 
okay-ish, but I'd like to here from other folks.

@royjacobson Have you looked into that? Do you know how involved would it be?

Either way, we should make sure that if that bit is dealt with separately, we 
have an issue or something to track it, maybe a note in `cxx_status`.




Comment at: clang/docs/ReleaseNotes.rst:494-497
+- Implemented "Conditionally Trivial Special Member Functions" (`P0848 
`_).
+  Note: The handling of deleted functions is not yet compliant, as Clang
+  does not implement `DR1496 
`_
+  and `DR1734 
`_.

royjacobson wrote:
> h-vetinari wrote:
> > Is that lack of compliance worth a note in `cxx_status`?
> I'm not very opinionated about this, but I tend to not significant enough. I 
> mean, 7 years later and only MSVC have even bothered to implement them.
We might has well, I think it's a good way to not lose track of it.



Comment at: clang/lib/Sema/SemaDecl.cpp:17875
+return true;
+  if (!Context.hasSameType(M1->getParamDecl(0)->getType(),
+   M2->getParamDecl(0)->getType()))

royjacobson wrote:
> royjacobson wrote:
> > shafik wrote:
> > > What happens if we have further parameters with default arguments? Unless 
> > > I am missing something they are still special member functions but the 
> > > proposal does not seem to cover them.
> > That's an excellent question.
> > 
> > I'm not sure what to do about default arguments. In a context where the 
> > additional parameters matter, you're not using them as constructors 
> > anymore, right? So why would this affect the type traits?
> > On the one hand [over.match.best] is against this idea of comparing 
> > constraints when the parameters differ. So also every context where this 
> > actually matters the overload resolution would probably be ambiguous anyway?
> > 
> > @BRevzin, what do you think? Is the wording intentional to include 
> > copy/move constructors with default arguments as well?
> > 
> > I checked with GCC and they seem to handle default arguments separately: 
> > https://godbolt.org/z/1ch3M7MjP
> > 
> FWIW, I filed https://github.com/cplusplus/CWG/issues/110
> 
> I'm not on the reflector myself, so I don't know if there's been a follow-up 
> there.
Yes, i think it's reasonable to not concerns ourselves with default parameters 
until wg21 decides otherwise



Comment at: clang/test/SemaCXX/constrained-special-member-functions.cpp:200
+// FIXME: We should not throw an error, instead SFINAE should make the 
constraint
+// silently unsatisfied. See [temp.constr.constr]p5
+template 

Have you been able to investigate that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added a comment.

I would like to see this land. This also fixes 
https://github.com/llvm/llvm-project/issues/57046.

Are there any pending concerns of the reviewers which we need to address ? 
Happy to help in any way to move this patch forward.

CC: @erichkeane @cor3ntin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:494-497
+- Implemented "Conditionally Trivial Special Member Functions" (`P0848 
`_).
+  Note: The handling of deleted functions is not yet compliant, as Clang
+  does not implement `DR1496 
`_
+  and `DR1734 
`_.

h-vetinari wrote:
> Is that lack of compliance worth a note in `cxx_status`?
I'm not very opinionated about this, but I tend to not significant enough. I 
mean, 7 years later and only MSVC have even bothered to implement them.



Comment at: clang/lib/Sema/SemaDecl.cpp:17875
+return true;
+  if (!Context.hasSameType(M1->getParamDecl(0)->getType(),
+   M2->getParamDecl(0)->getType()))

royjacobson wrote:
> shafik wrote:
> > What happens if we have further parameters with default arguments? Unless I 
> > am missing something they are still special member functions but the 
> > proposal does not seem to cover them.
> That's an excellent question.
> 
> I'm not sure what to do about default arguments. In a context where the 
> additional parameters matter, you're not using them as constructors anymore, 
> right? So why would this affect the type traits?
> On the one hand [over.match.best] is against this idea of comparing 
> constraints when the parameters differ. So also every context where this 
> actually matters the overload resolution would probably be ambiguous anyway?
> 
> @BRevzin, what do you think? Is the wording intentional to include copy/move 
> constructors with default arguments as well?
> 
> I checked with GCC and they seem to handle default arguments separately: 
> https://godbolt.org/z/1ch3M7MjP
> 
FWIW, I filed https://github.com/cplusplus/CWG/issues/110

I'm not on the reflector myself, so I don't know if there's been a follow-up 
there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 453411.
royjacobson added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 16
   
   
 https://wg21.link/p1616r1;>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,210 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<0>));
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<2>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<3>));
+static_assert(__is_trivial(MoveAssignmentChecker<0>));
+// FIXME: DR1734.

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 450072.
royjacobson added a comment.

Rebase on main and re-target LLVM16 in cxx_status


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 16
   
   
 https://wg21.link/p1616r1;>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,210 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<0>));
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<2>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<3>));
+static_assert(__is_trivial(MoveAssignmentChecker<0>));
+// 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

@erichkeane @cor3ntin

If you have time for review right now, I think it's reasonably ready and I 
would still like to merge it this week so it can land in Clang15.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 447425.
royjacobson marked an inline comment as done.
royjacobson added a comment.

Rebase on main and slightly update docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 15
   
   
 https://wg21.link/p1616r1;>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,210 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<0>));
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<2>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<3>));

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17875
+return true;
+  if (!Context.hasSameType(M1->getParamDecl(0)->getType(),
+   M2->getParamDecl(0)->getType()))

shafik wrote:
> What happens if we have further parameters with default arguments? Unless I 
> am missing something they are still special member functions but the proposal 
> does not seem to cover them.
That's an excellent question.

I'm not sure what to do about default arguments. In a context where the 
additional parameters matter, you're not using them as constructors anymore, 
right? So why would this affect the type traits?
On the one hand [over.match.best] is against this idea of comparing constraints 
when the parameters differ. So also every context where this actually matters 
the overload resolution would probably be ambiguous anyway?

@BRevzin, what do you think? Is the wording intentional to include copy/move 
constructors with default arguments as well?

I checked with GCC and they seem to handle default arguments separately: 
https://godbolt.org/z/1ch3M7MjP



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/DeclCXX.cpp:899
+  // triviality properties of the class until selecting a destructor and
+  // computing the eligibility of SMFs. This is because those member
+  // functions may have constraints that we need to evaluate and compare

I think we should spell out `Special Member Function` instead of using `SMF` 



Comment at: clang/lib/Sema/SemaDecl.cpp:17875
+return true;
+  if (!Context.hasSameType(M1->getParamDecl(0)->getType(),
+   M2->getParamDecl(0)->getType()))

What happens if we have further parameters with default arguments? Unless I am 
missing something they are still special member functions but the proposal does 
not seem to cover them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/test/SemaCXX/constrained-special-member-functions.cpp:103
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(CopyAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(CopyAssignmentChecker<2>));

cor3ntin wrote:
> Have you rebased since D127593 landed?
Yes. It's related but it's different DRs. And unfortunately they seem much more 
ABI breaking to fix - GCC don't implement them, for example


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/SemaCXX/constrained-special-member-functions.cpp:103
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(CopyAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(CopyAssignmentChecker<2>));

Have you rebased since D127593 landed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

cor3ntin wrote:
> erichkeane wrote:
> > royjacobson wrote:
> > > erichkeane wrote:
> > > > royjacobson wrote:
> > > > > erichkeane wrote:
> > > > > > cor3ntin wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > cor3ntin wrote:
> > > > > > > > > royjacobson wrote:
> > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > This seems problematic, doesn't it?  Checking this 
> > > > > > > > > > > constraint will (once I figure out how to get deferred 
> > > > > > > > > > > instantiation to work) cause instantiation, which can 
> > > > > > > > > > > cause issues with incomplete types/CRTP/etc.
> > > > > > > > > > > 
> > > > > > > > > > > I think the result is that we cannot 'calculate' this 
> > > > > > > > > > > until it is queried, else we will cause incorrect errors.
> > > > > > > > > > Making this queried on demand is a relatively big change to 
> > > > > > > > > > how we handle type triviality, so I want to be sure we 
> > > > > > > > > > actually need to do this to be conformant.
> > > > > > > > > > 
> > > > > > > > > > When I started working on this I checked what GCC does and 
> > > > > > > > > > it instantiates those constraints during class completion 
> > > > > > > > > > as well. For example this CRTP case: 
> > > > > > > > > > https://godbolt.org/z/EdoYf96zq. MSVC seem to do it as well.
> > > > > > > > > > 
> > > > > > > > > > So maybe it's OK to check the constraints of SMFs 
> > > > > > > > > > specifically?
> > > > > > > > > > 
> > > > > > > > > I think this is done on completeness already in this patch, 
> > > > > > > > > unless i misunderstood the code.
> > > > > > > > > I don't think doing it on demand is a great direction, as 
> > > > > > > > > this does not only affect type traits but also code gen, etc. 
> > > > > > > > > It would create instanciations in unexpected places. wouldn't 
> > > > > > > > > it.
> > > > > > > > > Does the standard has wording suggesting it should be done 
> > > > > > > > > later than on type completeness?
> > > > > > > > The problem, at least for the deferred concepts, is that it 
> > > > > > > > breaks in the CRTP as required to implement ranges.  So 
> > > > > > > > something like this: https://godbolt.org/z/hPqrcqhx5 breaks.
> > > > > > > > 
> > > > > > > > I'm currently working to 'fix' that, so if this patch causes us 
> > > > > > > > to 'check' constraints early, it'll go back to breaking that 
> > > > > > > > example.  The example that Roy showed seems to show that it is 
> > > > > > > > actually checking 'delayed' right now (that is, on demand) in 
> > > > > > > > GCC/MSVC.  I don't know of the consequence/standardeeze that 
> > > > > > > > causes that though.
> > > > > > > Thanks, 
> > > > > > > Follow up stupid question then, do we care about the triviality 
> > > > > > > of dependant types?
> > > > > > > I think doing the check on complete non-dependant types might be 
> > > > > > > a better solution than trying to do it lazily on triviality 
> > > > > > > checks?
> > > > > > I would think it would be not a problem on non-dependent types.  
> > > > > > BUT concepts are only allowed on templated functions (note not only 
> > > > > > on function-templates!) anyway, so I don't think this would be a 
> > > > > > problem?  
> > > > > Erich, I'm a bit confused by your response. I think my example 
> > > > > demonstrates that for default constructors (and other SMFs) GCC and 
> > > > > MSVC instantiate the constraints on class completion and **not** on 
> > > > > demand. This is what I would like to do as well, if we don't have a 
> > > > > good reason not to. (For destructors, performing the checks is even 
> > > > > explicit in the standard.)
> > > > > 
> > > > > Not doing this can introduce some REALLY bad edge cases. What does 
> > > > > this do if we defer the triviality computation?
> > > > > 
> > > > > ```c++
> > > > > 
> > > > > template 
> > > > > struct Base {
> > > > >   Base() = default;
> > > > >   Base() requires (!std::is_trivial_v);
> > > > > };
> > > > > 
> > > > > struct Child : Base { };
> > > > > ```
> > > > > We defer the computation of the constraints on `Base`, and complete 
> > > > > `Child` somehow, but if `Child` is complete then 
> > > > > `std::is_trivial_v` should be well-formed, right? But we get a 
> > > > > logical contradiction instead.
> > > > > 
> > > > > 
> > > > >Erich, I'm a bit confused by your response
> > > > It is entirely possible we're talking past eachother, or 
> > > > misunderstanding eachothers examples.  I'm totally open to that being 
> > > > part of this issue.
> > > > 
> > > > In that example, if we calculate the triviality at '`Base` Class 
> > > > completion', `Child` is not yet complete, right?  So the is_trivial_v 
> > > > would be UB.  

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 446264.
royjacobson added a comment.

Add a test for the CRTP constraints timings


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 15
   
   
 https://wg21.link/p1616r1;>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,210 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<0>));
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<2>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<3>));
+static_assert(__is_trivial(MoveAssignmentChecker<0>));
+// FIXME: 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

erichkeane wrote:
> royjacobson wrote:
> > erichkeane wrote:
> > > royjacobson wrote:
> > > > erichkeane wrote:
> > > > > cor3ntin wrote:
> > > > > > erichkeane wrote:
> > > > > > > cor3ntin wrote:
> > > > > > > > royjacobson wrote:
> > > > > > > > > erichkeane wrote:
> > > > > > > > > > This seems problematic, doesn't it?  Checking this 
> > > > > > > > > > constraint will (once I figure out how to get deferred 
> > > > > > > > > > instantiation to work) cause instantiation, which can cause 
> > > > > > > > > > issues with incomplete types/CRTP/etc.
> > > > > > > > > > 
> > > > > > > > > > I think the result is that we cannot 'calculate' this until 
> > > > > > > > > > it is queried, else we will cause incorrect errors.
> > > > > > > > > Making this queried on demand is a relatively big change to 
> > > > > > > > > how we handle type triviality, so I want to be sure we 
> > > > > > > > > actually need to do this to be conformant.
> > > > > > > > > 
> > > > > > > > > When I started working on this I checked what GCC does and it 
> > > > > > > > > instantiates those constraints during class completion as 
> > > > > > > > > well. For example this CRTP case: 
> > > > > > > > > https://godbolt.org/z/EdoYf96zq. MSVC seem to do it as well.
> > > > > > > > > 
> > > > > > > > > So maybe it's OK to check the constraints of SMFs 
> > > > > > > > > specifically?
> > > > > > > > > 
> > > > > > > > I think this is done on completeness already in this patch, 
> > > > > > > > unless i misunderstood the code.
> > > > > > > > I don't think doing it on demand is a great direction, as this 
> > > > > > > > does not only affect type traits but also code gen, etc. It 
> > > > > > > > would create instanciations in unexpected places. wouldn't it.
> > > > > > > > Does the standard has wording suggesting it should be done 
> > > > > > > > later than on type completeness?
> > > > > > > The problem, at least for the deferred concepts, is that it 
> > > > > > > breaks in the CRTP as required to implement ranges.  So something 
> > > > > > > like this: https://godbolt.org/z/hPqrcqhx5 breaks.
> > > > > > > 
> > > > > > > I'm currently working to 'fix' that, so if this patch causes us 
> > > > > > > to 'check' constraints early, it'll go back to breaking that 
> > > > > > > example.  The example that Roy showed seems to show that it is 
> > > > > > > actually checking 'delayed' right now (that is, on demand) in 
> > > > > > > GCC/MSVC.  I don't know of the consequence/standardeeze that 
> > > > > > > causes that though.
> > > > > > Thanks, 
> > > > > > Follow up stupid question then, do we care about the triviality of 
> > > > > > dependant types?
> > > > > > I think doing the check on complete non-dependant types might be a 
> > > > > > better solution than trying to do it lazily on triviality checks?
> > > > > I would think it would be not a problem on non-dependent types.  BUT 
> > > > > concepts are only allowed on templated functions (note not only on 
> > > > > function-templates!) anyway, so I don't think this would be a 
> > > > > problem?  
> > > > Erich, I'm a bit confused by your response. I think my example 
> > > > demonstrates that for default constructors (and other SMFs) GCC and 
> > > > MSVC instantiate the constraints on class completion and **not** on 
> > > > demand. This is what I would like to do as well, if we don't have a 
> > > > good reason not to. (For destructors, performing the checks is even 
> > > > explicit in the standard.)
> > > > 
> > > > Not doing this can introduce some REALLY bad edge cases. What does this 
> > > > do if we defer the triviality computation?
> > > > 
> > > > ```c++
> > > > 
> > > > template 
> > > > struct Base {
> > > >   Base() = default;
> > > >   Base() requires (!std::is_trivial_v);
> > > > };
> > > > 
> > > > struct Child : Base { };
> > > > ```
> > > > We defer the computation of the constraints on `Base`, and complete 
> > > > `Child` somehow, but if `Child` is complete then 
> > > > `std::is_trivial_v` should be well-formed, right? But we get a 
> > > > logical contradiction instead.
> > > > 
> > > > 
> > > >Erich, I'm a bit confused by your response
> > > It is entirely possible we're talking past eachother, or misunderstanding 
> > > eachothers examples.  I'm totally open to that being part of this issue.
> > > 
> > > In that example, if we calculate the triviality at '`Base` Class 
> > > completion', `Child` is not yet complete, right?  So the is_trivial_v 
> > > would be UB.  If you instead require `sizeof(T)`, we typically give a 
> > > diagnostic.
> > > 
> > > In this case, you'd at MINIMUM have to wait to calculate the 
> > > requires-clause until after `Child` is complete.  And it isn't clear to 
> > > me 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

royjacobson wrote:
> erichkeane wrote:
> > royjacobson wrote:
> > > erichkeane wrote:
> > > > cor3ntin wrote:
> > > > > erichkeane wrote:
> > > > > > cor3ntin wrote:
> > > > > > > royjacobson wrote:
> > > > > > > > erichkeane wrote:
> > > > > > > > > This seems problematic, doesn't it?  Checking this constraint 
> > > > > > > > > will (once I figure out how to get deferred instantiation to 
> > > > > > > > > work) cause instantiation, which can cause issues with 
> > > > > > > > > incomplete types/CRTP/etc.
> > > > > > > > > 
> > > > > > > > > I think the result is that we cannot 'calculate' this until 
> > > > > > > > > it is queried, else we will cause incorrect errors.
> > > > > > > > Making this queried on demand is a relatively big change to how 
> > > > > > > > we handle type triviality, so I want to be sure we actually 
> > > > > > > > need to do this to be conformant.
> > > > > > > > 
> > > > > > > > When I started working on this I checked what GCC does and it 
> > > > > > > > instantiates those constraints during class completion as well. 
> > > > > > > > For example this CRTP case: https://godbolt.org/z/EdoYf96zq. 
> > > > > > > > MSVC seem to do it as well.
> > > > > > > > 
> > > > > > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > > > > > 
> > > > > > > I think this is done on completeness already in this patch, 
> > > > > > > unless i misunderstood the code.
> > > > > > > I don't think doing it on demand is a great direction, as this 
> > > > > > > does not only affect type traits but also code gen, etc. It would 
> > > > > > > create instanciations in unexpected places. wouldn't it.
> > > > > > > Does the standard has wording suggesting it should be done later 
> > > > > > > than on type completeness?
> > > > > > The problem, at least for the deferred concepts, is that it breaks 
> > > > > > in the CRTP as required to implement ranges.  So something like 
> > > > > > this: https://godbolt.org/z/hPqrcqhx5 breaks.
> > > > > > 
> > > > > > I'm currently working to 'fix' that, so if this patch causes us to 
> > > > > > 'check' constraints early, it'll go back to breaking that example.  
> > > > > > The example that Roy showed seems to show that it is actually 
> > > > > > checking 'delayed' right now (that is, on demand) in GCC/MSVC.  I 
> > > > > > don't know of the consequence/standardeeze that causes that though.
> > > > > Thanks, 
> > > > > Follow up stupid question then, do we care about the triviality of 
> > > > > dependant types?
> > > > > I think doing the check on complete non-dependant types might be a 
> > > > > better solution than trying to do it lazily on triviality checks?
> > > > I would think it would be not a problem on non-dependent types.  BUT 
> > > > concepts are only allowed on templated functions (note not only on 
> > > > function-templates!) anyway, so I don't think this would be a problem?  
> > > Erich, I'm a bit confused by your response. I think my example 
> > > demonstrates that for default constructors (and other SMFs) GCC and MSVC 
> > > instantiate the constraints on class completion and **not** on demand. 
> > > This is what I would like to do as well, if we don't have a good reason 
> > > not to. (For destructors, performing the checks is even explicit in the 
> > > standard.)
> > > 
> > > Not doing this can introduce some REALLY bad edge cases. What does this 
> > > do if we defer the triviality computation?
> > > 
> > > ```c++
> > > 
> > > template 
> > > struct Base {
> > >   Base() = default;
> > >   Base() requires (!std::is_trivial_v);
> > > };
> > > 
> > > struct Child : Base { };
> > > ```
> > > We defer the computation of the constraints on `Base`, and complete 
> > > `Child` somehow, but if `Child` is complete then 
> > > `std::is_trivial_v` should be well-formed, right? But we get a 
> > > logical contradiction instead.
> > > 
> > > 
> > >Erich, I'm a bit confused by your response
> > It is entirely possible we're talking past eachother, or misunderstanding 
> > eachothers examples.  I'm totally open to that being part of this issue.
> > 
> > In that example, if we calculate the triviality at '`Base` Class 
> > completion', `Child` is not yet complete, right?  So the is_trivial_v would 
> > be UB.  If you instead require `sizeof(T)`, we typically give a diagnostic.
> > 
> > In this case, you'd at MINIMUM have to wait to calculate the 
> > requires-clause until after `Child` is complete.  And it isn't clear to me 
> > that we're delaying it until then.
> > 
> > That is what I intend to show with: https://godbolt.org/z/1YjsdY73P
> > 
> > As far as doing it 'on demand', I definitely see your circular argument 
> > here, but I think the is_trivial_v test is UB 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

erichkeane wrote:
> royjacobson wrote:
> > erichkeane wrote:
> > > cor3ntin wrote:
> > > > erichkeane wrote:
> > > > > cor3ntin wrote:
> > > > > > royjacobson wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > This seems problematic, doesn't it?  Checking this constraint 
> > > > > > > > will (once I figure out how to get deferred instantiation to 
> > > > > > > > work) cause instantiation, which can cause issues with 
> > > > > > > > incomplete types/CRTP/etc.
> > > > > > > > 
> > > > > > > > I think the result is that we cannot 'calculate' this until it 
> > > > > > > > is queried, else we will cause incorrect errors.
> > > > > > > Making this queried on demand is a relatively big change to how 
> > > > > > > we handle type triviality, so I want to be sure we actually need 
> > > > > > > to do this to be conformant.
> > > > > > > 
> > > > > > > When I started working on this I checked what GCC does and it 
> > > > > > > instantiates those constraints during class completion as well. 
> > > > > > > For example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC 
> > > > > > > seem to do it as well.
> > > > > > > 
> > > > > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > > > > 
> > > > > > I think this is done on completeness already in this patch, unless 
> > > > > > i misunderstood the code.
> > > > > > I don't think doing it on demand is a great direction, as this does 
> > > > > > not only affect type traits but also code gen, etc. It would create 
> > > > > > instanciations in unexpected places. wouldn't it.
> > > > > > Does the standard has wording suggesting it should be done later 
> > > > > > than on type completeness?
> > > > > The problem, at least for the deferred concepts, is that it breaks in 
> > > > > the CRTP as required to implement ranges.  So something like this: 
> > > > > https://godbolt.org/z/hPqrcqhx5 breaks.
> > > > > 
> > > > > I'm currently working to 'fix' that, so if this patch causes us to 
> > > > > 'check' constraints early, it'll go back to breaking that example.  
> > > > > The example that Roy showed seems to show that it is actually 
> > > > > checking 'delayed' right now (that is, on demand) in GCC/MSVC.  I 
> > > > > don't know of the consequence/standardeeze that causes that though.
> > > > Thanks, 
> > > > Follow up stupid question then, do we care about the triviality of 
> > > > dependant types?
> > > > I think doing the check on complete non-dependant types might be a 
> > > > better solution than trying to do it lazily on triviality checks?
> > > I would think it would be not a problem on non-dependent types.  BUT 
> > > concepts are only allowed on templated functions (note not only on 
> > > function-templates!) anyway, so I don't think this would be a problem?  
> > Erich, I'm a bit confused by your response. I think my example demonstrates 
> > that for default constructors (and other SMFs) GCC and MSVC instantiate the 
> > constraints on class completion and **not** on demand. This is what I would 
> > like to do as well, if we don't have a good reason not to. (For 
> > destructors, performing the checks is even explicit in the standard.)
> > 
> > Not doing this can introduce some REALLY bad edge cases. What does this do 
> > if we defer the triviality computation?
> > 
> > ```c++
> > 
> > template 
> > struct Base {
> >   Base() = default;
> >   Base() requires (!std::is_trivial_v);
> > };
> > 
> > struct Child : Base { };
> > ```
> > We defer the computation of the constraints on `Base`, and complete `Child` 
> > somehow, but if `Child` is complete then `std::is_trivial_v` should 
> > be well-formed, right? But we get a logical contradiction instead.
> > 
> > 
> >Erich, I'm a bit confused by your response
> It is entirely possible we're talking past eachother, or misunderstanding 
> eachothers examples.  I'm totally open to that being part of this issue.
> 
> In that example, if we calculate the triviality at '`Base` Class completion', 
> `Child` is not yet complete, right?  So the is_trivial_v would be UB.  If you 
> instead require `sizeof(T)`, we typically give a diagnostic.
> 
> In this case, you'd at MINIMUM have to wait to calculate the requires-clause 
> until after `Child` is complete.  And it isn't clear to me that we're 
> delaying it until then.
> 
> That is what I intend to show with: https://godbolt.org/z/1YjsdY73P
> 
> As far as doing it 'on demand', I definitely see your circular argument here, 
> but I think the is_trivial_v test is UB if we calculate it at `Base' 
> completion.
I'm arguing for checking the constraints at the completion of `Base`, and for 
making `is_trivial_v/sizeof` ill formed in this context.

Your GCC example is a bit 

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

royjacobson wrote:
> erichkeane wrote:
> > cor3ntin wrote:
> > > erichkeane wrote:
> > > > cor3ntin wrote:
> > > > > royjacobson wrote:
> > > > > > erichkeane wrote:
> > > > > > > This seems problematic, doesn't it?  Checking this constraint 
> > > > > > > will (once I figure out how to get deferred instantiation to 
> > > > > > > work) cause instantiation, which can cause issues with incomplete 
> > > > > > > types/CRTP/etc.
> > > > > > > 
> > > > > > > I think the result is that we cannot 'calculate' this until it is 
> > > > > > > queried, else we will cause incorrect errors.
> > > > > > Making this queried on demand is a relatively big change to how we 
> > > > > > handle type triviality, so I want to be sure we actually need to do 
> > > > > > this to be conformant.
> > > > > > 
> > > > > > When I started working on this I checked what GCC does and it 
> > > > > > instantiates those constraints during class completion as well. For 
> > > > > > example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC seem 
> > > > > > to do it as well.
> > > > > > 
> > > > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > > > 
> > > > > I think this is done on completeness already in this patch, unless i 
> > > > > misunderstood the code.
> > > > > I don't think doing it on demand is a great direction, as this does 
> > > > > not only affect type traits but also code gen, etc. It would create 
> > > > > instanciations in unexpected places. wouldn't it.
> > > > > Does the standard has wording suggesting it should be done later than 
> > > > > on type completeness?
> > > > The problem, at least for the deferred concepts, is that it breaks in 
> > > > the CRTP as required to implement ranges.  So something like this: 
> > > > https://godbolt.org/z/hPqrcqhx5 breaks.
> > > > 
> > > > I'm currently working to 'fix' that, so if this patch causes us to 
> > > > 'check' constraints early, it'll go back to breaking that example.  The 
> > > > example that Roy showed seems to show that it is actually checking 
> > > > 'delayed' right now (that is, on demand) in GCC/MSVC.  I don't know of 
> > > > the consequence/standardeeze that causes that though.
> > > Thanks, 
> > > Follow up stupid question then, do we care about the triviality of 
> > > dependant types?
> > > I think doing the check on complete non-dependant types might be a better 
> > > solution than trying to do it lazily on triviality checks?
> > I would think it would be not a problem on non-dependent types.  BUT 
> > concepts are only allowed on templated functions (note not only on 
> > function-templates!) anyway, so I don't think this would be a problem?  
> Erich, I'm a bit confused by your response. I think my example demonstrates 
> that for default constructors (and other SMFs) GCC and MSVC instantiate the 
> constraints on class completion and **not** on demand. This is what I would 
> like to do as well, if we don't have a good reason not to. (For destructors, 
> performing the checks is even explicit in the standard.)
> 
> Not doing this can introduce some REALLY bad edge cases. What does this do if 
> we defer the triviality computation?
> 
> ```c++
> 
> template 
> struct Base {
>   Base() = default;
>   Base() requires (!std::is_trivial_v);
> };
> 
> struct Child : Base { };
> ```
> We defer the computation of the constraints on `Base`, and complete `Child` 
> somehow, but if `Child` is complete then `std::is_trivial_v` should be 
> well-formed, right? But we get a logical contradiction instead.
> 
> 
>Erich, I'm a bit confused by your response
It is entirely possible we're talking past eachother, or misunderstanding 
eachothers examples.  I'm totally open to that being part of this issue.

In that example, if we calculate the triviality at '`Base` Class completion', 
`Child` is not yet complete, right?  So the is_trivial_v would be UB.  If you 
instead require `sizeof(T)`, we typically give a diagnostic.

In this case, you'd at MINIMUM have to wait to calculate the requires-clause 
until after `Child` is complete.  And it isn't clear to me that we're delaying 
it until then.

That is what I intend to show with: https://godbolt.org/z/1YjsdY73P

As far as doing it 'on demand', I definitely see your circular argument here, 
but I think the is_trivial_v test is UB if we calculate it at `Base' completion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

erichkeane wrote:
> cor3ntin wrote:
> > erichkeane wrote:
> > > cor3ntin wrote:
> > > > royjacobson wrote:
> > > > > erichkeane wrote:
> > > > > > This seems problematic, doesn't it?  Checking this constraint will 
> > > > > > (once I figure out how to get deferred instantiation to work) cause 
> > > > > > instantiation, which can cause issues with incomplete 
> > > > > > types/CRTP/etc.
> > > > > > 
> > > > > > I think the result is that we cannot 'calculate' this until it is 
> > > > > > queried, else we will cause incorrect errors.
> > > > > Making this queried on demand is a relatively big change to how we 
> > > > > handle type triviality, so I want to be sure we actually need to do 
> > > > > this to be conformant.
> > > > > 
> > > > > When I started working on this I checked what GCC does and it 
> > > > > instantiates those constraints during class completion as well. For 
> > > > > example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC seem to 
> > > > > do it as well.
> > > > > 
> > > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > > 
> > > > I think this is done on completeness already in this patch, unless i 
> > > > misunderstood the code.
> > > > I don't think doing it on demand is a great direction, as this does not 
> > > > only affect type traits but also code gen, etc. It would create 
> > > > instanciations in unexpected places. wouldn't it.
> > > > Does the standard has wording suggesting it should be done later than 
> > > > on type completeness?
> > > The problem, at least for the deferred concepts, is that it breaks in the 
> > > CRTP as required to implement ranges.  So something like this: 
> > > https://godbolt.org/z/hPqrcqhx5 breaks.
> > > 
> > > I'm currently working to 'fix' that, so if this patch causes us to 
> > > 'check' constraints early, it'll go back to breaking that example.  The 
> > > example that Roy showed seems to show that it is actually checking 
> > > 'delayed' right now (that is, on demand) in GCC/MSVC.  I don't know of 
> > > the consequence/standardeeze that causes that though.
> > Thanks, 
> > Follow up stupid question then, do we care about the triviality of 
> > dependant types?
> > I think doing the check on complete non-dependant types might be a better 
> > solution than trying to do it lazily on triviality checks?
> I would think it would be not a problem on non-dependent types.  BUT concepts 
> are only allowed on templated functions (note not only on 
> function-templates!) anyway, so I don't think this would be a problem?  
Erich, I'm a bit confused by your response. I think my example demonstrates 
that for default constructors (and other SMFs) GCC and MSVC instantiate the 
constraints on class completion and **not** on demand. This is what I would 
like to do as well, if we don't have a good reason not to. (For destructors, 
performing the checks is even explicit in the standard.)

Not doing this can introduce some REALLY bad edge cases. What does this do if 
we defer the triviality computation?

```c++

template 
struct Base {
  Base() = default;
  Base() requires (!std::is_trivial_v);
};

struct Child : Base { };
```
We defer the computation of the constraints on `Base`, and complete `Child` 
somehow, but if `Child` is complete then `std::is_trivial_v` should be 
well-formed, right? But we get a logical contradiction instead.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

cor3ntin wrote:
> erichkeane wrote:
> > cor3ntin wrote:
> > > royjacobson wrote:
> > > > erichkeane wrote:
> > > > > This seems problematic, doesn't it?  Checking this constraint will 
> > > > > (once I figure out how to get deferred instantiation to work) cause 
> > > > > instantiation, which can cause issues with incomplete types/CRTP/etc.
> > > > > 
> > > > > I think the result is that we cannot 'calculate' this until it is 
> > > > > queried, else we will cause incorrect errors.
> > > > Making this queried on demand is a relatively big change to how we 
> > > > handle type triviality, so I want to be sure we actually need to do 
> > > > this to be conformant.
> > > > 
> > > > When I started working on this I checked what GCC does and it 
> > > > instantiates those constraints during class completion as well. For 
> > > > example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC seem to 
> > > > do it as well.
> > > > 
> > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > 
> > > I think this is done on completeness already in this patch, unless i 
> > > misunderstood the code.
> > > I don't think doing it on demand is a great direction, as this does not 
> > > only affect type traits but also code gen, etc. It would create 
> > > instanciations in unexpected places. wouldn't it.
> > > Does the standard has wording suggesting it should be done later than on 
> > > type completeness?
> > The problem, at least for the deferred concepts, is that it breaks in the 
> > CRTP as required to implement ranges.  So something like this: 
> > https://godbolt.org/z/hPqrcqhx5 breaks.
> > 
> > I'm currently working to 'fix' that, so if this patch causes us to 'check' 
> > constraints early, it'll go back to breaking that example.  The example 
> > that Roy showed seems to show that it is actually checking 'delayed' right 
> > now (that is, on demand) in GCC/MSVC.  I don't know of the 
> > consequence/standardeeze that causes that though.
> Thanks, 
> Follow up stupid question then, do we care about the triviality of dependant 
> types?
> I think doing the check on complete non-dependant types might be a better 
> solution than trying to do it lazily on triviality checks?
I would think it would be not a problem on non-dependent types.  BUT concepts 
are only allowed on templated functions (note not only on function-templates!) 
anyway, so I don't think this would be a problem?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

erichkeane wrote:
> cor3ntin wrote:
> > royjacobson wrote:
> > > erichkeane wrote:
> > > > This seems problematic, doesn't it?  Checking this constraint will 
> > > > (once I figure out how to get deferred instantiation to work) cause 
> > > > instantiation, which can cause issues with incomplete types/CRTP/etc.
> > > > 
> > > > I think the result is that we cannot 'calculate' this until it is 
> > > > queried, else we will cause incorrect errors.
> > > Making this queried on demand is a relatively big change to how we handle 
> > > type triviality, so I want to be sure we actually need to do this to be 
> > > conformant.
> > > 
> > > When I started working on this I checked what GCC does and it 
> > > instantiates those constraints during class completion as well. For 
> > > example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC seem to do 
> > > it as well.
> > > 
> > > So maybe it's OK to check the constraints of SMFs specifically?
> > > 
> > I think this is done on completeness already in this patch, unless i 
> > misunderstood the code.
> > I don't think doing it on demand is a great direction, as this does not 
> > only affect type traits but also code gen, etc. It would create 
> > instanciations in unexpected places. wouldn't it.
> > Does the standard has wording suggesting it should be done later than on 
> > type completeness?
> The problem, at least for the deferred concepts, is that it breaks in the 
> CRTP as required to implement ranges.  So something like this: 
> https://godbolt.org/z/hPqrcqhx5 breaks.
> 
> I'm currently working to 'fix' that, so if this patch causes us to 'check' 
> constraints early, it'll go back to breaking that example.  The example that 
> Roy showed seems to show that it is actually checking 'delayed' right now 
> (that is, on demand) in GCC/MSVC.  I don't know of the 
> consequence/standardeeze that causes that though.
Thanks, 
Follow up stupid question then, do we care about the triviality of dependant 
types?
I think doing the check on complete non-dependant types might be a better 
solution than trying to do it lazily on triviality checks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

cor3ntin wrote:
> royjacobson wrote:
> > erichkeane wrote:
> > > This seems problematic, doesn't it?  Checking this constraint will (once 
> > > I figure out how to get deferred instantiation to work) cause 
> > > instantiation, which can cause issues with incomplete types/CRTP/etc.
> > > 
> > > I think the result is that we cannot 'calculate' this until it is 
> > > queried, else we will cause incorrect errors.
> > Making this queried on demand is a relatively big change to how we handle 
> > type triviality, so I want to be sure we actually need to do this to be 
> > conformant.
> > 
> > When I started working on this I checked what GCC does and it instantiates 
> > those constraints during class completion as well. For example this CRTP 
> > case: https://godbolt.org/z/EdoYf96zq. MSVC seem to do it as well.
> > 
> > So maybe it's OK to check the constraints of SMFs specifically?
> > 
> I think this is done on completeness already in this patch, unless i 
> misunderstood the code.
> I don't think doing it on demand is a great direction, as this does not only 
> affect type traits but also code gen, etc. It would create instanciations in 
> unexpected places. wouldn't it.
> Does the standard has wording suggesting it should be done later than on type 
> completeness?
The problem, at least for the deferred concepts, is that it breaks in the CRTP 
as required to implement ranges.  So something like this: 
https://godbolt.org/z/hPqrcqhx5 breaks.

I'm currently working to 'fix' that, so if this patch causes us to 'check' 
constraints early, it'll go back to breaking that example.  The example that 
Roy showed seems to show that it is actually checking 'delayed' right now (that 
is, on demand) in GCC/MSVC.  I don't know of the consequence/standardeeze that 
causes that though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

royjacobson wrote:
> erichkeane wrote:
> > This seems problematic, doesn't it?  Checking this constraint will (once I 
> > figure out how to get deferred instantiation to work) cause instantiation, 
> > which can cause issues with incomplete types/CRTP/etc.
> > 
> > I think the result is that we cannot 'calculate' this until it is queried, 
> > else we will cause incorrect errors.
> Making this queried on demand is a relatively big change to how we handle 
> type triviality, so I want to be sure we actually need to do this to be 
> conformant.
> 
> When I started working on this I checked what GCC does and it instantiates 
> those constraints during class completion as well. For example this CRTP 
> case: https://godbolt.org/z/EdoYf96zq. MSVC seem to do it as well.
> 
> So maybe it's OK to check the constraints of SMFs specifically?
> 
I think this is done on completeness already in this patch, unless i 
misunderstood the code.
I don't think doing it on demand is a great direction, as this does not only 
affect type traits but also code gen, etc. It would create instanciations in 
unexpected places. wouldn't it.
Does the standard has wording suggesting it should be done later than on type 
completeness?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 446172.
royjacobson added a comment.

Fixed the tests, fixed the type comparison checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 15
   
   
 https://wg21.link/p1616r1;>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,195 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+// expected-no-diagnostics
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<0>));
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<2>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<3>));

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17870
+///   parameter type and the same cv-qualifiers and ref-qualifier, if any.
+bool AreSpecialMemberFunctionsSameKind(CXXMethodDecl *M1, CXXMethodDecl *M2,
+ Sema::CXXSpecialMember CSM) {

cor3ntin wrote:
> 
It's in an anonymous namespace. Is it a Clang convention to use static 
functions instead? I saw both used in Sema, I think



Comment at: clang/lib/Sema/SemaDecl.cpp:17874
+  return true;
+if (M1->getParamDecl(0)->getType() != M2->getParamDecl(0)->getType())
+  return false;

erichkeane wrote:
> I don't think you'd want to compare with equality here, it ends up not 
> desugaring/etc.  I believe `ASTContext::HasSameType` should be what you want. 
>  
Good catch! added a test for this.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

erichkeane wrote:
> This seems problematic, doesn't it?  Checking this constraint will (once I 
> figure out how to get deferred instantiation to work) cause instantiation, 
> which can cause issues with incomplete types/CRTP/etc.
> 
> I think the result is that we cannot 'calculate' this until it is queried, 
> else we will cause incorrect errors.
Making this queried on demand is a relatively big change to how we handle type 
triviality, so I want to be sure we actually need to do this to be conformant.

When I started working on this I checked what GCC does and it instantiates 
those constraints during class completion as well. For example this CRTP case: 
https://godbolt.org/z/EdoYf96zq. MSVC seem to do it as well.

So maybe it's OK to check the constraints of SMFs specifically?




Comment at: clang/lib/Sema/SemaDecl.cpp:17960-17961
+}
+if (AnotherMethodIsMoreConstrained)
+  break;
+  }

cor3ntin wrote:
> royjacobson wrote:
> > cor3ntin wrote:
> > > Is that codepath ever taken?
> > > I wonder if there is a way to do that that is not n^2. Maybe not.
> > It's not, I didn't mean to leave it there. Thanks for the catch :)
> > 
> > 
> > Generally finding a maximal set in a partial ordering is O(n^2) in the 
> > worst case which is when no two objects are comparable.
> > 
> > We could optimize this a bit for some cases: We can group the functions by 
> > their kind, and we can skip comparing functions if we already know that 
> > they are not maximal. But since the number of SMFs expected in a class is 
> > very (1-3) small, I didn't think those possible improvements are worth the 
> > extra complexity.
> > 
> > Another possible optimization is we could short-circuit this evaluation if 
> > we know that a type is not trivially [copyable], since that's the only 
> > outcome we really care about, but then the AST is left in a very awkward 
> > state.
> > 
> Thanks, I guess this is fine, I cannot really think of a better way either
Ok, did another look on this and it's because `AnotherMethodIsMoreConstrained` 
is the output of `IsAtLeastAsConstrained`, so that's why the extra check. Added 
it back :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I believe the calculation of the constraints here means this isn't conforming 
with concepts.  I believe this means we have to delay calculating the 
constraint differences until this property is actually queried, and cannot do 
it early like this has.




Comment at: clang/lib/Sema/SemaDecl.cpp:17874
+  return true;
+if (M1->getParamDecl(0)->getType() != M2->getParamDecl(0)->getType())
+  return false;

I don't think you'd want to compare with equality here, it ends up not 
desugaring/etc.  I believe `ASTContext::HasSameType` should be what you want.  



Comment at: clang/lib/Sema/SemaDecl.cpp:17876
+  return false;
+if (M1->getThisType() != M2->getThisType())
+  return false;

Probably same here.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

This seems problematic, doesn't it?  Checking this constraint will (once I 
figure out how to get deferred instantiation to work) cause instantiation, 
which can cause issues with incomplete types/CRTP/etc.

I think the result is that we cannot 'calculate' this until it is queried, else 
we will cause incorrect errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

@aaron.ballman @erichkeane I think this should be on your radar, it would be 
great to have it in 15. I did review it and I think it looks good but there are 
enough dragons here that i'd rather have more eye balls on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17870
+///   parameter type and the same cv-qualifiers and ref-qualifier, if any.
+bool AreSpecialMemberFunctionsSameKind(CXXMethodDecl *M1, CXXMethodDecl *M2,
+ Sema::CXXSpecialMember CSM) {





Comment at: clang/lib/Sema/SemaDecl.cpp:17888
+///   [CWG2595], if any, are satisfied is more constrained.
+void SetEligibleMethods(Sema , CXXRecordDecl* Record,
+ArrayRef Methods,





Comment at: clang/lib/Sema/SemaDecl.cpp:17960-17961
+}
+if (AnotherMethodIsMoreConstrained)
+  break;
+  }

royjacobson wrote:
> cor3ntin wrote:
> > Is that codepath ever taken?
> > I wonder if there is a way to do that that is not n^2. Maybe not.
> It's not, I didn't mean to leave it there. Thanks for the catch :)
> 
> 
> Generally finding a maximal set in a partial ordering is O(n^2) in the worst 
> case which is when no two objects are comparable.
> 
> We could optimize this a bit for some cases: We can group the functions by 
> their kind, and we can skip comparing functions if we already know that they 
> are not maximal. But since the number of SMFs expected in a class is very 
> (1-3) small, I didn't think those possible improvements are worth the extra 
> complexity.
> 
> Another possible optimization is we could short-circuit this evaluation if we 
> know that a type is not trivially [copyable], since that's the only outcome 
> we really care about, but then the AST is left in a very awkward state.
> 
Thanks, I guess this is fine, I cannot really think of a better way either


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:494-497
+- Implemented "Conditionally Trivial Special Member Functions" (`P0848 
`_).
+  Note: The handling of deleted functions is not yet compliant, as Clang
+  does not implement `DR1496 
`_
+  and `DR1734 
`_.

Is that lack of compliance worth a note in `cxx_status`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 445350.
royjacobson marked an inline comment as not done.
royjacobson added a comment.

Add a test case with more subsumption


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 15
   
   
 https://wg21.link/p1616r1;>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,186 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+// expected-no-diagnostics
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<0>));
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<2>));

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson marked an inline comment as done.
royjacobson added inline comments.



Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

BRevzin wrote:
> royjacobson wrote:
> > BRevzin wrote:
> > > It's possible that I just don't understand what these tests actually mean 
> > > but... where is the test for `DefaultConstructorCheck<2>` having a 
> > > deleted default constructor, or `DefaultConstructorCheck<3>` having a 
> > > non-defaulted one?
> > > 
> > > It'd also be worthwhile to have at least one test with constaints that 
> > > subsume each other instead of being mutually exclusive. 
> > Should I check this? Except destructors, the other SMFs are found during 
> > overload resolution using the usual lookup that already takes into account 
> > delete/default/constraints etc.
> > 
> > This patch is about making sure that we set the triviality attributes 
> > correctly according to the eligible functions, so this is what I added 
> > tests for.
> > 
> > Most of this testing is done in the sema test, but I need this AST test as 
> > well to make sure we get the `canPassInRegisters` attribute correctly - 
> > we're doing some custom processing over the functions without the usual 
> > type attributes because there are some weird compatibility edge cases.
> > 
> One of the motivations for the paper is to ensure that like given:
> 
> ```
> template 
> struct optional {
> optional(optional const&) requires copyable && trivially_copyableT> = 
> default;
> optional(optional const&) requires copyable;
> };
> ```
> 
> `optional` is copyable (but not trivially copyable), `optional` 
> is trivially copyable, and `optional>` isn't copyable. I'm 
> not sure what in here checks if that works. 
That's more-or-less the check in `constrained-special-member-functions.cpp:50`, 
I think.

Didn't acknowledge it in my first response - but yeah, I need some more 
complicated subsumption test cases



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Barry Revzin via Phabricator via cfe-commits]

BRevzin added inline comments.



Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

royjacobson wrote:
> BRevzin wrote:
> > It's possible that I just don't understand what these tests actually mean 
> > but... where is the test for `DefaultConstructorCheck<2>` having a deleted 
> > default constructor, or `DefaultConstructorCheck<3>` having a non-defaulted 
> > one?
> > 
> > It'd also be worthwhile to have at least one test with constaints that 
> > subsume each other instead of being mutually exclusive. 
> Should I check this? Except destructors, the other SMFs are found during 
> overload resolution using the usual lookup that already takes into account 
> delete/default/constraints etc.
> 
> This patch is about making sure that we set the triviality attributes 
> correctly according to the eligible functions, so this is what I added tests 
> for.
> 
> Most of this testing is done in the sema test, but I need this AST test as 
> well to make sure we get the `canPassInRegisters` attribute correctly - we're 
> doing some custom processing over the functions without the usual type 
> attributes because there are some weird compatibility edge cases.
> 
One of the motivations for the paper is to ensure that like given:

```
template 
struct optional {
optional(optional const&) requires copyable && trivially_copyableT> = 
default;
optional(optional const&) requires copyable;
};
```

`optional` is copyable (but not trivially copyable), `optional` is 
trivially copyable, and `optional>` isn't copyable. I'm not 
sure what in here checks if that works. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson marked 4 inline comments as done and an inline comment as not done.
royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17960-17961
+}
+if (AnotherMethodIsMoreConstrained)
+  break;
+  }

cor3ntin wrote:
> Is that codepath ever taken?
> I wonder if there is a way to do that that is not n^2. Maybe not.
It's not, I didn't mean to leave it there. Thanks for the catch :)


Generally finding a maximal set in a partial ordering is O(n^2) in the worst 
case which is when no two objects are comparable.

We could optimize this a bit for some cases: We can group the functions by 
their kind, and we can skip comparing functions if we already know that they 
are not maximal. But since the number of SMFs expected in a class is very (1-3) 
small, I didn't think those possible improvements are worth the extra 
complexity.

Another possible optimization is we could short-circuit this evaluation if we 
know that a type is not trivially [copyable], since that's the only outcome we 
really care about, but then the AST is left in a very awkward state.




Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

BRevzin wrote:
> It's possible that I just don't understand what these tests actually mean 
> but... where is the test for `DefaultConstructorCheck<2>` having a deleted 
> default constructor, or `DefaultConstructorCheck<3>` having a non-defaulted 
> one?
> 
> It'd also be worthwhile to have at least one test with constaints that 
> subsume each other instead of being mutually exclusive. 
Should I check this? Except destructors, the other SMFs are found during 
overload resolution using the usual lookup that already takes into account 
delete/default/constraints etc.

This patch is about making sure that we set the triviality attributes correctly 
according to the eligible functions, so this is what I added tests for.

Most of this testing is done in the sema test, but I need this AST test as well 
to make sure we get the `canPassInRegisters` attribute correctly - we're doing 
some custom processing over the functions without the usual type attributes 
because there are some weird compatibility edge cases.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 445044.
royjacobson edited the summary of this revision.
royjacobson added a comment.

Address Corentin's feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 15
   
   
 https://wg21.link/p1616r1;>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+// expected-no-diagnostics
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<0>));
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<2>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<3>));

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Barry Revzin via Phabricator via cfe-commits]

BRevzin added inline comments.



Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

It's possible that I just don't understand what these tests actually mean 
but... where is the test for `DefaultConstructorCheck<2>` having a deleted 
default constructor, or `DefaultConstructorCheck<3>` having a non-defaulted one?

It'd also be worthwhile to have at least one test with constaints that subsume 
each other instead of being mutually exclusive. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

Thanks for working on that!
The patch looks very good overall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17863-17869
+/// Assuming the suggested solution to CWG2595 gets accepted:
+/// [class.mem.special]p6:
+/// An eligible special member function is a special member function for which:
+/// - the function is not deleted,
+/// - the associated constraints, if any, are satisfied, and
+/// - no special member function of the same kind whose associated constraints,
+///   if any, are satisfied is more constrained.





Comment at: clang/lib/Sema/SemaDecl.cpp:17916
+
+  auto SetEligibleMethods = [, ,
+ AreSameKind](ArrayRef Methods,

That could be a separate (static) function



Comment at: clang/lib/Sema/SemaDecl.cpp:17937
+continue;
+  auto *MethodI = Methods[i];
+  const Expr *ConstraintsI = MethodI->getTrailingRequiresClause();

You should probably be explicit about the type here.



Comment at: clang/lib/Sema/SemaDecl.cpp:17960-17961
+}
+if (AnotherMethodIsMoreConstrained)
+  break;
+  }

Is that codepath ever taken?
I wonder if there is a way to do that that is not n^2. Maybe not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson created this revision.
Herald added a project: All.
royjacobson updated this revision to Diff 444173.
royjacobson added a comment.
royjacobson updated this revision to Diff 19.
royjacobson retitled this revision from "[WIP][Clang] Implement P0848 
(Conditionally Trivial Special Member Functions)" to "[Clang] Implement P0848 
(Conditionally Trivial Special Member Functions)".
royjacobson edited the summary of this revision.
royjacobson added reviewers: clang-language-wg, erichkeane, cor3ntin, BRevzin, 
CaseyCarter, aaron.ballman.
royjacobson published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The implementation is more or less complete, but I still need to add an AST 
test for canPassInRegisters.


royjacobson added a comment.

Implementation ready for CR.




Comment at: clang/www/cxx_status.html:930
 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 15
   

Should be `class="unreleased"`, I believe



Comment at: clang/www/cxx_status.html:930
 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 15
   

h-vetinari wrote:
> Should be `class="unreleased"`, I believe
Thanks, nice catch!


This patch implements P0848 in Clang.

During the instantiation of a C++ class, in `Sema::ActOnFields`, we evaluate 
constraints for all the SMFs and compare the constraints to compute the 
eligibility. We defer the computation of the type's [copy-]trivial bits from 
addedMember to the eligibility computation, like we did for destructors in 
D126194 . `canPassInRegisters` is modified as 
well to better respect the ineligibility of functions.

Note: Because of the non-implementation of DR1734 and DR1496, I treat deleted 
member functions as 'eligible' for the purpose of [copy-]triviallity. This is 
unfortunate, but I couldn't think of a way to make this make sense otherwise.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3;>P0848R3
-No
+Clang 15
   
   
 https://wg21.link/p1616r1;>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+// expected-no-diagnostics
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));