Make `BorrowedCursor<'a, T>` covariant in `'a` and drop an indirection by Ddystopia · Pull Request #160563 · rust-lang/rust

Ddystopia · 2026-08-05T12:41:44Z

This is a solution to #117693 (comment), with some improvements.

Currently 'a in BorrowedCursor is invariant, though people seem to talk about it as if it were covariant, and the feature is in FCP right now. The previous version with 'buf and 'data lifetimes had the same flaw: 'data was invariant.

A later PR landed that merged them and said that BorrowedCursor manually ensures that 'data won't be ever overwritten thus invariance should not be needed. But unfortunately the lifetime is still left invariant. You can see it here, and the error spells it out exactly:

#![feature(core_io_borrowed_buf)]

use std::io::{BorrowedBuf, BorrowedCursor};

// Accepted.
fn buf_covariant<'short, 'long: 'short>(buf: BorrowedBuf<'long, u8>) -> BorrowedBuf<'short, u8> {
    buf
}

// Rejected.
fn cursor_covariant<'short, 'long: 'short>(
    cursor: BorrowedCursor<'long, u8>,
) -> BorrowedCursor<'short, u8> {
    cursor
}

// Rejected.
fn cursor_contravariant<'short, 'long: 'short>(
    cursor: BorrowedCursor<'short, u8>,
) -> BorrowedCursor<'long, u8> {
    cursor
}

fn main() {}

And the errors (also say that BorrowedCursor is invariant over 'a):

error: lifetime may not live long enough
  --> src/main.rs:14:5
   |
11 | fn cursor_covariant<'short, 'long: 'short>(
   |                     ------  ----- lifetime `'long` defined here
   |                     |
   |                     lifetime `'short` defined here
...
14 |     cursor
   |     ^^^^^^ function was supposed to return data with lifetime `'long` but it is returning data with lifetime `'short`
   |
   = help: consider adding the following bound: `'short: 'long`
   = note: requirement occurs because of the type `BorrowedCursor<'_, u8>`, which makes the generic argument `'_` invariant
   = note: the struct `BorrowedCursor<'a, T>` is invariant over the parameter `'a`
   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance

error: lifetime may not live long enough
  --> src/main.rs:21:5
   |
18 | fn cursor_contravariant<'short, 'long: 'short>(
   |                         ------  ----- lifetime `'long` defined here
   |                         |
   |                         lifetime `'short` defined here
...
21 |     cursor
   |     ^^^^^^ function was supposed to return data with lifetime `'long` but it is returning data with lifetime `'short`
   |
   = help: consider adding the following bound: `'short: 'long`
   = note: requirement occurs because of the type `BorrowedCursor<'_, u8>`, which makes the generic argument `'_` invariant
   = note: the struct `BorrowedCursor<'a, T>` is invariant over the parameter `'a`
   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance

error: could not compile `play` (bin "play") due to 2 previous errors

This also removes the two mem::transmute calls that unfilled and reborrow used to shorten &'this mut BorrowedBuf<'data, T> into &'this mut BorrowedBuf<'this, T>.

Additionally I noticed that BorrowedCursor is not really as efficient as it could be, for a standard library: it contained a reference to the BorrowedBuf, which in turn contains a slice to the data. Without this, the fix is just replacing &'a mut BorrowedBuf<'a, T> with NonNull<BorrowedBuf<'a, T>>, plus some convenience helpers.

To fix this, I also stored a reborrowed pointer to the first element of the array, with the provenance to access the whole array. filled and init are still read from the pointer to BorrowedBuf, the buffer length is also read from it but carefully, in order to not create a retag which will trigger a foreign access to the pointer stored in BorrowedCursor, making it disabled. It increased the size of BorrowedCursor from one usize to two of them.

It is stored as the pointer rather than &mut [MaybeUninit<T>] to save a usize from the BorrowedCursor size.

rustbot · 2026-08-05T12:41:49Z

r? @jhpratt

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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: libs
libs expanded to 12 candidates
Random selection from JohnTitor, Mark-Simulacrum, clarfonthey, jhpratt, nia-e

jhpratt · 2026-08-06T04:41:54Z

Stepping away from reviews temporarily.

@rustbot reroll

Ddystopia · 2026-08-10T15:26:47Z

I'm not sure if the mention from rustbot worked correctly, so I'll repeat it myself: @clarfonthey

clarfonthey · 2026-08-10T21:50:04Z

It did, I've just been slow getting to this change. Feel free to @ me whenever I'm slow getting to reviews.

I'll take a look at this later. The main thing that caused me to pause a bit when I initially looked over this was if there's a way to ensure covariance without using NonNull (probably would require something ridiculous like &[Cell<T>]) but I couldn't figure out if there was actually anything reasonable for it.

Ddystopia · 2026-08-11T05:40:48Z

I think the same could be implemented without unsafe (modulo touching BorrowedBuf's invariants, but that's beyond the scope) like this:

struct BorrowedCursor<'a, T> {
    data: &'a mut [MaybeUninit<T>],
    filled: &'a mut usize,
    init: &'a mut usize,
}

In other words, cursor will fully reborrow the buffer and other things, without the indirection that causes invariance.

But it will blow up the size of BorrowedCursor to 4 * size_of::<usize>(). As well as any other approach icluding the slice, because the slice is already 2 words while filled and init still have to be referenced. Well, init could be stored as the highest bit inside filled to reduce the size by one word.

I get that it may be harder to maintain, but a) I believe the standard library is expected to provide efficiency, and b) the unsafe code touching the invariants is kind of localized to a single small impl block, so it is not spread through the whole module. Well, it was my intention at least.

Also the code will look a lot less scary if the BorrowedBuf will store the buffer as the pointer and the length, with a simple method that will return the slice on demand (so basically a couple of sites get additional ()). That way there is no need for this game to read the length out of &mut [] without triggering retagging.

rustbot · 2026-08-11T11:12:20Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Ddystopia · 2026-08-11T11:52:10Z

I added another version of this PR, which is more elegant and less error prone in my opinion, but touches BorrowedBuf too. improve-borrowed-cursor-v2 branch.

Ddystopia · 2026-08-18T06:04:13Z

@clarfonthey hi, could this be a good time?

clarfonthey · 2026-08-21T10:59:07Z

+    // Safety invariant: this points to the start of the *whole* buffer of `*borrowed_buf` and is
+    // valid for reads and writes of `(*borrowed_buf).buf.len()` elements, so that
+    // `(*borrowed_buf).filled` indexes into it.
+    buf: NonNull<MaybeUninit<T>>,
+    /// The buffer this cursor was created from.
+    // Safety invariants:
+    // 1. `(*borrowed_buf).buf` is *never* accessed by the owner of the pointee while the `buf`
+    //    field above is alive, because there is a `&mut` of the pointee while the cursor is alive.
+    // 2. We promise to only access the `filled` and `init` fields and the metadata of the `buf`
+    //    field through the `borrowed_buf` pointer, never triggering any retag of `buf`'s pointer,
+    //    as the `buf` field above holds a reborrow of it and reaching the parent again would be a
+    //    foreign access for that reborrow. This includes not making a reference to the whole
+    //    pointee out of `borrowed_buf`, but only accessing those fields directly through pointer
+    //    manipulation.


Technically doing what the code was doing before, but in general, I would recommend just making the invariant documentation also doc comments, since they're useful if you document the private items with rustdoc.

View changes since the review

I force-pushed to make them doc comments instead of plain comments.

clarfonthey · 2026-08-21T11:08:44Z

Sorry I took so long to get to this. This all looks good; I have one small note about comments, and I think it would be nice if we explicitly ran some of this code through miri in the tests to make sure that there aren't any additional changes we need to add to avoid improper tagging.

None of those are blocking however, so, I'll give you a bit of time to respond to my comments and can still merge later if you don't have time. (r=me, basically)

This is a solution to rust-lang#117693 (comment), with some improvements. Currently `'a` in `BorrowedCursor` is invariant, though people seem to talk about it as if it were covariant, and the feature is in FCP right now. The previous version with `'buf` and `'data` lifetimes had the same flaw: `'data` was invariant. A later PR landed that merged them and said that `BorrowedCursor` manually ensures that `'data` won't be ever overwritten thus invariance should not be needed. But unfortunately the lifetime is still left invariant. You can see it here, and the error spells it out exactly: ```rust use std::io::{BorrowedBuf, BorrowedCursor}; // Accepted. fn buf_covariant<'short, 'long: 'short>(buf: BorrowedBuf<'long, u8>) -> BorrowedBuf<'short, u8> { buf } // Rejected. fn cursor_covariant<'short, 'long: 'short>( cursor: BorrowedCursor<'long, u8>, ) -> BorrowedCursor<'short, u8> { cursor } // Rejected. fn cursor_contravariant<'short, 'long: 'short>( cursor: BorrowedCursor<'short, u8>, ) -> BorrowedCursor<'long, u8> { cursor } fn main() {} ``` And the errors (also say that `BorrowedCursor` is invariant over `'a`): ``` error: lifetime may not live long enough --> src/main.rs:14:5 | 11 | fn cursor_covariant<'short, 'long: 'short>( | ------ ----- lifetime `'long` defined here | | | lifetime `'short` defined here ... 14 | cursor | ^^^^^^ function was supposed to return data with lifetime `'long` but it is returning data with lifetime `'short` | = help: consider adding the following bound: `'short: 'long` = note: requirement occurs because of the type `BorrowedCursor<'_, u8>`, which makes the generic argument `'_` invariant = note: the struct `BorrowedCursor<'a, T>` is invariant over the parameter `'a` = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance error: lifetime may not live long enough --> src/main.rs:21:5 | 18 | fn cursor_contravariant<'short, 'long: 'short>( | ------ ----- lifetime `'long` defined here | | | lifetime `'short` defined here ... 21 | cursor | ^^^^^^ function was supposed to return data with lifetime `'long` but it is returning data with lifetime `'short` | = help: consider adding the following bound: `'short: 'long` = note: requirement occurs because of the type `BorrowedCursor<'_, u8>`, which makes the generic argument `'_` invariant = note: the struct `BorrowedCursor<'a, T>` is invariant over the parameter `'a` = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance error: could not compile `play` (bin "play") due to 2 previous errors ``` This also removes the two `mem::transmute` calls that `unfilled` and `reborrow` used to shorten `&'this mut BorrowedBuf<'data, T>` into `&'this mut BorrowedBuf<'this, T>`. They were sound only as long as nobody ever assigned into `BorrowedCursor::buf`, which the cursor can no longer do at all, since it never holds a `BorrowedBuf` reference now. --- Additionally I noticed that `BorrowedCursor` is not really as efficient as it could be, for a standard library: it contained a reference to the `BorrowedBuf`, which in turn contains a slice to the data. Without this, the fix is just replacing `&'a mut BorrowedBuf<'a, T>` with `NonNull<BorrowedBuf<'a, T>>`, plus some convenience helpers. To fix this, I also stored a reborrowed pointer to the first element of the array, with the provenance to access the whole array. `filled` and `init` are still read from the pointer to `BorrowedBuf`, the buffer length is also read from it but carefully, in order to not create a retag which will trigger a foreign access to the pointer stored in `BorrowedCursor`, making it disabled. It increased the size of `BorrowedCursor` from one `usize` to two of them. It is stored as the pointer rather than `&mut [MaybeUninit<T>]` to save a `usize` from the `BorrowedCursor` size.

Ddystopia · 2026-08-21T12:07:24Z

and I think it would be nice if we explicitly ran some of this code through miri in the tests to make sure that there aren't any additional changes we need to add to avoid improper tagging.

Well, I once pulled out the implementation to the other crate and run miri tests on it and it came out clean, but you probably want something more concrete?

Ddystopia · 2026-08-21T12:24:53Z

Right now with the condition of cargo llvm-cov getting 100% coverage I asked AI to build tests, and they pass miri. Regardless of test quality, just the fact that all code is covered means miri touched every path and didn't bail, so it should be sufficient? It also generated canary test that just produces UB to ensure UB is visible.

Do you consider this as evidence (it is fine if not)? I attach the tar below.

borrowed_buf_miri.tar.gz

clarfonthey · 2026-08-21T12:26:02Z

and I think it would be nice if we explicitly ran some of this code through miri in the tests to make sure that there aren't any additional changes we need to add to avoid improper tagging.

Well, I once pulled out the implementation to the other crate and run miri tests on it and it came out clean, but you probably want something more concrete?

Right, I figured that they pass now; my main concern is as miri works on tree borrows and other changes whether they will continue to pass in the future. But like I said, I don't think that's something we need to figure out now.

clarfonthey · 2026-08-21T12:26:41Z

On that note:

@bors r+ rollup

Thank you for these changes!

rust-bors · 2026-08-21T12:26:49Z

📌 Commit c7de933 has been approved by clarfonthey

It is now in the queue for this repository.

Rollup of 20 pull requests Successful merges: - #161259 (move some attribute related structs out of rustc_attr_ir) - #160853 (delegation: simplify matches on `FnKind`, minor refactorings) - #161161 (proc_macro: add support for 16-bit targets) - #159899 ( `GenericArgs::types` triage + possible fixes) - #160459 (Use attribute parser for `deprecated` attribute checking) - #160536 (Inline `String::into_raw_parts` and reuse `from_utf16` optimization) - #160563 (Make `BorrowedCursor<'a, T>` covariant in `'a` and drop an indirection) - #160595 (Clarify `str::split_at` docs) - #160813 (Optimize linked list iterator performance) - #161271 (doc: document safety requirements for core WTF-8) - #161317 (LLVM 24: configure float-abi via module flag) - #161320 (Only suggest `RUST_MIN_STACK` if maybe stack overflow) - #161369 (Add regression test for confusing lifetime error message issue) - #161393 (Configure LLM policy URL for triagebot) - #161403 (splat-fn-ptr-ptr-tuple.rs: add `let` to avoid UB) - #161409 (Add back `tests/rustdoc-gui/notable-trait.goml` test) - #161410 (Fix rustdoc remapping `documentation` scope documentation) - #161415 (Update expect messages in path docs to better follow guidelines) - #161438 (Change triagebot backport to ping T-libs-fcp) - #161442 (Add regression test for dead code on type alias used in impl self type) Failed merges: - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`)

Rollup merge of #160563 - Ddystopia:improve-buffer-cursor, r=clarfonthey Make `BorrowedCursor<'a, T>` covariant in `'a` and drop an indirection This is a solution to #117693 (comment), with some improvements. Currently `'a` in `BorrowedCursor` is invariant, though people seem to talk about it as if it were covariant, and the feature is in FCP right now. The previous version with `'buf` and `'data` lifetimes had the same flaw: `'data` was invariant. A later PR landed that merged them and said that `BorrowedCursor` manually ensures that `'data` won't be ever overwritten thus invariance should not be needed. But unfortunately the lifetime is still left invariant. You can see it here, and the error spells it out exactly: ```rust #![feature(core_io_borrowed_buf)] use std::io::{BorrowedBuf, BorrowedCursor}; // Accepted. fn buf_covariant<'short, 'long: 'short>(buf: BorrowedBuf<'long, u8>) -> BorrowedBuf<'short, u8> { buf } // Rejected. fn cursor_covariant<'short, 'long: 'short>( cursor: BorrowedCursor<'long, u8>, ) -> BorrowedCursor<'short, u8> { cursor } // Rejected. fn cursor_contravariant<'short, 'long: 'short>( cursor: BorrowedCursor<'short, u8>, ) -> BorrowedCursor<'long, u8> { cursor } fn main() {} ``` And the errors (also say that `BorrowedCursor` is invariant over `'a`): ``` error: lifetime may not live long enough --> src/main.rs:14:5 | 11 | fn cursor_covariant<'short, 'long: 'short>( | ------ ----- lifetime `'long` defined here | | | lifetime `'short` defined here ... 14 | cursor | ^^^^^^ function was supposed to return data with lifetime `'long` but it is returning data with lifetime `'short` | = help: consider adding the following bound: `'short: 'long` = note: requirement occurs because of the type `BorrowedCursor<'_, u8>`, which makes the generic argument `'_` invariant = note: the struct `BorrowedCursor<'a, T>` is invariant over the parameter `'a` = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance error: lifetime may not live long enough --> src/main.rs:21:5 | 18 | fn cursor_contravariant<'short, 'long: 'short>( | ------ ----- lifetime `'long` defined here | | | lifetime `'short` defined here ... 21 | cursor | ^^^^^^ function was supposed to return data with lifetime `'long` but it is returning data with lifetime `'short` | = help: consider adding the following bound: `'short: 'long` = note: requirement occurs because of the type `BorrowedCursor<'_, u8>`, which makes the generic argument `'_` invariant = note: the struct `BorrowedCursor<'a, T>` is invariant over the parameter `'a` = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance error: could not compile `play` (bin "play") due to 2 previous errors ``` This also removes the two `mem::transmute` calls that `unfilled` and `reborrow` used to shorten `&'this mut BorrowedBuf<'data, T>` into `&'this mut BorrowedBuf<'this, T>`. --- Additionally I noticed that `BorrowedCursor` is not really as efficient as it could be, for a standard library: it contained a reference to the `BorrowedBuf`, which in turn contains a slice to the data. Without this, the fix is just replacing `&'a mut BorrowedBuf<'a, T>` with `NonNull<BorrowedBuf<'a, T>>`, plus some convenience helpers. To fix this, I also stored a reborrowed pointer to the first element of the array, with the provenance to access the whole array. `filled` and `init` are still read from the pointer to `BorrowedBuf`, the buffer length is also read from it but carefully, in order to not create a retag which will trigger a foreign access to the pointer stored in `BorrowedCursor`, making it disabled. It increased the size of `BorrowedCursor` from one `usize` to two of them. It is stored as the pointer rather than `&mut [MaybeUninit<T>]` to save a `usize` from the `BorrowedCursor` size.

Rollup of 20 pull requests Successful merges: - rust-lang/rust#161259 (move some attribute related structs out of rustc_attr_ir) - rust-lang/rust#160853 (delegation: simplify matches on `FnKind`, minor refactorings) - rust-lang/rust#161161 (proc_macro: add support for 16-bit targets) - rust-lang/rust#159899 ( `GenericArgs::types` triage + possible fixes) - rust-lang/rust#160459 (Use attribute parser for `deprecated` attribute checking) - rust-lang/rust#160536 (Inline `String::into_raw_parts` and reuse `from_utf16` optimization) - rust-lang/rust#160563 (Make `BorrowedCursor<'a, T>` covariant in `'a` and drop an indirection) - rust-lang/rust#160595 (Clarify `str::split_at` docs) - rust-lang/rust#160813 (Optimize linked list iterator performance) - rust-lang/rust#161271 (doc: document safety requirements for core WTF-8) - rust-lang/rust#161317 (LLVM 24: configure float-abi via module flag) - rust-lang/rust#161320 (Only suggest `RUST_MIN_STACK` if maybe stack overflow) - rust-lang/rust#161369 (Add regression test for confusing lifetime error message issue) - rust-lang/rust#161393 (Configure LLM policy URL for triagebot) - rust-lang/rust#161403 (splat-fn-ptr-ptr-tuple.rs: add `let` to avoid UB) - rust-lang/rust#161409 (Add back `tests/rustdoc-gui/notable-trait.goml` test) - rust-lang/rust#161410 (Fix rustdoc remapping `documentation` scope documentation) - rust-lang/rust#161415 (Update expect messages in path docs to better follow guidelines) - rust-lang/rust#161438 (Change triagebot backport to ping T-libs-fcp) - rust-lang/rust#161442 (Add regression test for dead code on type alias used in impl self type) Failed merges: - rust-lang/rust#160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`)

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 5, 2026

rustbot assigned jhpratt Aug 5, 2026

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Aug 5, 2026

Ddystopia force-pushed the improve-buffer-cursor branch from a23dd99 to 920ec31 Compare August 5, 2026 12:47

rustbot assigned clarfonthey and unassigned jhpratt Aug 6, 2026

This comment has been minimized.

Sign in to view

Ddystopia force-pushed the improve-buffer-cursor branch from 920ec31 to a94449b Compare August 11, 2026 11:12

clarfonthey reviewed Aug 21, 2026

View reviewed changes

Comment thread library/core/src/io/borrowed_buf.rs

clarfonthey reviewed Aug 21, 2026

View reviewed changes

Ddystopia force-pushed the improve-buffer-cursor branch from a94449b to c7de933 Compare August 21, 2026 12:05

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 21, 2026

jhpratt mentioned this pull request Aug 21, 2026

Rollup of 20 pull requests #161458

Merged

rust-bors Bot merged commit f4870fd into rust-lang:main Aug 21, 2026
13 checks passed

rustbot added this to the 1.100.0 milestone Aug 21, 2026

Uh oh!

Uh oh!

Conversation

Ddystopia commented Aug 5, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 5, 2026

Uh oh!

jhpratt commented Aug 6, 2026

Uh oh!

Ddystopia commented Aug 10, 2026

Uh oh!

clarfonthey commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ddystopia commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Aug 11, 2026

Uh oh!

Ddystopia commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ddystopia commented Aug 18, 2026

Uh oh!

Uh oh!

clarfonthey Aug 21, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ddystopia Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

clarfonthey commented Aug 21, 2026

Uh oh!

Ddystopia commented Aug 21, 2026

Uh oh!

Ddystopia commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clarfonthey commented Aug 21, 2026

Uh oh!

clarfonthey commented Aug 21, 2026

Uh oh!

rust-bors Bot commented Aug 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ddystopia commented Aug 5, 2026 •

edited by rustbot

Loading

clarfonthey commented Aug 10, 2026 •

edited

Loading

Ddystopia commented Aug 11, 2026 •

edited

Loading

Ddystopia commented Aug 11, 2026 •

edited

Loading

clarfonthey Aug 21, 2026 •

edited by rustbot

Loading

Ddystopia commented Aug 21, 2026 •

edited

Loading