Is it possible to allow to visit a value referencing data owned by the current function?

[taiki-e]

AFAIK, in the current API, Valuable cannot be implemented well if I need a temporarily owned value for serialization.

struct PathAsString(std::path::PathBuf);

impl Valuable for PathAsString {
    fn as_value(&self) -> Value<'_> {
        /*
        error[E0515]: cannot return value referencing temporary value
          --> lib.rs:74:9
           |
        74 |         Value::String(&self.0.display().to_string())
           |         ^^^^^^^^^^^^^^^----------------------------^
           |         |              |
           |         |              temporary value created here
           |         returns a value referencing data owned by the current function

        error: aborting due to previous error
        */
        Value::String(&self.0.display().to_string())
    }

    fn visit(&self, v: &mut dyn Visit) {
        v.visit_value(Value::String(&self.0.display().to_string()))
    }
}

Is there a way to handle such cases well?

(I was thinking a bit about removing the Valuable::as_value, but not sure if that would be a good idea.)

[taiki-e]

Another case of this problem is when I want to pass Value::Error to visitors. This can be avoided by making the error static (like the following example), but the problem remains that the error message cannot contain the details.

struct PathAsStr(std::path::PathBuf);

impl Valuable for PathAsStr {
    fn as_value(&self) -> Value<'_> {
        #[derive(Debug)]
        struct Error;
        static ERROR: Error = Error;
        impl std::fmt::Display for Error {
            fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
                std::fmt::Display::fmt(&"path is not valid UTF-8 string", f)
            }
        }
        impl std::error::Error for Error {}

        match self.0.to_str() {
            Some(s) => Value::String(s),
            None => Value::Error(&ERROR),
        }
    }

    fn visit(&self, visit: &mut dyn Visit) {
        visit.visit_value(self.as_value());
    }
}

[carllerche]

Well, the intent is that we avoid work in the as_value() fn, so a conversion to string should be avoided. For both cases, we could possibly add a Display variant:

Value::Display(&dyn Display)

In the second case, the error is then received when visiting and the visitor can handle it as it wishes.

[carllerche]

Ref: #4

We also may want to add a Debug(&dyn Debug) fallback option?

[taiki-e]

so a conversion to string should be avoided. For both cases, we could possibly add a Display variant:

Hmm. The creation of String is a red herring. The struct returned by Path::display() is also owned by the function, so the addition of Value::Display does not solve the problem.

That said, I think that Value::Display might be useful regardless of this problem.

[KodrAus]

When I ran into this case in the past I ended up with an API like this: https://docs.rs/value-bag/1.0.0-alpha.7/value_bag/fill/index.html

The variant on Value is effectively a &'a dyn Fn(Visit), so your value itself satisfies the 'a lifetime, but the data you pass to the visitor can be short-lived.

[carllerche]

@taiki-e Ah right, my mistake.

The strategy we use is similar to what @KodrAus is describing, but we use *able traits instead of fns. So, here, hypothetically, we would have trait Displayable and that would call a Visit::visit_display(display: &dyn Display) or something.

[sunng87]

When working with valuable in handlebars, I may have a case for this issue as well.

During a template rendering process, some intermediate values are computed and are owned by internal context. If I will use valuable::Value as my internal type system, it needs capability to hold data like CoW.

Not quite sure if this matches original rational of valuable. But it seems to be required to use in handlebars.

[taiki-e]

triage: A variant of Value to handle this (Value::Displayable?) can be added later without API break, so there is no need to consider this as a blocker of 0.1.0.