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

Make slice iterator constructors unstably const #137738

Merged
merged 1 commit into from
Apr 3, 2025

Conversation

Daniel-Aaron-Bloom
Copy link

@Daniel-Aaron-Bloom Daniel-Aaron-Bloom commented Feb 27, 2025

See tracking issue for justification.

try-job: aarch64-apple
try-job: x86_64-gnu

@rustbot
Copy link
Collaborator

rustbot commented Feb 27, 2025

r? @thomcc

rustbot has assigned @thomcc.
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 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 Feb 27, 2025
@Daniel-Aaron-Bloom
Copy link
Author

@rustbot label -T-libs +T-libs-api

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 27, 2025
@Daniel-Aaron-Bloom
Copy link
Author

r? libs-api

@rustbot rustbot assigned m-ou-se and unassigned thomcc Feb 27, 2025
@scottmcm
Copy link
Member

It's not obvious that these are desirable yet, since other could-trivially-be-const things like https://doc.rust-lang.org/nightly/std/iter/fn.once.html aren't. Probably worth a team decision on the broader question, then the reviews would be simple.

@bors
Copy link
Collaborator

bors commented Mar 16, 2025

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

@m-ou-se m-ou-se added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). I-libs-api-nominated Nominated for discussion during a libs-api team meeting. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2025
@m-ou-se m-ou-se assigned scottmcm and unassigned m-ou-se Mar 18, 2025
@dtolnay dtolnay assigned dtolnay and unassigned scottmcm Mar 28, 2025
@dtolnay
Copy link
Member

dtolnay commented Mar 28, 2025

This line from the tracking issue is spot on: "having unnecessary const limitations is preventing code reuse between const and non-const functions." I have personally experienced this much more often than I might have guessed. Writing some const fn gets stuck on some trivial standard library function not being const, look into the history of whether that function has been proposed to be const and why it isn't, and there is someone explaining why 'theoretically' nobody should ever want that function to be const because it would be useless. (Like for these iterator constructors, something like "because you can't call next on them in const anyway.") And yet…

So I buy the justification. This PR looks good to me as soon as you rebase to solve the merge conflict.

@dtolnay dtolnay added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed I-libs-api-nominated Nominated for discussion during a libs-api team meeting. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Mar 28, 2025
@Daniel-Aaron-Bloom
Copy link
Author

@rustbot ready

@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 Apr 1, 2025
@tgross35
Copy link
Contributor

tgross35 commented Apr 1, 2025

Crosslinking the zulip discussion #t-libs-api/api-changes > const slice iterator constructors

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 1, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 1, 2025
…e_iter, r=dtolnay

Make slice iterator constructors unstably const

See [tracking issue](rust-lang#137737) for justification.
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 1, 2025
…e_iter, r=dtolnay

Make slice iterator constructors unstably const

See [tracking issue](rust-lang#137737) for justification.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 1, 2025
Rollup of 12 pull requests

Successful merges:

 - rust-lang#110406 (rustdoc-json: Add test for #[automatically_derived] attribute)
 - rust-lang#137738 (Make slice iterator constructors unstably const)
 - rust-lang#138492 (remove `feature(inline_const_pat)`)
 - rust-lang#138928 (Fix UWP reparse point check)
 - rust-lang#138950 (replace extra_filename with strict version hash in metrics file names)
 - rust-lang#139002 (Add release notes for 1.86.0)
 - rust-lang#139022 (increment depth of nested obligations)
 - rust-lang#139060 (replace commit placeholder in vendor status with actual commit)
 - rust-lang#139102 (coverage: Avoid splitting spans during span extraction/refinement)
 - rust-lang#139129 (Add tests for slice bounds check optimization)
 - rust-lang#139188 (PassWrapper: adapt for llvm/llvm-project@94122d58fc77079a291a3d008914…)
 - rust-lang#139193 (Feed HIR for by-move coroutine body def, since the inliner tries to read its attrs)

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

Make slice iterator constructors unstably const

See [tracking issue](rust-lang#137737) for justification.
@bors
Copy link
Collaborator

bors commented Apr 1, 2025

⌛ Testing commit beb8e9e with merge 88dae5c...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Apr 1, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 1, 2025
@tgross35
Copy link
Contributor

tgross35 commented Apr 2, 2025

@bors try

@Daniel-Aaron-Bloom could you squash? Also FYI those are MIR tests (the IR), nor Miri (the interpreter).

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 2, 2025
…iter, r=<try>

Make slice iterator constructors unstably const

See [tracking issue](rust-lang#137737) for justification.

try-job: aarch64-apple
try-job: x86_64-gnu
@bors
Copy link
Collaborator

bors commented Apr 2, 2025

⌛ Trying commit 7e4cbc4 with merge bddf132...

@tgross35
Copy link
Contributor

tgross35 commented Apr 2, 2025

@bors r=dtolnay

@bors
Copy link
Collaborator

bors commented Apr 2, 2025

📌 Commit 20417a9 has been approved by dtolnay

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 2, 2025
@bors
Copy link
Collaborator

bors commented Apr 3, 2025

⌛ Testing commit 20417a9 with merge e0883a2...

@bors
Copy link
Collaborator

bors commented Apr 3, 2025

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing e0883a2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 3, 2025
@bors bors merged commit e0883a2 into rust-lang:master Apr 3, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 3, 2025
Copy link

github-actions bot commented Apr 3, 2025

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing b6d74b5 (parent) -> e0883a2 (this PR)

Test differences

Show 485 test diffs

Stage 2

  • [run-make] tests/run-make/compressed-debuginfo-zstd: ignore (ignored if LLVM wasn't build with zstd for ELF section compression (we want LLVM/LLD to be built with zstd support)) -> pass (J0)

Additionally, 484 doctest diffs were found. These are ignored, as they are noisy.

Job group index

  • J0: x86_64-gnu-nopt

Job duration changes

  1. dist-apple-various: 7774.8s -> 8824.2s (13.5%)
  2. x86_64-apple-2: 5874.0s -> 6600.6s (12.4%)
  3. x86_64-gnu-llvm-18-2: 5941.6s -> 6140.3s (3.3%)
  4. x86_64-gnu-llvm-19-2: 6009.8s -> 6110.0s (1.7%)
  5. dist-various-1: 4402.2s -> 4439.8s (0.9%)
  6. i686-msvc-2: 7318.7s -> 7377.3s (0.8%)
  7. dist-x86_64-msvc-alt: 7459.3s -> 7474.4s (0.2%)
  8. x86_64-gnu-llvm-19-1: 5357.3s -> 5365.6s (0.2%)
  9. x86_64-msvc-2: 6910.3s -> 6781.7s (-1.9%)
  10. x86_64-gnu-llvm-19-3: 6922.3s -> 6792.5s (-1.9%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e0883a2): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
0.3% [0.3%, 0.3%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% [0.3%, 0.3%] 1

Max RSS (memory usage)

Results (primary -2.0%, secondary -0.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.0% [2.0%, 2.0%] 1
Improvements ✅
(primary)
-2.0% [-2.0%, -2.0%] 1
Improvements ✅
(secondary)
-2.1% [-2.1%, -2.1%] 1
All ❌✅ (primary) -2.0% [-2.0%, -2.0%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results (primary -0.1%, secondary -0.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.1% [-0.2%, -0.0%] 55
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.1%] 4
All ❌✅ (primary) -0.1% [-0.2%, -0.0%] 55

Bootstrap: 778.474s -> 777.235s (-0.16%)
Artifact size: 365.89 MiB -> 365.89 MiB (0.00%)

@Daniel-Aaron-Bloom Daniel-Aaron-Bloom deleted the const_slice_make_iter branch April 3, 2025 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

10 participants