Move IoSlice and IoSliceMut to core::io#155849
Conversation
|
r? @jhpratt rustbot has assigned @jhpratt. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
227cc04 to
868ecad
Compare
This comment has been minimized.
This comment has been minimized.
868ecad to
37093d7
Compare
|
a long awating staff, thanks. |
This comment has been minimized.
This comment has been minimized.
37093d7 to
987b7d7
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
987b7d7 to
5f6ae4a
Compare
This comment has been minimized.
This comment has been minimized.
5f6ae4a to
1cf7794
Compare
This comment has been minimized.
This comment has been minimized.
1cf7794 to
c1f8dbe
Compare
|
I've split this PR into 3 commits to better highlight how little actually changes in order to facilitate this move. For reviewers, I recommend going through each commit individually. |
c1f8dbe to
ac81554
Compare
|
As a heads up, I am currently exploring Europe and won't be home for a couple more weeks. If you'd like a review before then, feel free to reroll. |
Thanks for the heads up, hope you have a great time! r? libs |
This comment was marked as outdated.
This comment was marked as outdated.
|
I will probably also take a day or two before I can get to this, but given the stuff it's touching it's likely worth a perf run: @bors try @rust-timer queue |
Move `IoSlice` and `IoSliceMut` to `core::io` ACP: rust-lang/libs-team#755 Tracking issue: #154046 Related: #152918 Related: #155625 ## Description Moves `std::io::IoSlice` and `std::io::IoSliceMut` into `core::io`. This is required for the `Read` and `Write` traits to be moved into `alloc` and/or `core`, as they contain stable methods which work with IO slices. Similar to #155574, this PR inlines the `std::sys` types required to create an ABI compatible type for the platforms where such compatibility is guaranteed. Additionally, I've moved the relevant tests out of `std::io::tests` into `coretests::io::io_slice`. --- ## Notes * This PR overlaps with #152918, but goes further than moving the IO slice types to `alloc` and instead moves them straight to `core`. Since these types have no interaction with allocation, and doing so will allow `Write` to move to `core::io`, I consider this a better home for these types. * Some discussion around the decision to not use a `core::sys` module can be found [here](#155574 (comment)). * I've renamed `unsupported` to `generic` to better reflect that `IoSlice(Mut)` _is_ supported by all platforms, it just doesn't have a special ABI-compatible type. I don't want to imply that parts of `core` "don't work" depending on the target; `IoSlice(Mut)` works exactly as expected on all targets. * I've made `pub` items within each platform-specific representation `pub(super)` to highlight that everything within `core::io::io_slice` is an internal implementation detail not meant for any other part of the crate to be aware of. * No AI tooling of any kind was used during the creation of this PR.
This comment has been minimized.
This comment has been minimized.
|
💔 Test for 27478a4 failed: CI. Failed job:
|
|
Let's do a try build after this is fixed. @rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
GitHub is being broken again, I can't see the logs where it failed since GitHub refuses to show the full logs and the show raw logs link 404s, they're stuck in the LLVM build portion (which is a GitHub problem, not a building-LLVM problem). So, maybe we should do a try build now in hopes of actually getting some logs with which to deduce the problem. |
|
@bors try jobs=dist-x86_64-apple |
This comment has been minimized.
This comment has been minimized.
Move `IoSlice` and `IoSliceMut` to `core::io` try-job: dist-x86_64-apple
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@bors retry |
This comment has been minimized.
This comment has been minimized.
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 a857d06 (parent) -> c58275e (this PR) Test differencesShow 198 test diffsStage 1
Stage 2
Additionally, 166 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard c58275e0369d09fc3959b8ba87dcbcbe73797465 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (c58275e): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -3.0%, secondary 1.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -1.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 523.807s -> 515.367s (-1.61%) |

View all comments
ACP: rust-lang/libs-team#755
Tracking issue: #154046
Related: #152918
Related: #155625
Description
Moves
std::io::IoSliceandstd::io::IoSliceMutintocore::io. This is required for theReadandWritetraits to be moved intoallocand/orcore, as they contain stable methods which work with IO slices. Similar to #155574, this PR inlines thestd::systypes required to create an ABI compatible type for the platforms where such compatibility is guaranteed.Additionally, I've moved the relevant tests out of
std::io::testsintocoretests::io::io_slice.Notes
std::iotoalloc#152918, but goes further than moving the IO slice types toallocand instead moves them straight tocore. Since these types have no interaction with allocation, and doing so will allowWriteto move tocore::io, I consider this a better home for these types.core::sysmodule can be found here.unsupportedtogenericto better reflect thatIoSlice(Mut)is supported by all platforms, it just doesn't have a special ABI-compatible type. I don't want to imply that parts ofcore"don't work" depending on the target;IoSlice(Mut)works exactly as expected on all targets.pubitems within each platform-specific representationpub(super)to highlight that everything withincore::io::io_sliceis an internal implementation detail not meant for any other part of the crate to be aware of.