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

Tracking issue for Result::cloned, Result::cloned_err, Result::copied, Result::copied_err #63168

Closed
ksqsf opened this issue Jul 31, 2019 · 9 comments
Labels
A-result-option Area: Result and Option combinators B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Tracked Libs issues that are tracked on the team's project board. requires-nightly This issue requires a nightly compiler in some way. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@ksqsf
Copy link
Contributor

ksqsf commented Jul 31, 2019

PR

@Centril Centril added B-unstable Blocker: Implemented in the nightly compiler and unstable. requires-nightly This issue requires a nightly compiler in some way. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 31, 2019
@alexcrichton
Copy link
Member

alexcrichton commented Sep 3, 2019

Some unresolved questions for us to resolve while this is unstable:

  • Do *_err method carry their weight?
  • Is it right for cloned to work on Result<&T, E> or should it work on Result<&T, &E>?
  • Should cloned be cloned_ok?

@KodrAus KodrAus added Libs-Tracked Libs issues that are tracked on the team's project board. A-result-option Area: Result and Option combinators labels Jul 30, 2020
@Fishrock123
Copy link
Contributor

My 2c:

cloned and copied for Result should do the respective operation to either branch, it seems to me that in most cases where you'd need it fro one you'd need it to handle both branches.

I realize this does make the copied one kinda marginal since most errors are unlikely to be Copy, but perhaps there is a workaround in the odd case you'd want to copy and yet be able exclude Err?

@spinda
Copy link

spinda commented Sep 28, 2021

I'm in favor of cloned only cloning the Ok side, i.e., going from Result<&T, E> to Result<T, E>. This API mirrors Option::cloned, and I think it's more likely that a function of the form fn do_fallible_thing() -> Option<&T> eventually evolves into fn do_fallible_thing() -> Result<&T, SomeError> than fn do_fallible_thing() -> Result<&T, &SomeError>. It's still the possibly-present success value that you want to clone; the fact that the failure path now carries an error (versus just None) is an incidental detail.

@matklad
Copy link
Member

matklad commented Dec 17, 2021

Given that we already have precedent of Result::map, which applies only to Ok, and is named map rather then map_ok, I feel moderately confident that:

  • cloned should only clone &T, not E
  • it should be named cloned, rather than cloned_ok

Not sure about _err variants, but they are stabilizable separately. So, it seems that we potentially can write a stabilization report here and FCP?

@ksqsf
Copy link
Contributor Author

ksqsf commented Dec 19, 2021

Thanks @matklad for bringing the attention. I believe we generally agree on cloned and copied so the _err variants are left out here.

Stabilization Report

Brief summary

  • Add Result::cloned() for converting Result<&T, E> and Result<&mut T, E> into Result<T, E> by cloning the contents of the Ok part.
  • Add Result::copied() for converting Result<&T, E> and Result<&mut T, E> into Result<T, E> by copying the contents of the Ok part.

Test cases

Documentation

Things not covered

  • The _err variants are not proposed to be stabilized.

@joshtriplett
Copy link
Member

Shall we stabilize the cloned and copied methods on Result, per the above stabilization report?

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Dec 19, 2021

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Dec 19, 2021
@rfcbot
Copy link

rfcbot commented Dec 22, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Dec 22, 2021
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jan 1, 2022
@rfcbot
Copy link

rfcbot commented Jan 1, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 5, 2022
Stabilize `result_cloned` and `result_copied`

Tracking issue: rust-lang#63168

The FCP is now completed.
@m-ou-se m-ou-se closed this as completed Jan 5, 2022
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-result-option Area: Result and Option combinators B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Tracked Libs issues that are tracked on the team's project board. requires-nightly This issue requires a nightly compiler in some way. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests