projectfluent / fluent-rs

Rust implementation of Project Fluent
https://projectfluent.org
Apache License 2.0
1.05k stars 95 forks source link

Arguments & ownership #190

Open vpzomtrrfrt opened 4 years ago

vpzomtrrfrt commented 4 years ago

Here's a simplified version of what I'm looking at, based on the example from the Good Practices page:

    let bundle: FluentBundle<FluentResource> = get_bundle_somehow();
    let mut errors = Vec::new();

    let num = get_number_somehow();

    let text = if num == 0 {
        let pattern = bundle.get_message("items-select").unwrap().value.unwrap();
        bundle.format_pattern(&pattern, None, &mut errors)
    } else {
        let pattern = bundle.get_message("items-selected").unwrap().value.unwrap();
        let args = std::iter::once(("num", num.into())).collect();
        bundle.format_pattern(&pattern, Some(&args), &mut errors)
    };

    println!("{}", text);

This doesn't compile, because the lifetime requirement on args is returned to the block:

error[E0597]: `args` does not live long enough
  --> src/main.rs:23:46
   |
17 |     let text = if num == 0 {
   |         ---- borrow later stored here
...
23 |         bundle.format_pattern(&pattern, Some(&args), &mut errors)
   |                                              ^^^^^ borrowed value does not live long enough
24 |     };
   |     - `args` dropped here while still borrowed

Is there something I'm missing here? How could this be made to work?

zbraniecki commented 3 years ago

I think the issue here is that you want to return from the else a value that depends on the args while they're dropped at the end of it.

In #179 we're talking about switching the keys to be String, but the values of the args will still have lifetimes, so I'm not sure if you can workaround it.

zbraniecki commented 3 years ago

@vpzomtrrfrt can you check if the current master solved it? We now accept Cow for FluentArgs which allows you to allocate there.

vpzomtrrfrt commented 3 years ago

I don't think this is resolved, format_pattern still returns a value bound to the lifetime of the args

zbraniecki commented 3 years ago

I see. The challenge here is that we're trying to be really light on how we handle allocations. That means that in theory, your items-selected may look like this:

items-selected = { $userName }

in which case we want to take userName which will be Cow<str> as FluentValue::String and return it untouched. For that to happen, we need the same lifetime on userName and on return value from format_pattern.

For numbers, we perform formatting so in theory maybe we could tailor the lifetimes to not span to other FluentValue variants, but that's complicated.

If you are ok with owning the result, you can just do:

    let text = if num == 0 {
        let pattern = bundle.get_message("items-select").unwrap().value.unwrap();
        bundle.format_pattern(&pattern, None, &mut errors).into_owned()
    } else {
        let pattern = bundle.get_message("items-selected").unwrap().value.unwrap();
        let args = std::iter::once(("num", num.into())).collect();
        bundle.format_pattern(&pattern, Some(&args), &mut errors).into_owned()
    };

or, you could do:

    let mut s = String::new();
    let text = if num == 0 {
        let pattern = bundle.get_message("items-select").unwrap().value.unwrap();
        bundle.write_pattern(&mut s, &pattern, None, &mut errors)
    } else {
        let pattern = bundle.get_message("items-selected").unwrap().value.unwrap();
        let args = std::iter::once(("num", num.into())).collect();
        bundle.write_pattern(&mut s, &pattern, Some(&args), &mut errors)
    };

    println!("{}", s)

would that resolve it for you?

vpzomtrrfrt commented 3 years ago

I guess this isn't actually a good example, since it always needs allocation.

The real case I'm looking at is one with no arguments, and ideally I would expect that to return with the lifetime of the resource, while currently it requires the lifetime of the bundle

Or maybe I'm just using bundles wrong?

zbraniecki commented 3 years ago

The real case I'm looking at is one with no arguments, and ideally I would expect that to return with the lifetime of the resource, while currently it requires the lifetime of the bundle

That sounds reasonable. Can you construct an example that you'd expect to work? I believe that if you pass no arguments and request a simple message that doesn't require any resolution then yes, it should return it with the lifetime of the resource that holds it.

Or maybe I'm just using bundles wrong?

I think you're not wrong. It's just tricky to get all lifetimes correctly :)

vpzomtrrfrt commented 3 years ago
    let mut errors = Vec::new();

    let text = {
        let bundle: FluentBundle<&'static FluentResource> = get_bundle_somehow();

        let pattern = bundle.get_message("hello").unwrap().value.unwrap();
        bundle.format_pattern(&pattern, None, &mut errors)
    };

    println!("{}", text);

I would hope to get the message with 'static in this case

zbraniecki commented 3 years ago

Ah, that's really tricky. Because if you have two messages, one that is a simple message from 'static Resource, and the other that is a straight placeable with a single argument, then those two messages in theory should require different lifetimes - one the lifetime of the resource, and the other the lifetime of the args.

The way we resolve it is that we return with a lifetime of a bundle and the bundle lifetime is that of resource and args.

I think the best way for such scenarios is to use write_pattern or take ownership of the returned value with into_owned.

vpzomtrrfrt commented 3 years ago

into_owned is what I'm using currently, but I'd rather not. I guess this would need something like a three-valued Cow?

vpzomtrrfrt commented 3 years ago

that wouldn't even help, since it would still need the shortest of the lifetimes

zbraniecki commented 3 years ago

No, it would need to be flexible with which lifetime it needs because it depends on what the pattern needs.

vpzomtrrfrt commented 3 years ago

or actually I think a multi-cow could help, since it could have a method to convert it to a Cow of the larger lifetime by cloning if necessary

zbraniecki commented 3 years ago

Hmm, it might work. If you want to take a stab at PR, I'm happy to review.