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

Change impls of PartialEq and friends in libstd to be more generic #2245

Closed
wants to merge 1 commit into from

Conversation

sgrif
Copy link
Contributor

@sgrif sgrif commented Dec 12, 2017

I can compare &[&str] with Vec<String>. I should be able to compare
Option<&str> with Option<String> and Option<&[&str]> with
Option<Vec<String>>.

Rendered

I can compare `&[&str]` with `Vec<String>`. I should be able to compare
`Option<&str>` with `Option<String>` and `Option<&[&str]>` with
`Option<Vec<String>>`.
@petrochenkov
Copy link
Contributor

This is blocked on some kind of default type parameter fallback in type inference.
I tried to implement this for Option even before 1.0 and it's a really breaking change, rustc alone had multiple instances of broken code.

Some previous issues about this - #917 rust-lang/rust#20063 rust-lang/rust#20927

@sgrif
Copy link
Contributor Author

sgrif commented Dec 12, 2017

I'd be interested in seeing a crater run for this. I don't think it's as common outside of rustc as you might think for code to be returning Option<T>. Some("foo") == None seems more likely to crop up, but I'd be surprised if there's that much code doing this vs .is_none(). As I mentioned in the drawbacks section, this is considered semver compatible by #1105.

Even if we choose to exclude Option or all enums from this, I think there is still value in applying this to the other types mentioned -- especially box and tuples. I wouldn't call this blocked.

@Centril Centril added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Dec 13, 2017
@rpjohnst
Copy link

Just skimmed the RFC, but it would be good to keep in mind rust-lang/rust#44762

That is, it would be nice to be able to flip a switch to turn on auto(de)ref in operators as a replacement for some of the impls in std. The closer this RFC and whatever we end up there, the better.

@leoyvens
Copy link

leoyvens commented Feb 3, 2018

#2321 should be able to solve this without breakage by using defaults.

@sgrif
Copy link
Contributor Author

sgrif commented Jun 21, 2018

Since this has gone without review for 6 months, and I suspect is unlikely to land, I'm going to save triage some trouble. 😄

@sgrif sgrif closed this Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants