Map entry API: add Entry::or_insert_with_result()

[birkenfeld]

Takes a default function that returns a Result, and bails out
on Err cases. Use case: caches where producing the value
to cache can fail.

To clarify:

• is this a useful addition?
• is the name ok?
• is the feature name ok?

[rust-highfive]

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

[birkenfeld]

Hm, another question: since we're introducing a new type of closure here anyway, should it receive a borrow of the key? (Thinking about cases like @sgrif's https://gist.github.com/sgrif/68cbba9a87084da3a2e8c42a12e7d6cf here.)

[aturon]

cc @rust-lang/libs

I do think these kinds of additions can be useful, but we don't currently have a great convention for them, and of course in the long run it would be sad if every closure-taking function ends up having a Result-based variant.

We'll discuss this in the next libs team meeting.

[bors]

☔️ The latest upstream changes (presumably #33148) made this pull request unmergeable. Please resolve the merge conflicts.

[sgrif]

Just to chime in since some code from Diesel got referenced in the use case. This is what the code would look like with/without this feature (assuming the closure did in fact take the key as an argument). https://gist.github.com/sgrif/95bdbd302124010a0e4dbc39862313f2

I agree that we don't want every closure-taking function to have a special case for Result. I do think it is more common with the entry API though, as a map is much more likely to be used for memoization, and you likely don't want to memoize failure.

This seems like a good addition, but I don't think it makes sense to change it to take the key, for consistency with or_insert_with (Though if we were pre-1.0 I would argue that or_insert should pass the key to the closure as well). This means we wouldn't be able to use it for the referenced case, but I don't think it's terribly common to have a case where:

• The map is being used as a cache
• Errors need to be returned outright but not cached
• The key is used to calculate the value

Also my vote for a name would be or_insert_if_ok instead of or_insert_with_result

[aturon]

We didn't quite get to this at this week's libs meeting, but should next week.

[alexcrichton]

The libs team discussed this PR at triage the other day and the conclusion was that we probably don't want to merge this at this time. Handling this in the "most clean" fashion (e.g. with the current or_insert_with method) would likely require HKT of some form. Today we basically have to decide if closure-taking APIs handle results (like this) or not (like it does today). We're not comfortable starting a convention just yet of having dual APIs for this (as Result isn't the only kind of error), so for now so long as the functionality is available via some other means we're gonna stick with the one-closure APIs we have today.

Thanks for the PR though @birkenfeld!