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

Should the std library implement Equiv for Str? #16738

Closed
glennw opened this issue Aug 24, 2014 · 5 comments
Closed

Should the std library implement Equiv for Str? #16738

glennw opened this issue Aug 24, 2014 · 5 comments

Comments

@glennw
Copy link

glennw commented Aug 24, 2014

Anything that implements Str trait gets an implicit implementation of Equiv (see http://doc.rust-lang.org/std/cmp/trait.Equiv.html#tymethod.equiv).

In Servo, we have an Atom type that represents an interned string. For efficiency reasons, the hash value of the Atom is != the hash value of the string it represents.

If you then have a HashMap and call find_equiv(&str) it fails silently due to the hash mismatches.

Ideally we want this to be a compile error (you should always construct an Atom rather than use find_equiv).

However, due to the implicit trait implementation mentioned above, any type that implements Str also implements Equiv.

We have a workaround - we currently don't implement Str for Atom, and instead just implement as_slice() as a method on the Atom type.

Should we consider removing the implicit implementation of Equiv for anything that implements Str?

@alexcrichton
Copy link
Member

cc @aturon

@aturon
Copy link
Member

aturon commented Aug 26, 2014

@glennw Thanks for raising this point. There is no reason for libstd to provide this blanket implementation of Equiv, since Str is meant as an extension trait. I'll provide a fix.

I'm curious, though, about why you want to implement Str anyway? Do you have methods that take a Str bound on a generic type? The original intent of Str was just as a way to provide methods on the string slice type, since it's a primitive type and we (currently) cannot provide methods on it directly.

@glennw
Copy link
Author

glennw commented Aug 26, 2014

I'm not actually sure if we have methods that take Str as a bound. When implementing it seemed to make sense to implement traits that make it appear string-like, but it may not actually be needed.

Thanks for looking into it!

@kmcallister
Copy link
Contributor

In the html5ever C API I defined

pub fn from_str<T: Str>(x: &'a T) -> LifetimeBuf<'a> {

so that I could convert both Str and Atom to LifetimeBuf. But I could also live with a new ToLifetimeBuf trait.

kmcallister added a commit to servo/html5ever that referenced this issue Sep 6, 2014
@steveklabnik
Copy link
Member

Equiv is dead, so this is moot.

matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this issue Mar 10, 2024
Update rustc_pattern_analysis

This should fix rust-lang/rust-analyzer#16656 but I can't check because we don't have a reproducer.
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

No branches or pull requests

5 participants