-
Notifications
You must be signed in to change notification settings - Fork 259
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
Bind lifetimes of &str returned from Key by the lifetime of 'k rather than the lifetime of the Key struct #648
Conversation
… than the lifetime of the Key struct
src/kv/key.rs
Outdated
@@ -57,7 +57,7 @@ impl<'k> fmt::Display for Key<'k> { | |||
} | |||
|
|||
impl<'k> AsRef<str> for Key<'k> { | |||
fn as_ref(&self) -> &str { | |||
fn as_ref(&self) -> &'k str { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has no effect unless you add #[refine]
(https://rust-lang.github.io/rfcs/3245-refined-impls.html).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it almost seems weird this compiles at all 👀 I hadn't seen #[refine]
before. Thanks for pointing it out!
Thanks @gbbosak |
I should've left a comment on the type suggesting why it was that way, but we intentionally hadn't used |
@KodrAus do we want to revert the pr to keep that option open? |
@Thomasdezeeuw I think we should, and provide a way to get |
This allows things inside a Visitor to persist these strings for the lifetime of 'k without having to copy them.