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.

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

Added a "same_receiver" method to (Unbounded)Sender #1617

Merged
merged 3 commits into from
May 20, 2019
Merged

Added a "same_receiver" method to (Unbounded)Sender #1617

merged 3 commits into from
May 20, 2019

Conversation

r3v2d0g
Copy link
Contributor

@r3v2d0g r3v2d0g commented May 17, 2019

Hello!
This was asked in #1588 and I wanted to make a first contribution 😄. As proposed by the issue's creator, I used Arc::eq_ptr to compare the inner field of SenderInner. I also added a partial_eq (though, I'm not really sure about its name) same_receiver test which compares a Sender with a clone of it when both are connected to the channel, when one of them is disconnected and when the channel is closed; it also compares two different channels' Senders.

@ghost
Copy link

ghost commented May 17, 2019

In crossbeam-channel, we were considering adding the same impls but, after a long discussion, concluded it would be better to have methods that test whether senders/receivers are equivalent: crossbeam-rs/crossbeam#319

@r3v2d0g
Copy link
Contributor Author

r3v2d0g commented May 17, 2019

Oh, right, so I read the changes from crossbeam-rs/crossbeam#320 and it seems that the way it compares is similar to Arc::ptr_eq (in some cases it even is how it does it). So, if I understand correctly, removing the two impls and replacing them with a identical_to method would be OK?

@cramertj
Copy link
Member

Perhaps fn same_receiver(&self, other: &Self) -> bool?

Also removed the `PartialEq` and `Eq` impls for both.
@r3v2d0g r3v2d0g changed the title Implemented PartialEq and Eq for (Unbounded)Sender Added a "same_receiver" method to (Unbounded)Sender May 18, 2019
@cramertj
Copy link
Member

Great, thanks!

@cramertj cramertj merged commit 1dccf75 into rust-lang:master May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants