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

Add Ref/RefMut try_map method #118087

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GrigorenkoPV
Copy link
Contributor

@GrigorenkoPV GrigorenkoPV commented Nov 20, 2023

A more generalized version of filter_map, which allows to return some data on failure.

Safety

As far as I can tell, E cannot contain any 'b data, so it is impossible to duplicate the &'b [mut] reference into the RefCell's data.

Other than this E, everything is analogous to the already stable filter_map.

Try / Residual

I have considered generalizing this to use the Try & Residual just like #79711 does for array::try_map, but it does not seem to be possible: we want to essentially .map_err(|e| (orig, e)) but this does not seem to be supported with Try. (Plus I am not even sure if it is possible to express the fact that &U in Try::Output would have to have the same lifetime as the &T input of F.)

ACP

As far as I can tell, this is not mandatory, and the implementation is small enough to probably be smaller than the doc I would have to write.

rust-lang/libs-team#341

@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2023

r? @Mark-Simulacrum

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 20, 2023
@Mark-Simulacrum Mark-Simulacrum added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 9, 2023
@Mark-Simulacrum
Copy link
Member

ACPs are the recommended way to get the libs-api team to approve new unstable APIs. I've marked this as waiting-on-team which I believe also is in their queue.

(libs-api -- we may want to update the guidance on the linked docs to just require an ACP)

@m-ou-se
Copy link
Member

m-ou-se commented Dec 19, 2023

As far as I can tell, this is not mandatory, and the implementation is small enough to probably be smaller than the doc I would have to write.

The motivation is an important part of the ACP. We do need to understand the motivation, ideally with clear use cases, before accepting new (unstable) APIs.

@Amanieu Amanieu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Dec 19, 2023
@Dylan-DPC Dylan-DPC added the needs-acp This change is blocked on the author creating an ACP. label Feb 17, 2024
@Dylan-DPC
Copy link
Member

@GrigorenkoPV any updates on creating the ACP? if you have done so, kindly add it to the pr description. Thanks

@GrigorenkoPV
Copy link
Contributor Author

The ACP: rust-lang/libs-team#341

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 21, 2024
@Dylan-DPC Dylan-DPC added S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. and removed needs-acp This change is blocked on the author creating an ACP. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 22, 2024
@stegaBOB
Copy link

Thanks for your work on this @GrigorenkoPV! We would love for this method to be added. @Mark-Simulacrum is there anything else needed here to get it across the finish line? I'd be happy to put some time into this if I can help in any way.

@Dylan-DPC
Copy link
Member

@stegaBOB this is waiting on the ACP getting merged, so we need to wait fro that and nothing is expected from author or reviewer about it.

@bors
Copy link
Contributor

bors commented Dec 9, 2024

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

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 9, 2024
@GrigorenkoPV
Copy link
Contributor Author

#133567 changed a place in rustc_interface where this method could be used and now it no longer fits, oh well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants