Skip to content

Promise array::from_fn is generated in order of increasing indices #139099

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

Merged
merged 1 commit into from
Apr 9, 2025

Conversation

scottmcm
Copy link
Member

Fixes #139061

I agree this needs to be documented because of the FnMut, either with a guarantee or to explicitly disclaim one.

I'm pretty sure this will be non-controversial (like the other "well sure you could do it in a different order, but why?" things were), but I couldn't find any previous libs-api decision on it so it's seemingly a new promise that will need FCP.

Basically, yes, it would be plausible to fill in the reverse order, but there's no obvious way we could ever know that that might even be a good idea, so forward seems like an easy thing to promise. We could always add a from_fn_rev or something later if there's ever a strong enough need, but it seems unlikely.

Let's just do the obvious thing so it matches what [gen(0), gen(1), …, gen(N-1)] does.

@scottmcm scottmcm added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels Mar 29, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 29, 2025

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2025
@scottmcm scottmcm added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Mar 29, 2025
@scottmcm scottmcm changed the title Promise array::from_fn in generated in order of increasing indices Promise array::from_fn is generated in order of increasing indices Mar 29, 2025
@Amanieu
Copy link
Member

Amanieu commented Mar 29, 2025

There seems to be little downside to guaranteeing this.

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Mar 29, 2025

Team member @Amanieu 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 the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 29, 2025
@Amanieu Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Mar 29, 2025
@rfcbot rfcbot added 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 Mar 29, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Mar 29, 2025

🔔 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 Mar 29, 2025
@scottmcm scottmcm added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Mar 29, 2025
@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 Apr 8, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 8, 2025

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.

@scottmcm
Copy link
Member Author

scottmcm commented Apr 8, 2025

Since this is (essentially) just a comment change, I think I can treat the libs-api FCP as a review of it. We can always make it more-better in later PRs.

@bors r=Amanieu rollup=always

@bors
Copy link
Collaborator

bors commented Apr 8, 2025

📌 Commit 6a91596 has been approved by Amanieu

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 8, 2025
@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 Apr 8, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 9, 2025
Promise `array::from_fn` is generated in order of increasing indices

Fixes rust-lang#139061

I agree this needs to be documented because of the `FnMut`, either with a guarantee or to explicitly disclaim one.

I'm pretty sure this will be non-controversial (like the other "well sure you *could* do it in a different order, but why?" things were), but I couldn't find any previous libs-api decision on it so it's seemingly a new promise that will need FCP.

Basically, yes, it would be plausible to fill in the reverse order, but there's no obvious way we could ever know that that might even be a good idea, so forward seems like an easy thing to promise.  We could always add a `from_fn_rev` or something later if there's ever a strong enough need, but it seems unlikely.

Let's just do the obvious thing so it matches what `[gen(0), gen(1), …, gen(N-1)]` does.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped)
 - rust-lang#138869 (Try not to use verbatim paths in `Command::current_dir`)
 - rust-lang#138993 (Make `cfg_match!` a semitransparent macro)
 - rust-lang#139099 (Promise `array::from_fn` is generated in order of increasing indices)
 - rust-lang#139364 (Make the compiler suggest actual paths instead of visible paths if the visible paths are through any doc hidden path.)
 - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`)
 - rust-lang#139481 (Add job summary links to post-merge report)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138869 (Try not to use verbatim paths in `Command::current_dir`)
 - rust-lang#138993 (Make `cfg_match!` a semitransparent macro)
 - rust-lang#139099 (Promise `array::from_fn` is generated in order of increasing indices)
 - rust-lang#139364 (Make the compiler suggest actual paths instead of visible paths if the visible paths are through any doc hidden path.)
 - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`)
 - rust-lang#139481 (Add job summary links to post-merge report)
 - rust-lang#139573 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138869 (Try not to use verbatim paths in `Command::current_dir`)
 - rust-lang#138993 (Make `cfg_match!` a semitransparent macro)
 - rust-lang#139099 (Promise `array::from_fn` is generated in order of increasing indices)
 - rust-lang#139364 (Make the compiler suggest actual paths instead of visible paths if the visible paths are through any doc hidden path.)
 - rust-lang#139468 (Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`)
 - rust-lang#139481 (Add job summary links to post-merge report)
 - rust-lang#139573 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4d1a639 into rust-lang:master Apr 9, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 9, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2025
Rollup merge of rust-lang#139099 - scottmcm:from_fn_docs, r=Amanieu

Promise `array::from_fn` is generated in order of increasing indices

Fixes rust-lang#139061

I agree this needs to be documented because of the `FnMut`, either with a guarantee or to explicitly disclaim one.

I'm pretty sure this will be non-controversial (like the other "well sure you *could* do it in a different order, but why?" things were), but I couldn't find any previous libs-api decision on it so it's seemingly a new promise that will need FCP.

Basically, yes, it would be plausible to fill in the reverse order, but there's no obvious way we could ever know that that might even be a good idea, so forward seems like an easy thing to promise.  We could always add a `from_fn_rev` or something later if there's ever a strong enough need, but it seems unlikely.

Let's just do the obvious thing so it matches what `[gen(0), gen(1), …, gen(N-1)]` does.
@scottmcm scottmcm deleted the from_fn_docs branch April 9, 2025 18:02
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 19, 2025
Promise `array::from_fn` is generated in order of increasing indices

Fixes rust-lang#139061

I agree this needs to be documented because of the `FnMut`, either with a guarantee or to explicitly disclaim one.

I'm pretty sure this will be non-controversial (like the other "well sure you *could* do it in a different order, but why?" things were), but I couldn't find any previous libs-api decision on it so it's seemingly a new promise that will need FCP.

Basically, yes, it would be plausible to fill in the reverse order, but there's no obvious way we could ever know that that might even be a good idea, so forward seems like an easy thing to promise.  We could always add a `from_fn_rev` or something later if there's ever a strong enough need, but it seems unlikely.

Let's just do the obvious thing so it matches what `[gen(0), gen(1), …, gen(N-1)]` does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

array::from_fn behavior unclear from documentation
5 participants