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

Use a macro to not have to copy-paste ConstFnMutClosure::new(&mut fold, NeverShortCircuit::wrap_mut_2_imp)).0 everywhere #102300

Merged
merged 1 commit into from
Oct 7, 2022

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Sep 26, 2022

Also use that macro to replace a bunch of places that had custom closure-wrappers.

+35 -114 sounds good to me.

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 26, 2022
@rustbot

This comment was marked as resolved.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 26, 2022
library/core/src/iter/mod.rs Outdated Show resolved Hide resolved
@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 26, 2022
…ld, NeverShortCircuit::wrap_mut_2_imp)).0` everywhere

Also use that macro to replace a bunch of places that had custom closure-wrappers.
@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 Sep 26, 2022
@Mark-Simulacrum
Copy link
Member

Yeah, I feel like my take is "we should not have landed the thing this macro avoids" (https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/const.20in.20the.20standard.20library).

I feel like the macro is an improvement in some ways (obviously avoids duplication) but it's not really a way out; the places in std that use macros to avoid duplication are already a pain (e.g., integer methods) to review and edit changes to.

But I agree this is an improvement on the current state. r=me though I would also prefer waiting for that Zulip discussion somewhat.

@scottmcm
Copy link
Member Author

scottmcm commented Oct 1, 2022

I'm fine with waiting a bit -- I'll wait a couple days and see if the libs meeting reaches a conclusion.

The macro should be much less painful here than for, say, the unsigned integers because this is only used in trait impls. That all the mess about the manual #[doc]s doesn't happen for them.

(Of course, what I really want is just to make this the default fold and rfold implementations and stop needing these everywhere. But perf has been mad at me every time I've tried, so I've stopped fighting that fight until we get closer to Try stabilizing.)

@scottmcm scottmcm added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Oct 1, 2022
@scottmcm
Copy link
Member Author

scottmcm commented Oct 6, 2022

I doesn't seem like we got much a decision here in either libs or libs-api meeting this week, so I'm just going to merge this.

Feel free to ping me on Zulip if something gets decided and I'll happily revert or update if needed.

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Oct 6, 2022

📌 Commit 55492de has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 6, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 7, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#102300 (Use a macro to not have to copy-paste `ConstFnMutClosure::new(&mut fold, NeverShortCircuit::wrap_mut_2_imp)).0` everywhere)
 - rust-lang#102475 (unsafe keyword: trait examples and unsafe_op_in_unsafe_fn update)
 - rust-lang#102760 (Avoid repeated re-initialization of the BufReader buffer)
 - rust-lang#102764 (Check `WhereClauseReferencesSelf` after all other object safety checks)
 - rust-lang#102779 (Fix `type_of` ICE)
 - rust-lang#102780 (run Miri CI when std::sys changes)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2592609 into rust-lang:master Oct 7, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 7, 2022
@scottmcm scottmcm deleted the simpler-fold-closures branch October 8, 2022 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants