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

Arguments & ownership #190

Open
vpzomtrrfrt opened this issue Jul 22, 2020 · 13 comments
Open

Arguments & ownership #190

vpzomtrrfrt opened this issue Jul 22, 2020 · 13 comments
Labels
enhancement help wanted We need help making decisions or writing PRs for this.

Comments

@vpzomtrrfrt
Copy link

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
Copy link
Collaborator

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
Copy link
Collaborator

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

@zbraniecki zbraniecki added this to the 0.13 milestone Sep 18, 2020
@vpzomtrrfrt
Copy link
Author

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

@zbraniecki
Copy link
Collaborator

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?

@zbraniecki zbraniecki modified the milestones: 0.13, 0.14 Sep 25, 2020
@vpzomtrrfrt
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

    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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Author

vpzomtrrfrt commented Sep 25, 2020

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

@zbraniecki
Copy link
Collaborator

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

@vpzomtrrfrt
Copy link
Author

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
Copy link
Collaborator

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

@zbraniecki zbraniecki added enhancement help wanted We need help making decisions or writing PRs for this. labels Sep 25, 2020
@alerque alerque removed this from the 0.16 milestone May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted We need help making decisions or writing PRs for this.
Projects
None yet
Development

No branches or pull requests

3 participants