Skip to content
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.

Sign up for GitHub

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

Jump to bottom

No impl of Ord for RefCell<T> or Cell<T> when T: Ord #33305

Closed
vadixidav opened this issue May 1, 2016 · 4 comments
Closed

No impl of Ord for RefCell<T> or Cell<T> when T: Ord #33305

vadixidav opened this issue May 1, 2016 · 4 comments

Comments

@vadixidav
Copy link
Contributor

Title says it all. I am working on it right now. Do I need to create an RFC or can I just send a PR?
@vadixidav vadixidav mentioned this issue May 1, 2016
Merged
@sfackler
Copy link
Member

sfackler commented May 1, 2016

The implementation for RefCell is not super workable since it would have to either panic or return nonsensical results if the cell is borrowed mutably at the time. An impl for Cell seems like it should be fine though.

@Diggsey
Copy link
Contributor

Diggsey commented May 3, 2016

What's the problem with panicking? That's what borrow() does anyway if there's an outstanding mutable borrow. Given that this is single-threaded, it would be a program error anyway.

@Aatch
Copy link
Contributor

Aatch commented May 3, 2016

@sfackler there is an impl of PartialEq for RefCell<T: PartialEq>, so PartialOrd/Ord should behave the same way anyway.

bors added a commit that referenced this issue May 12, 2016
@bors
Auto merge of #33306 - vadixidav:master, r=alexcrichton
992bb13 
add implementation of Ord for Cell<T> and RefCell<T> where T: Ord

Raised this in issue #33305.
@tbu-
Copy link
Contributor

tbu- commented May 13, 2016

Fixed by #33306.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@alexcrichton @Aatch @Diggsey @sfackler @tbu- @vadixidav