-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix indirect comparisons #489
base: dev-jbcoe-no-deferred-requirements
Are you sure you want to change the base?
Fix indirect comparisons #489
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev-jbcoe-no-deferred-requirements #489 +/- ##
===================================================================
Coverage 99.55% 99.55%
===================================================================
Files 7 7
Lines 669 669
Branches 75 75
===================================================================
Hits 666 666
Misses 3 3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good.
@@ -867,12 +869,12 @@ class indirect { | |||
template <class U, class AA> | |||
friend constexpr auto operator<=>( | |||
const indirect& lhs, const indirect<U, AA>& rhs) noexcept(see below) | |||
-> compare_three_way_result_t<T, U>; | |||
-> synth-three-way-result<T, U>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we had previously made this change in the LWG session in Tokyo. This is want we want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the constraints here seems fine to me. Given we return synth-three-way-result
I'm not sure what we get by additionally specifying it as convertible to bool.
Should it become a “mandates” or should we lose it altogether. I’m not sure that there is existing precedent in the library. |
If we want to be consistent with other types in the standard then looking at the equivalent in |
Do not merge until #478 is submitted.