rsmith added a comment.
Please also update the P2448 <https://reviews.llvm.org/P2448> row in
cxx_status.html and add release notes.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9410-9421
+def ext_incorrect_defaulted_comparison_constexpr : Extension<
"defaulted definition of %select{%sub{select_defaulted_comparison_kind}1|"
"three-way comparison operator}0 "
- "cannot be declared %select{constexpr|consteval}2 because "
+ "declared %select{constexpr|consteval}2 but "
"%select{it|the corresponding implicit 'operator=='}0 "
- "invokes a non-constexpr comparison function">;
+ "invokes a non-constexpr comparison function are a C++2b extension">,
InGroup<CXX2bDefCompCallsNonConstexpr>;
+def warn_cxx2b_incorrect_defaulted_comparison_constexpr : Warning<
----------------
The grammar of these diagnostics looks a bit peculiar. Suggested an
alternative, but it's still a bit awkward ("...but for which..."), maybe you
can find something better.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9416
+ "invokes a non-constexpr comparison function are a C++2b extension">,
InGroup<CXX2bDefCompCallsNonConstexpr>;
+def warn_cxx2b_incorrect_defaulted_comparison_constexpr : Warning<
+ "defaulted definition of %select{%sub{select_defaulted_comparison_kind}1|"
----------------
We usually spell these `CXXabCompat` diagnostics `warn_cxxab_compat_...`
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8799-8806
// C++2a [dcl.fct.def.default]p3 [P2002R0]:
// An explicitly-defaulted function that is not defined as deleted may be
// declared constexpr or consteval only if it is constexpr-compatible.
// C++2a [class.compare.default]p3 [P2002R0]:
// A defaulted comparison function is constexpr-compatible if it satisfies
// the requirements for a constexpr function [...]
// The only relevant requirements are that the parameter and return types are
----------------
Would be good to mention the change in C++23 here.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8808-8809
if (FD->isConstexpr()) {
if (CheckConstexprReturnType(*this, FD, CheckConstexprKind::Diagnose) &&
CheckConstexprParameterTypes(*this, FD, CheckConstexprKind::Diagnose)
&&
!Info.Constexpr) {
----------------
We'll also be getting diagnostics here if the parameter and return types are
not literal types. You can pass `...::CheckValid` instead of `...::Diagnose` to
find out if there would be an error, but we should probably instead make the
`CheckConstexpr...` functions produce warnings (`ExtWarn` prior to C++23)
rather than errors in general. That's a much larger change, though -- there's
probably half a dozen diagnostics in the `CheckConstexpr*` functions that will
need to be updated to properly support P2448R2. If you just want to implement
this one part of P2448 for now and leave the rest for a later change, that
seems fine to me.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8813
+ (getLangOpts().CPlusPlus2b ?
diag::warn_cxx2b_incorrect_defaulted_comparison_constexpr :
+
diag::ext_incorrect_defaulted_comparison_constexpr ))
<< FD->isImplicit() << (int)DCK << FD->isConsteval();
----------------
Don't need the parens around this. This layout is a little unusual, what does
clang-format do here?
================
Comment at: clang/test/CXX/class/class.compare/class.compare.default/p3.cpp:132
A b;
- friend constexpr bool operator==(const E&, const E&) = default; //
expected-error {{cannot be declared constexpr because it invokes a
non-constexpr comparison function}}
+ friend constexpr bool operator==(const E&, const E&) = default;
friend constexpr bool operator!=(const E&, const E&) = default;
----------------
Can we test these both ways, like you did in p4?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146090/new/
https://reviews.llvm.org/D146090
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
