aaron.ballman added inline comments.
================ Comment at: clang/docs/ReleaseNotes.rst:91-92 expressions and how their types are deduced therein, in all C++ language versions. +- Implemented partial support for `P2448R2: Relaxing some constexpr restrictions <https://wg21.link/p2448r2>`_ + Explicitly defaulted functions no longer have to be constexpr-compatible but merely constexpr suitable. ---------------- Should we also say what's still missing that causes this to be partial? ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9415-9421 + "invokes a non-constexpr comparison function is a C++2b extension">, InGroup<CXX2bDefCompCallsNonConstexpr>; +def warn_cxx2b_compat_incorrect_defaulted_comparison_constexpr : Warning< + "defaulted definition of %select{%sub{select_defaulted_comparison_kind}1|" + "three-way comparison operator}0 that is " + "declared %select{constexpr|consteval}2 but" + "%select{|for which the corresponding implicit 'operator==' }0 " + "invokes a non-constexpr comparison function is a C++2b extension">, InGroup<CXXPre2bCompat>, DefaultIgnore; ---------------- Formatting fix-up and a wording correction. ================ 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< ---------------- shafik wrote: > rsmith wrote: > > 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. > @aaron.ballman any suggestions for less awkward wording here? I don't think > what there is here now is too bad but asking for a another opinion. Yeah, that wording is a bit... wordy, but I'm struggling to think of something more approachable that's still accurate. ================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8811 + // The concept of constexpr-compatible was removed. + // C++23 [dcl.fct.def.default]/p3 [P2448R2] + // A function explicitly defaulted on its first declaration is implicitly ---------------- ================ Comment at: clang/test/CXX/class/class.compare/class.compare.default/p4.cpp:153 + + constexpr friend bool operator==(const my_struct &, const my_struct &) noexcept = default; // no diagnostic extension-warning {{declared constexpr but invokes a non-constexpr comparison function is a C++2b extension}} +}; ---------------- `no diagnostic extension-warning`? ================ Comment at: clang/test/CXX/class/class.compare/class.compare.default/p4.cpp:162 + +my_struct<non_constexpr_type> obj; // extension-note {{in instantiation of template class 'GH61238::my_struct<GH61238::non_constexpr_type>' requested here}} +} ---------------- Can you add an instantiation that does use a constexpr suitable type to show that we don't issue the diagnostic on that instantiation, or do you think that is sufficiently covered by other test coverage? ================ Comment at: clang/www/cxx_status.html:1485 <td><a href="https://wg21.link/P2448R2">P2448R2</a></td> - <td class="none" align="center">No</td> + <td class="none" align="center">Partial</td> </tr> ---------------- Please add <details> markup to explain why the support is partial, and it's probably not a bad idea to say `Clang 17 (Partial)` so users know when the partial support started. 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