From 2a4ece65b1229eb8733194033052de1d71792c5e Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Sun, 18 Aug 2024 17:09:40 +0200 Subject: [PATCH 1/2] [libc++][chono] Use hidden friends for leap_second comparison. The function template requires three_way_comparable_with> constexpr auto operator<=>(const leap_second& x, const sys_time& y) noexcept; Has a recursive constrained. This caused an infinite loop in GCC and is now hit by https://github.com/llvm/llvm-project/pull/102857. A fix would be to make this function a hidden friend, this solution is propsed in LWG4139. For consistency all comparisons are made hidden friends. Since the issue causes compilation failures no additional test are needed. Fixes: https://github.com/llvm/llvm-project/issues/104700 --- libcxx/docs/Status/Cxx2cIssues.csv | 1 + libcxx/include/__chrono/leap_second.h | 131 +++++++++++++------------- 2 files changed, 69 insertions(+), 63 deletions(-) diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv index 60c6dc532dc71..3e49738443db5 100644 --- a/libcxx/docs/Status/Cxx2cIssues.csv +++ b/libcxx/docs/Status/Cxx2cIssues.csv @@ -77,4 +77,5 @@ "`LWG4106 `__","``basic_format_args`` should not be default-constructible","2024-06 (St. Louis)","|Complete|","19.0","|format|" "","","","","","" "`LWG3343 `__","Ordering of calls to ``unlock()`` and ``notify_all()`` in Effects element of ``notify_all_at_thread_exit()`` should be reversed","Not Adopted Yet","|Complete|","16.0","" +"`LWG3343 `__","§[time.zone.leap] recursive constraint in <=>","Not Adopted Yet","|Complete|","20.0","" "","","","","","" diff --git a/libcxx/include/__chrono/leap_second.h b/libcxx/include/__chrono/leap_second.h index 1a0e7f3107de8..d79111ed8eecf 100644 --- a/libcxx/include/__chrono/leap_second.h +++ b/libcxx/include/__chrono/leap_second.h @@ -50,70 +50,75 @@ class leap_second { private: sys_seconds __date_; seconds __value_; -}; -_LIBCPP_HIDE_FROM_ABI inline constexpr bool operator==(const leap_second& __x, const leap_second& __y) { - return __x.date() == __y.date(); -} - -_LIBCPP_HIDE_FROM_ABI inline constexpr strong_ordering operator<=>(const leap_second& __x, const leap_second& __y) { - return __x.date() <=> __y.date(); -} - -template -_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(const leap_second& __x, const sys_time<_Duration>& __y) { - return __x.date() == __y; -} - -template -_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const leap_second& __x, const sys_time<_Duration>& __y) { - return __x.date() < __y; -} - -template -_LIBCPP_HIDE_FROM_ABI constexpr bool operator<(const sys_time<_Duration>& __x, const leap_second& __y) { - return __x < __y.date(); -} - -template -_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const leap_second& __x, const sys_time<_Duration>& __y) { - return __y < __x; -} - -template -_LIBCPP_HIDE_FROM_ABI constexpr bool operator>(const sys_time<_Duration>& __x, const leap_second& __y) { - return __y < __x; -} - -template -_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const leap_second& __x, const sys_time<_Duration>& __y) { - return !(__y < __x); -} - -template -_LIBCPP_HIDE_FROM_ABI constexpr bool operator<=(const sys_time<_Duration>& __x, const leap_second& __y) { - return !(__y < __x); -} - -template -_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const leap_second& __x, const sys_time<_Duration>& __y) { - return !(__x < __y); -} - -template -_LIBCPP_HIDE_FROM_ABI constexpr bool operator>=(const sys_time<_Duration>& __x, const leap_second& __y) { - return !(__x < __y); -} - -# ifndef _LIBCPP_COMPILER_GCC -// This requirement cause a compilation loop in GCC-13 and running out of memory. -// TODO TZDB Test whether GCC-14 fixes this. -template - requires three_way_comparable_with> -_LIBCPP_HIDE_FROM_ABI constexpr auto operator<=>(const leap_second& __x, const sys_time<_Duration>& __y) { - return __x.date() <=> __y; -} -# endif + // The function + // template + // requires three_way_comparable_with> + // constexpr auto operator<=>(const leap_second& x, const sys_time& y) noexcept; + // + // Has constraints that are recursive (LWG4139). The proposed resolution is + // to make the funcion a hidden friend. For consistency make this change for + // all comparison functions. + + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator==(const leap_second& __x, const leap_second& __y) { + return __x.date() == __y.date(); + } + + _LIBCPP_HIDE_FROM_ABI friend constexpr strong_ordering operator<=>(const leap_second& __x, const leap_second& __y) { + return __x.date() <=> __y.date(); + } + + template + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator==(const leap_second& __x, const sys_time<_Duration>& __y) { + return __x.date() == __y; + } + + template + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator<(const leap_second& __x, const sys_time<_Duration>& __y) { + return __x.date() < __y; + } + + template + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator<(const sys_time<_Duration>& __x, const leap_second& __y) { + return __x < __y.date(); + } + + template + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator>(const leap_second& __x, const sys_time<_Duration>& __y) { + return __y < __x; + } + + template + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator>(const sys_time<_Duration>& __x, const leap_second& __y) { + return __y < __x; + } + + template + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator<=(const leap_second& __x, const sys_time<_Duration>& __y) { + return !(__y < __x); + } + + template + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator<=(const sys_time<_Duration>& __x, const leap_second& __y) { + return !(__y < __x); + } + + template + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator>=(const leap_second& __x, const sys_time<_Duration>& __y) { + return !(__x < __y); + } + + template + _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator>=(const sys_time<_Duration>& __x, const leap_second& __y) { + return !(__x < __y); + } + + template + requires three_way_comparable_with> + _LIBCPP_HIDE_FROM_ABI friend constexpr auto operator<=>(const leap_second& __x, const sys_time<_Duration>& __y) { + return __x.date() <=> __y; + } +}; } // namespace chrono From e5822ee28119793bbdb1b9b69f1b4a3964a01e13 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Tue, 20 Aug 2024 19:13:01 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: h-vetinari --- libcxx/docs/Status/Cxx2cIssues.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv index 3e49738443db5..be0621caaa9c6 100644 --- a/libcxx/docs/Status/Cxx2cIssues.csv +++ b/libcxx/docs/Status/Cxx2cIssues.csv @@ -77,5 +77,5 @@ "`LWG4106 `__","``basic_format_args`` should not be default-constructible","2024-06 (St. Louis)","|Complete|","19.0","|format|" "","","","","","" "`LWG3343 `__","Ordering of calls to ``unlock()`` and ``notify_all()`` in Effects element of ``notify_all_at_thread_exit()`` should be reversed","Not Adopted Yet","|Complete|","16.0","" -"`LWG3343 `__","§[time.zone.leap] recursive constraint in <=>","Not Adopted Yet","|Complete|","20.0","" +"`LWG4139 `__","§[time.zone.leap] recursive constraint in <=>","Not Adopted Yet","|Complete|","20.0","" "","","","","",""