-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Delegate to inner vec::IntoIter
from env::ArgsOs
#139847
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
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @workingjubilee. Use |
3a60980
to
b63849b
Compare
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.
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.
b63849b
to
13e6873
Compare
☔ The latest upstream changes (presumably #140282) made this pull request unmergeable. Please resolve the merge conflicts. |
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 |
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 |
Reminder, once the PR becomes ready for a review, use |
ah, the concern is with a third type! thank you, that is much clearer. |
90b027d
to
ac00d49
Compare
I've rebased to resolve merge conflicts and touched up a comment, separately, so it's sane to compare with this terrible UI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
ac00d49
to
def3a30
Compare
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.
def3a30
to
28deaa6
Compare
Thanks! @bors 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.
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
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 usevec::IntoIter
as the inner type, so prioritize delegating to the methods it provides.std::env::Args
is implemented atopstd::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.