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

impl Borrow<[u8]> for Tendril #13

Merged
merged 2 commits into from
Jul 21, 2015

Conversation

chris-morgan
Copy link
Contributor

This makes using a Tendril as a HashMap key a little more palatable. Sadly, str and [u8] hash differently at present, so we shouldn’t implement Borrow<str> for StrTendril. An alternative would be making the Hash implementations for Tendril<F> manual and making Tendril<UTF8> use the str Hash implementation and the rest use the [u8] one.

See also rust-lang/rust#27108 which deals with fixing the underlying problem of the differing Hash implementations.

Review on Reviewable

This makes using a Tendril as a HashMap key a little more palatable.
Sadly, str and [u8] hash differently at present, so we shouldn’t
implement Borrow<str> for StrTendril. An alternative would be making
the Hash implementations for Tendril<F> manual and making Tendril<UTF8>
use the str Hash implementation and the rest use the [u8] one.
See also rust-lang/rust#27108 which deals with
fixing the underlying problem of the differing Hash implementations.
@chris-morgan
Copy link
Contributor Author

It occurs to me just now that I should for maximal assurance add a test of this, using tendrils as HashMap keys. I’ll probably do this on Monday; just now I’m going to bed—genuinely forgetting to add a relevant test before making such a PR shows I need it!

@chris-morgan
Copy link
Contributor Author

I’ve now added a basic test of using tendrils as HashMap keys.

@SimonSapin
Copy link
Member

Looks good, thanks!

SimonSapin added a commit that referenced this pull request Jul 21, 2015
@SimonSapin SimonSapin merged commit 72da446 into servo:master Jul 21, 2015
@chris-morgan chris-morgan deleted the impl-borrow-for-tendril branch July 23, 2015 00:43
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