[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 &S, CXXRecordDecl* Record,
+                        ArrayRef<CXXMethodDecl *> 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
         <td><a href="https://wg21.link/p0848r3">P0848R3</a></td>
-        <td rowspan="1" class="none" align="center">No</td>
+        <td rowspan="1" class="unreleased" align="center">Clang 16 <a href="#p0848">(12)</a></td>
       </tr>
----------------
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