-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Implement most of RFC 2930, providing the ReadBuf abstraction #81156
Conversation
r? @kennytm (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
@drmeepster do you mind squashing the commits? 41 is a lot to read through. |
0e763a6
to
bc98fc9
Compare
Alright I squashed it. |
This comment has been minimized.
This comment has been minimized.
Hopefully, weird errors in files I didn't touch don't show up this time. |
This comment has been minimized.
This comment has been minimized.
uh I think this PR is cursed |
This comment has been minimized.
This comment has been minimized.
We debugged this locally, here's an MCVE:
Notice how the error unexpectedly cuts off after |
The |
Very excited, thanks for working on this! I don't have a lot of experience with larger std changes like this, but I feel like it would be easier to land this if you focused on getting an initial PR merged that only includes the |
This comment has been minimized.
This comment has been minimized.
775f103
to
e8c956f
Compare
This comment has been minimized.
This comment has been minimized.
e8c956f
to
0cd1075
Compare
This comment has been minimized.
This comment has been minimized.
r? @KodrAus |
r? @sfackler |
In the interests of collecting possible naming considerations in one place:
|
@joshtriplett which parts will be insta-stable? |
We reviewed this in today's @rust-lang/libs-api meeting, and confirmed that there are no insta-stable bits of this. We do see two potential issues with this that should be covered and addressed in the tracking issue:
However, since this isn't insta-stable, we can address both of those through further development on nightly. @rfcbot cancel @bors r+ |
@joshtriplett proposal cancelled. |
📌 Commit cd23799 has been approved by |
FWIW, I think it would be nice to squash the commits here, but it doesn't seem worth blocking over and it looks like the feature letting upstream maintainers push to the branch isn't turned on for this PR. |
☀️ Test successful - checks-actions |
Finished benchmarking commit (3b263ce): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
// | ||
// - Only the standard library gets to soundly "ignore" this, | ||
// based on its privileged knowledge of unstable rustc | ||
// internals; |
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.
Ah, I am so happy to see this comment disappear. :)
…yaahc kmc-solid: Add `std::sys::solid::fs::File::read_buf` This PR adds `std::sys::solid::fs::File::read_buf` to catch up with the changes introduced by rust-lang#81156 and fix the [`*-kmc-solid_*`](https://doc.rust-lang.org/nightly/rustc/platform-support/kmc-solid.html) Tier 3 targets..
This replaces the
Initializer
abstraction for permitting reading into uninitialized buffers, closing #42788.This leaves several APIs described in the RFC out of scope for the initial implementation:
ReadBufs
Closes #42788, by removing the relevant APIs.