-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Implement PartialOrd<&B>
for &mut A
#87328
Implement PartialOrd<&B>
for &mut A
#87328
Conversation
r? @scottmcm (rust-highfive has picked a reviewer for you, use r? to override) |
Hmm, but there's no If this were going to be added, then I'd expect both impls to be added. But I'm not sure it's legal to add this, because references are fundamental. (See how But this is an insta-stable impl, so I'll delegate this over to libs-api |
I tried to add it, but it wouldn't build. I presumed, without much thinking about it, that that was broken in some way this wasn't. Silly of me, I guess. ;) These continual trait problems are really frustrating from a stability without stagnation perspective. Not like I know how to fix them though. |
I guess one way works because of coercions but the other doesn't. Hypothesis: if the LHS is a (AKA the classic "inference and coercions don't mix" kind of problem.) |
Hmmm. If that's the problem, maybe the real solution is to tweak the coercion rules somehow. I'll confess that I have no clue how they work or whether that's viable though. |
I'm going to close this. I think we need to do something about this, but this PR is probably too backwards incompatible to actually be worth merging. |
Due to an apparent oversight, the impl
PartialOrd<&B> for &mut A
did not previously exist. This leads to some very weird asymmetry. To quote #65589, "you can't compare &mut i32 with &i32, but you can compare &i32 with &mut i32". This PR adds the missing impl.This is essentially copied from Duddino's work in #65609.
Fixes #65589.