-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Trim BorrowedCursor
API
#143829
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
Trim BorrowedCursor
API
#143829
Conversation
This method was not really useful: at no point one would only need to read the initialized part of the cursor without mutating it.
I assume that this method was there for completeness, but there is hardly any useful use of it: the buffer it gave was not always connected to the start of the cursor and its use required `unsafe` anyway to mark the bytes as initialized.
This enable removing the `start` field, so `BorrowedCursor` fits in a single register. Because `written` is almost always used in difference with another call, this changes nothing else in practice.
I'm surprised by the removal of |
Why for? It gives read-only bytes that you're not supposed to read, just like the buffer of |
I don't think me as a reviewer is the right people to make choices here. Feels like more an unresolved question on a tracking issue -- after all, one can always stabilize a subset and decide whether to remove methods later. r? libs-api |
Yes, I was, thank you. |
We discussed this during today's libs-api meeting. The changes look ok and we agree with the motivations. But there have been some recent requests to make further changes to the API that you might be interested in: |
The implementation and test changes look good to me @bors r+ |
Thanks, I didn't know about the first one! I have not sent a PR/ACP/Zulip thread yet (I don't know which you be right), but I have a draft of changing the About accessing the filled bytes, forbidding it was does on purpose, but I think that reevaluating this idea might be interesting. |
…enton Trim `BorrowedCursor` API This PR removes some method from the unstable `BorrowedCursor` type. A rational for each change can be found in the message of each commit. I don't think that an ACP is required for this, please tell me if it is not the case. Cc rust-lang#78485 rust-lang#117693
…enton Trim `BorrowedCursor` API This PR removes some method from the unstable `BorrowedCursor` type. A rational for each change can be found in the message of each commit. I don't think that an ACP is required for this, please tell me if it is not the case. Cc rust-lang#78485 rust-lang#117693
…enton Trim `BorrowedCursor` API This PR removes some method from the unstable `BorrowedCursor` type. A rational for each change can be found in the message of each commit. I don't think that an ACP is required for this, please tell me if it is not the case. Cc rust-lang#78485 rust-lang#117693
Rollup of 11 pull requests Successful merges: - #143595 (add `const_make_global`; err for `const_allocate` ptrs if didn't call) - #143678 (Added error for invalid char cast) - #143793 (Opaque type collection: Guard against endlessly recursing free alias types) - #143820 (Fixed a core crate compilation failure when enabling the `optimize_for_size` feature on some targets) - #143829 (Trim `BorrowedCursor` API) - #143856 (Linting public reexport of private dependencies) - #143891 (Port `#[coverage]` to the new attribute system) - #143914 (Reword mismatched-lifetime-syntaxes text based on feedback) - #143922 (Improve path segment joining) - #143926 (Remove deprecated fields in bootstrap) - #143975 (type_id_eq: check that the hash fully matches the type) r? `@ghost` `@rustbot` modify labels: rollup
…enton Trim `BorrowedCursor` API This PR removes some method from the unstable `BorrowedCursor` type. A rational for each change can be found in the message of each commit. I don't think that an ACP is required for this, please tell me if it is not the case. Cc rust-lang#78485 rust-lang#117693
Rollup of 15 pull requests Successful merges: - #142304 (tests: Add `RUST_BACKTRACE` and `-Cpanic` revisions to `panic-main.rs` test) - #143388 (Various refactors to the LTO handling code) - #143409 (Enable xgot feature for mips64 musl targets) - #143592 (UWP: link ntdll functions using raw-dylib) - #143595 (add `const_make_global`; err for `const_allocate` ptrs if didn't call) - #143678 (Added error for invalid char cast) - #143820 (Fixed a core crate compilation failure when enabling the `optimize_for_size` feature on some targets) - #143829 (Trim `BorrowedCursor` API) - #143846 (pass --gc-sections if -Zexport-executable-symbols is enabled and improve tests) - #143851 (ci cleanup: rustdoc-gui-test now installs browser-ui-test) - #143856 (Linting public reexport of private dependencies) - #143895 (Dont collect assoc ty item bounds from trait where clause for host effect predicates) - #143922 (Improve path segment joining) - #143964 (Fix handling of SCRIPT_ARG in docker images) - #144016 (trait_sel: `MetaSized` always holds temporarily) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 15 pull requests Successful merges: - #142304 (tests: Add `RUST_BACKTRACE` and `-Cpanic` revisions to `panic-main.rs` test) - #143388 (Various refactors to the LTO handling code) - #143409 (Enable xgot feature for mips64 musl targets) - #143592 (UWP: link ntdll functions using raw-dylib) - #143595 (add `const_make_global`; err for `const_allocate` ptrs if didn't call) - #143678 (Added error for invalid char cast) - #143820 (Fixed a core crate compilation failure when enabling the `optimize_for_size` feature on some targets) - #143829 (Trim `BorrowedCursor` API) - #143851 (ci cleanup: rustdoc-gui-test now installs browser-ui-test) - #143856 (Linting public reexport of private dependencies) - #143895 (Dont collect assoc ty item bounds from trait where clause for host effect predicates) - #143922 (Improve path segment joining) - #143964 (Fix handling of SCRIPT_ARG in docker images) - #144002 (Update poison.rs) - #144016 (trait_sel: `MetaSized` always holds temporarily) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143829 - a1phyr:trim_borrowed_buf, r=ChrisDenton Trim `BorrowedCursor` API This PR removes some method from the unstable `BorrowedCursor` type. A rational for each change can be found in the message of each commit. I don't think that an ACP is required for this, please tell me if it is not the case. Cc #78485 #117693
This PR removes some method from the unstable
BorrowedCursor
type. A rational for each change can be found in the message of each commit.I don't think that an ACP is required for this, please tell me if it is not the case.
Cc #78485 #117693