Delegate to inner `vec::IntoIter` from `env::ArgsOs` by thaliaarchi · Pull Request #139847 · rust-lang/rust

thaliaarchi · 2025-04-15T05:58:43Z

Delegate from std::env::ArgsOs to the methods of the inner platform-specific iterators, when it would be more efficient than just using the default methods of its own impl. Most platforms use vec::IntoIter as the inner type, so prioritize delegating to the methods it provides.

std::env::Args is implemented atop std::env::ArgsOs and performs UTF-8 validation with a panic for invalid data. This is a visible effect which users certainly rely on, so we can't skip any arguments. Any further iterator methods would skip some elements, so no change is needed for that type.

Add #[inline] for any methods which simply wrap the inner iterator.

rustbot · 2025-04-15T05:58:48Z

r? @workingjubilee

rustbot has assigned @workingjubilee.
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

The zkVM implementation of `env::ArgsOs` reports the full length even after having iterated once, which is incorrect. Instead, use a double cursor approach which works out to be simpler. Also, implement more iterator methods like the other platforms in rust-lang#139847.

The zkVM implementation of `env::ArgsOs` incorrectly reports the full length even after having iterated. Instead, use a range approach which works out to be simpler. Also, implement more iterator methods like the other platforms in rust-lang#139847.

bors · 2025-04-25T16:35:01Z

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

workingjubilee · 2025-05-01T05:43:54Z

Since env::ArgsOs performs UTF-8 validation, it cannot benefit from this,

uhhh. hm.

I am slightly confused as to how to read the pull request's description, as my first attempt to understand it leads to deriving "the changes in this PR cannot benefit env::ArgsOs", which raises the question of why we are implementing these changes. can you clarify?

workingjubilee · 2025-05-01T05:47:29Z

the code seems cromulent besides that, but I'll take a closer look with an explanation for what I should be looking for, because I feel like I missed a beat (lose the rhythm, and nothing falls into place! only missed by a fraction, slipped a little off your pace! oh!)

@rustbot author

rustbot · 2025-05-01T05:47:33Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

library/std/src/env.rs

workingjubilee · 2025-05-01T06:17:00Z

ah, the concern is with a third type! thank you, that is much clearer.

thaliaarchi · 2025-05-01T06:32:12Z

I've rebased to resolve merge conflicts and touched up a comment, separately, so it's sane to compare with this terrible UI.

workingjubilee

one nit, feel free to revise my suggestion as you see fit as long as it states this is a current understanding or "quality of implementation" detail and it is not completely clear that our contract hard-and-fast promises anything here.

library/std/src/env.rs

library/std/src/sys/args/unsupported.rs

Delegate from `std::env::ArgsOs` to the methods of the inner platform-specific iterators, when it would be more efficient than just using the default methods of its own impl. Most platforms use `vec::IntoIter` as the inner type, so prioritize delegating to the methods it provides. `std::env::Args` is implemented atop `std::env::ArgsOs` and performs UTF-8 validation with a panic for invalid data. This is a visible effect which users certainly rely on, so we can't skip any arguments. Any further iterator methods would skip some elements, so no change is needed for that type. Add `#[inline]` for any methods which simply wrap the inner iterator.

workingjubilee · 2025-05-02T00:47:42Z

Thanks!

@bors r+

bors · 2025-05-02T00:47:45Z

📌 Commit 28deaa6 has been approved by workingjubilee

It is now in the queue for this repository.

Rollup of 12 pull requests Successful merges: - rust-lang#134034 (handle paren in macro expand for let-init-else expr) - rust-lang#137474 (pretty-print: Print shebang at the top of the output) - rust-lang#138872 (rustc_target: RISC-V `Zfinx` is incompatible with `{ILP32,LP64}[FD]` ABIs) - rust-lang#139046 (Improve `Lifetime::suggestion`) - rust-lang#139206 (std: use the address of `errno` to identify threads in `unique_thread_exit`) - rust-lang#139608 (Clarify `async` block behaviour) - rust-lang#139847 (Delegate to inner `vec::IntoIter` from `env::ArgsOs`) - rust-lang#140159 (Avoid redundant WTF-8 checks in `PathBuf`) - rust-lang#140197 (Document breaking out of a named code block) - rust-lang#140389 (Remove `avx512dq` and `avx512vl` implication for `avx512fp16`) - rust-lang#140430 (Improve test coverage of HIR pretty printing.) - rust-lang#140507 (rustc_target: RISC-V: feature addition batch 3) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#139847 - thaliaarchi:args/delegate-iter, r=workingjubilee Delegate to inner `vec::IntoIter` from `env::ArgsOs` Delegate from `std::env::ArgsOs` to the methods of the inner platform-specific iterators, when it would be more efficient than just using the default methods of its own impl. Most platforms use `vec::IntoIter` as the inner type, so prioritize delegating to the methods it provides. `std::env::Args` is implemented atop `std::env::ArgsOs` and performs UTF-8 validation with a panic for invalid data. This is a visible effect which users certainly rely on, so we can't skip any arguments. Any further iterator methods would skip some elements, so no change is needed for that type. Add `#[inline]` for any methods which simply wrap the inner iterator.

The zkVM implementation of `env::ArgsOs` incorrectly reports the full length even after having iterated. Instead, use a range approach which works out to be simpler. Also, implement more iterator methods like the other platforms in rust-lang#139847.

…workingjubilee Delegate to inner `vec::IntoIter` from `env::ArgsOs` Delegate from `std::env::ArgsOs` to the methods of the inner platform-specific iterators, when it would be more efficient than just using the default methods of its own impl. Most platforms use `vec::IntoIter` as the inner type, so prioritize delegating to the methods it provides. `std::env::Args` is implemented atop `std::env::ArgsOs` and performs UTF-8 validation with a panic for invalid data. This is a visible effect which users certainly rely on, so we can't skip any arguments. Any further iterator methods would skip some elements, so no change is needed for that type. Add `#[inline]` for any methods which simply wrap the inner iterator.

Fix `env::ArgsOs` for zkVM The zkVM implementation of `env::ArgsOs` incorrectly reports the full length even after having iterated. Instead, use a range approach which works out to be simpler. Also, implement more iterator methods like the other platforms in #139847. cc `@flaub` `@jbruestle` `@SchmErik`

The zkVM implementation of `env::ArgsOs` incorrectly reports the full length even after having iterated. Instead, use a range approach which works out to be simpler. Also, implement more iterator methods like the other platforms in rust-lang#139847.

Fix `env::ArgsOs` for zkVM The zkVM implementation of `env::ArgsOs` incorrectly reports the full length even after having iterated. Instead, use a range approach which works out to be simpler. Also, implement more iterator methods like the other platforms in rust-lang#139847. cc `@flaub` `@jbruestle` `@SchmErik`

rustbot assigned workingjubilee Apr 15, 2025

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 Apr 15, 2025

thaliaarchi force-pushed the args/delegate-iter branch 2 times, most recently from 3a60980 to b63849b Compare April 15, 2025 06:08

thaliaarchi mentioned this pull request Apr 15, 2025

Fix env::ArgsOs for zkVM #139849

Merged

thaliaarchi force-pushed the args/delegate-iter branch from b63849b to 13e6873 Compare April 22, 2025 11:12

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 May 1, 2025

workingjubilee reviewed May 1, 2025

View reviewed changes

library/std/src/env.rs Outdated Show resolved Hide resolved

thaliaarchi force-pushed the args/delegate-iter branch 2 times, most recently from 90b027d to ac00d49 Compare May 1, 2025 06:30

workingjubilee approved these changes May 1, 2025

View reviewed changes

library/std/src/env.rs Outdated Show resolved Hide resolved

library/std/src/sys/args/unsupported.rs Show resolved Hide resolved

thaliaarchi force-pushed the args/delegate-iter branch from ac00d49 to def3a30 Compare May 1, 2025 22:16

thaliaarchi force-pushed the args/delegate-iter branch from def3a30 to 28deaa6 Compare May 1, 2025 22:18

thaliaarchi mentioned this pull request May 1, 2025

Implement Iterator::last for vec::IntoIter #139773

Merged

bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 2, 2025

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 2, 2025

Zalathar mentioned this pull request May 2, 2025

Rollup of 12 pull requests #140581

Merged

bors merged commit 6fc78d4 into rust-lang:master May 2, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone May 2, 2025

thaliaarchi deleted the args/delegate-iter branch May 3, 2025 03:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Delegate to inner `vec::IntoIter` from `env::ArgsOs`#139847

Delegate to inner `vec::IntoIter` from `env::ArgsOs`#139847
bors merged 1 commit intorust-lang:masterfrom
thaliaarchi:args/delegate-iter

thaliaarchi commented Apr 15, 2025 •

edited

Loading

Uh oh!

rustbot commented Apr 15, 2025

Uh oh!

bors commented Apr 25, 2025

Uh oh!

workingjubilee commented May 1, 2025

Uh oh!

workingjubilee commented May 1, 2025

Uh oh!

rustbot commented May 1, 2025

Uh oh!

Uh oh!

workingjubilee commented May 1, 2025

Uh oh!

thaliaarchi commented May 1, 2025

Uh oh!

workingjubilee left a comment

Uh oh!

Uh oh!

Uh oh!

workingjubilee commented May 2, 2025

Uh oh!

bors commented May 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

thaliaarchi commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 15, 2025

Uh oh!

bors commented Apr 25, 2025

Uh oh!

workingjubilee commented May 1, 2025

Uh oh!

workingjubilee commented May 1, 2025

Uh oh!

rustbot commented May 1, 2025

Uh oh!

Uh oh!

workingjubilee commented May 1, 2025

Uh oh!

thaliaarchi commented May 1, 2025

Uh oh!

workingjubilee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

workingjubilee commented May 2, 2025

Uh oh!

bors commented May 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thaliaarchi commented Apr 15, 2025 •

edited

Loading