Provide Way to Avoid Allocating To Collect Formatting Errors

[zicklag]

The bundle.format_pattern() method takes a &mut Vec<FluentError> to allow you to respond to errors, but I have a situation where my bundle struct is accessed concurrently across multiple threads with just a read-only reference.

I'm trying to avoid allocating every time I need to format a message. Usually, if I had mutable access, I'd just pre-allocate the vector for collecting the errors, and re-use that vector every time I format a message, but since I've only got concurrent access to my helper function for formatting, that isn't possible.

On thought is that I could pass in an FnMut closure that will be called every time there is an error. This would allow me to choose how I handle the error message without requiring an allocation.

In this case, I simply wish to log the error using the log or tracing crates.

---

On a related note, I just noticed that FluentArgs also allocates since it contains a Vec. I wonder if we could make that an iterator or something like that. 🤔 If it was a double-ended iterator, then that would allow us to iterate back and forth through it to find the keys in it as needed.

I'm using fluent for game development so avoiding allocations per-frame is really important where possible.

[zicklag]

I might be interested in contributing for this if it's something you guys are interested in.

[zbraniecki]

For the errors - we could swap to accept a mutable reference to a trait object that implements push_error. You can make it a no-op and default impl for Vec.

For args - this is a bit more tricky. I'm not sure if an iterator is actually saving us here, and I suspect it would incur a perf penalty. What we could do, again, is swap it for a trait that you can implement for Vec, HashMap, and your own type.

I'm open to PRs!

[zicklag]

Oh, good idea, I didn't think of using trait objects!

I think I'll try that out if I get the chance and open a PR if it looks like it works.

[alerque]

@zicklag Did you by any chance get an opportunity to experiment with this?

[zicklag]

No I didn't, and I probably won't end up being able to any time soon, though it's still something I'd like to come back to eventually.

[alerque]

Fair enough, I was just doing triage and wondered if there was anything out there to review, whether this was still something desired or not, etc.

[alerque]

This problem space is also covered in #254, but for different reasons. Given that any refactoring of this API surface should probably take both problems into account at once I'm going to close that issue as a duplicate of this one, but the issue/proposed fix is coming at this from a different angle than concurrency, so I'm duplicating the relevant discussion here:

From @clarfonthey:

Right now, APIs like format_message will just add to a list of localisation errors, which really sucks from multiple usability standpoints:

1. It's extremely un-rusty for an API, since an error occurring will still return a string, even if that string is ill-formed.
2. The user has to wait for all errors to be registered and stored in the vec, instead of being able to just take the first one.
3. The only feasible ways of using this kind of API across a program are with global state (bad) or allocating a vec on every usage (also bad).

My proposal is instead to make these APIs return a type along the lines of Result<Cow, ErrorIter>, where ErrorIter is an iterator over the errors found by fluent. This iterator can either be added to a vec directly, or passed on through multiple callers so that eventually it reaches one who is in charge of displaying errors.

Additionally, users can take advantage of iterator adapters by chaining multiple ErrorIter if they want to return more, or they can choose to take the easy route and just return a Vec in these cases.

Basically, this puts control back into the user of the API, rather than making a decision on how they should handle errors for them.

And from @gregtatum:

This seems like a reasonable approach to me. It'll be a bit of a big API change for consumers. I know Firefox would need to adjust our integration for it with the change.