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

Add "impl PartialEq<&str> for Ident" #186

Closed
CreepySkeleton opened this issue Jul 23, 2019 · 5 comments
Closed

Add "impl PartialEq<&str> for Ident" #186

CreepySkeleton opened this issue Jul 23, 2019 · 5 comments

Comments

@CreepySkeleton
Copy link
Contributor

Currently there's only one way to examine contents of an Ident - &*ident.to_string() == "foo". I'd say this is kind of cumbersome and the more direct approach would be useful - ident == "foo".

By the way, I had encountered some people from the core team claim it's not safe to expose an a str reference from Ident - I don't know why and I would really love to be given an explanation. But keeping this in mind, still, proposed impl doesn't expose anything so it's should be safe anyway.

@CreepySkeleton
Copy link
Contributor Author

Probably we could use impl PartialEq<&str> for LitStr too

@dtolnay
Copy link
Owner

dtolnay commented Jul 23, 2019

This impl already exists. Playground

@CreepySkeleton
Copy link
Contributor Author

Oooh... now I'm really curious why my code wasn't working. So what about AsStr for Ident? People just keep saying "no way, use to_string()` but nobody can explain why

@dtolnay
Copy link
Owner

dtolnay commented Jul 24, 2019

Idents are stored in a thread local interner -- rust-lang/rust#38356. That's why they are not Send. Exposing a conversion from &'a Ident to &'a str would be unsound because you could do Box::leak(Box::new(ident)) to get a &'static Ident, convert to &'static str, hand it across to a different thread, have the original thread die and clear the interner, and then the &str continuing to exist would be undefined behavior.

@CreepySkeleton
Copy link
Contributor Author

@dtolnay Thank's for clarification, I believe this issue should be closed

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

2 participants