Skip to content

Add experimental fallible::FallibleVec - #157394

Open
ChrisDenton wants to merge 1 commit into
rust-lang:mainfrom
ChrisDenton:fallible-vec
Open

ChrisDenton wants to merge 1 commit into
rust-lang:mainfrom
ChrisDenton:fallible-vec

Conversation

@ChrisDenton

@ChrisDenton ChrisDenton commented Jun 3, 2026 •

Copy link
Copy Markdown
Member

View all comments

Tracking issue: #157392

There are a few things to be aware of here:

  1. This is still experimental. It might be removed at any time so we have to be careful about any interdependency between this and Vec
  2. Vec is a pretty crucial part of rust's standard library. We have to be extremely careful touching its code. We might regress performance or make debugging worse.

With that in mind, this PR does the minimum necessary for an MVP fallible::FallibleVec. The only actually new thing it adds is fallible push and push_mut methods. The rest just forwards to Vec equivalents. Further APIs can be added (carefully) afterwards.

I apologise for the number of lines changed in the PR but they are mostly copy/pasted docs (I wanted them for the testable examples mostly). We should probably find a better way of duplicating them but I think this works for a minimally intrusive experiment.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 3, 2026
@rustbot

rustbot commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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 8 candidates
  • Random selection from Mark-Simulacrum, jhpratt

@rust-log-analyzer

This comment has been minimized.

@ChrisDenton

Copy link
Copy Markdown
Member Author
 -	   = note: multiple `impl`s satisfying `Vec<_>: aux::Trait` found in the `unstable_impl_method_selection_aux` crate:
 -	           - impl aux::Trait for Vec<u32>;
 -	           - impl aux::Trait for Vec<u64>
 +	   = note: multiple `impl`s satisfying `std::vec::Vec<_>: aux::Trait` found in the `unstable_impl_method_selection_aux` crate:
 +	           - impl aux::Trait for std::vec::Vec<u32>;
 +	           - impl aux::Trait for std::vec::Vec<u64>

Ooph, that's unfortunate. Simplest thing is to rename this type. But I do wonder if that could be fixed.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@ChrisDenton ChrisDenton changed the title Add experimental fallible::Vec Add experimental fallible::FallibleVec Jun 4, 2026
@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2026
@Mark-Simulacrum

Copy link
Copy Markdown
Member

Let me know if this is actually waiting on a review -- for now since CI is failing I'll move it back to waiting on author.

@rustbot

rustbot commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Any special-casing of Miri in the standard library requires review.

cc @rust-lang/miri

@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@ChrisDenton
ChrisDenton force-pushed the fallible-vec branch 2 times, most recently from b98f62a to ed80ad2 Compare September 16, 2026 15:47
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@ChrisDenton

Copy link
Copy Markdown
Member Author

@Mark-Simulacrum sorry about the wait, I have more time to move this forward now. As a reminder to myself as much as you the naming "FallibleVec" is pretty much like "yeet", it's probably not what we end up with but it can act as a placeholder until we figure out that particular bikeshed. Also diagnostics need some work, but I'm told this isn't trivial so I'd rather that not be a blocker atm. As noted in the OP, this is intended to be a MVP. It's likely everything about this API will be argued over but I think we should start with something rather than wait until we've fully designed everything.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 16, 2026
@rustbot

rustbot commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

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.

@ChrisDenton

Copy link
Copy Markdown
Member Author

I removed the Allocator API stuff. When I originally made this PR it had been more or less unchanged for ages but now it's had a lot of work done and is nearing stabilization so I want to avoid stepping on any toes. It's easy to add back the Allocator methods in a follow up PR once Allocator is properly stable and things have settled a bit.

@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum Mark-Simulacrum left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with some nits fixed (or if you want to make more extensive changes then we can do more review)

View changes since this review

#[unstable(feature = "fallible_vec", issue = "157392")]
#[derive(Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
pub struct FallibleVec<T> {
buf: InfallibleVec<T>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, do you think the code is more readable with this called InfallibleVec vs. just Vec?

/// All allocating methods will return [`AllocError`][super::AllocError] if allocation fails.
/// Otherwise are the same as their infallible `Vec` counterparts.
///
/// Both `fallible::Vec` and `Vec` are fully ABI compatible, which means you can freely cast between with no cost.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Both `fallible::Vec` and `Vec` are fully ABI compatible, which means you can freely cast between with no cost.
/// Both `fallible::Vec<T>` and `Vec<T>` are fully layout and ABI compatible, which means you can freely cast between the two.

/// # Safety
///
/// This is highly unsafe, due to the number of invariants that aren't
/// checked:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to find a way to not duplicate all of the safety requirements, code examples, etc. -- maybe it makes sense to just forward to main Vec docs here and in other methods?

I think at least right now most users should understand this type as being "equivalent to" Vec, modulo some specific details, and having all the docs duplicated makes it harder to find those differences.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you noted this in the PR description -- ultimately happy to defer this but I think it'll continue being a painpoint until/after stabilization of this API surface area since we'll keep forgetting to update both places (or need to review duplicate diffs).

#[rustc_const_unstable(feature = "fallible_vec", issue = "157392")]
pub const fn to_fallible(self) -> FallibleVec<T> {
FallibleVec { buf: self }
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we provide conversions over &mut self and &self? I think we can transmute those references too, right?

/// ```
#[inline]
pub fn leak<'a>(self) -> &'a mut [T]
where {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: bad formatting

/// vector's elements to a larger allocation. This expensive operation is
/// offset by the *capacity* *O*(1) insertions it allows.
#[inline]
#[must_use = "if you don't need a reference to the value, use `Vec::push` instead"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: InfallibleVec::push

Comment on lines +907 to +909
/// As of Rust 1.57, this method does not reallocate or shrink the `FallibleVec`,
/// so the leaked allocation may include unused capacity that is not part
/// of the returned slice.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: FallibleVec would nead to make this decision prior to stabilization, since the return type would be different.


#[inline]
fn deref(&self) -> &[T] {
self.as_slice()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect the structure of this method is going to end up invalidating pointers that Vec wouldn't have? Or do we get away with it in practice? (Haven't thought too deeply about this and doesn't really block landing this, but probably worth adding to tracking issue until we have an answer)

(Specifically any guarantees or happens-to-work around e.g. spare capacity not getting invalidated when calling len())

}

/// A specialized version of `self.reserve(len, 1)` which requires the
/// caller to ensure `len == self.capacity()`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this wording suggests unsafe fn is warranted, but I guess it's pre-existing either way?

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 19, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants