Improving ergonomics with Result?

[leeola]

The replace_with_and_return F signature of FnOnce(T) -> (U, T) is rather awkward to use with Result'ing code.

What are your thoughts on a function that behaves similarly to replace_with_and_return, but tweaked to support Result and/or Option? A non-optimal path, but not quite a panic either. Something like:

pub fn replace_with_ok<T, U, D: FnOnce() -> T, F: FnOnce(T) -> Result<T, E>(
    dest: &mut T, 
    default: D,
    f: F
) -> Result<(), E>

I imagine the panic/abort behavior would remain the same. Using the default where applicable.

Though, I could see someone requesting a panic_default: D and err_default: D, but those don't match my use case as much.

Thoughts?

[alecmocatta]

Thanks @leeola! I see the motivation. It does complicate the semantics of when the default closure is called but I understand that may be the best compromise.

In your use case, and any other plausible use cases you're thinking of, would the f closure returning Result<T, (E, T)> be workable? How does this compare to the signature you outlined? Also @tage64, do you have any thoughts here?

[leeola]

I would imagine Result<T, (E, T)> would still be a bit problematic. Notably that it would require From<(E, T)> for error handling for ? to work - but it would still be a big improvement over the current -> (Result<(), E>, T).

That's why in my straw man example I had assumed D would create T in cases where Err(E) is present.

.. to counter my own statement though, in the final version of the code that motivated this post I ended up returning (Err(E), T) and preserving the already allocated T in many cases. Which is to say, I didn't default T in error cases, I kept the existing valid allocation of T when an Err(E) was encountered. So this behavior of being able to both Error and return a T seems of value.

If this feature is being added for the purpose of ergonomics, perhaps keeping it Result<T, E> is best? Users who want to return (Err(E), T) could still use the existing replace_with_and_return right?

Difficult choice :)

[tage64]

Can you give some examples how you currently need to write it with the existing replace_with_or_return*-functions and how you would like to write it with a new try_replace_with-function? Then we can easier figure out the best ergonomical solution.

[leeola]

Not sure what examples I can give, beyond just wanting ? to work. Eg:

try_replace_with(self.foo, Foo::default, |foo| {
  let z = baz()?;
  boom(z)?
})?;

There may be fine details to debate, but I think ? is the only requirement this issue thread is ultimately asking for. If I'm making sense :)

[tage64]

@leeola in your example you don't even use foo so you don't need the replace_with function. But the real problem is that you can't use the ?-operator if the F-closure should return Result<T, (T, E)> since (T, E> doesn't implement From. A solution might be your original proposal that the F-closure should return Result<T, E> and that the default closure should be called when an error is returned. But as @alecmocatta mentioned, it complicates the semantics when the default closure is called. That might not be a problem, but we need to make sure that that is what we really need. The problem is that it might not be obvious for the caller, or at least not a reader of the callers code, that the value in the mutable reference has changed to some unrelated default value when an error occurred.

.. to counter my own statement though, in the final version of the code that motivated this post I ended up returning (Err(E), T) and preserving the already allocated T in many cases. Which is to say, I didn't default T in error cases, I kept the existing valid allocation of T when an Err(E) was encountered. So this behavior of being able to both Error and return a T seems of value.

This shows that the wanted behavior of the function is unclear and hence it shouldn't maybe be implemented. After all, it isn't that difficult to write a function anyway. Either as @leeola did by preserving the already allocated value, or with a fallback function to generate a default value in case of an error.

[leeola]

@leeola in your example you don't even use foo so you don't need the replace_with function.

Yea it wasn't a working example hah. I was literally just talking about having ? work.

A solution might be your original proposal that the F-closure should return Result<T, E> and that the default closure should be called when an error is returned. But as @alecmocatta mentioned, it complicates the semantics when the default closure is called. That might not be a problem, but we need to make sure that that is what we really need. The problem is that it might not be obvious for the caller, or at least not a reader of the callers code, that the value in the mutable reference has changed to some unrelated default value when an error occurred.

Agreed

This shows that the wanted behavior of the function is unclear and hence it shouldn't maybe be implemented.

I disagree with the wanted behavior being unclear. My original intention (when this card was posted) was that the return signature made ? difficult, or maybe impossible, to use. The use case and wanted behavior is obvious in my view. Though, I agree the implementation and intuitive behavior is easily subject to debate.

This card isn't describing an unsolvable problem. You don't need ? to work. ? is about ease in error handling, ergonomics if you will. So if we want ? to work, in an intuitive way this card can discuss that. However it is very much not needed, you can match in place of every ?, your code just becomes a bit verbose.

I'm okay with either, I'll let ya'll hash it out. I just figured I should make the request against the library, or possibly write my own func to handle it.

Appreciate the library! Thanks :)

[tage64]

When try blocks gets stabilized, this will be easier and more clear. For example:

replace_with_or_abort_and_return(foo, |mut foo| {
    let res: Result<(), Error> = try { foo = some_fn()? }
    (res, foo)
})

[alecmocatta]

Thanks both for putting in the time to explore the solution space!

Try blocks do seem to be approaching stabilisation – a big outstanding design decision having been recently resolved.

If we operate under the assumption stabilisation is forthcoming, how do you feel @leeola about @tage64's suggested solution above?

[leeola]

While not being terribly familiar with try blocks, it looks to solve the ergonomics problem nicely. Minimal syntax addition to use ?, no need to add a new function to this library, and uses Result<(), Error> rather than needing Result<T, (E, T)>. Seems like a win on all sides. There may be downsides to try blocks I'm unfamiliar with, but it looks like the best solution from the outside 👍

[alecmocatta]

Great, going to close this as resolved but feel free to update if try blocks don't work out for whatever reason and we can re-evaluate. Thanks both!