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

Introduce a lifetime generic to Visit in order to allow borrowing out specific parts of the valuable::Value during visits #97

Open
nagisa opened this issue Jun 11, 2022 · 1 comment

Comments

@nagisa
Copy link

nagisa commented Jun 11, 2022

Motivating use-case

My motivating use-case is to write down in tracing 0.2 a visitor that is able to borrow Event’s message. Doing so today is impossible even though lifetimes should conceptually mostly work out. This problem extends to valuable::Value and its Visit trait as well, so the remaining of the issue will focus on valuable.

So lets say we begin by writing a function as such (any mistakes mine, I basically sketched this out in a github comment field):

fn extract_message<'val>(value: &'val valuable::Value<'val>) -> &'val str {
    struct Visitor<'val> {
        message: &'val str
    }
    impl<'val> valuable::Visit for Visitor<'val> {
        fn visit_entry(&mut self, key: valuable::Value<'_>, value: valuable::Value<'_>) {
            match (key, value) {
                (valuable::Value::String("message"), valuable::Value::String(v)) => {
                    self.message = v; // ERROR: `'_` does not outlive `'val`.
                }
                _ => {}
            }
        }
    }

    let mut visitor = Visitor<'val> { message: "" };
    value.visit(&mut visitor); 
    visitor.message    
}

Rust today provides no way to tell that, '_ will outlive 'val here, and so users are forced to allocate in the implementation to extract the string data here.

Proposed solution

The proposed solution here is to introduce a lifetime to Visit such that Visit becomes like this:

pub trait Visit<'a> {
    fn visit_value(&mut self, value: Value<'a>);
    fn visit_named_fields(&mut self, named_values: &NamedValues<'a>) { ... }
    fn visit_unnamed_fields(&mut self, values: &[Value<'a>]) { ... }
    fn visit_primitive_slice(&mut self, slice: Slice<'a>) { ... }
    fn visit_entry(&mut self, key: Value<'a>, value: Value<'a>) { ... }
}

This gives implementers an optional opportunity to tie the lifetime of values to the lifetime of the visitors contents. Any implementors wanting the current behaviour can write the implementation as such:

// Unfortunately a breaking change ­– you cannot leave out the `<'_>`.
impl Visit<'_> for SomeVisitor { ... }

While implementations such as one presented in the motivating use-case above are also made possible:

impl<'val> valuable::Visit<'val> for Visitor<'val> {
    fn visit_entry(&mut self, key: valuable::Value<'val>, value: valuable::Value<'val>) {
        match (key, value) {
            (valuable::Value::String("message"), valuable::Value::String(v)) => {
                self.message = v; // A-OK!
            }
            _ => {}
        }
    }
}
@CAD97
Copy link

CAD97 commented Jun 11, 2022

NB: I also proposed giving Visit a lifetime as part of tokio-rs/tracing#2048

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