Skip to content

Conversation

@a1phyr
Copy link
Contributor

@a1phyr a1phyr commented Oct 23, 2025

I haven't found any discussion on this, but I think that handling those OOM is desirable.

@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 Oct 23, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 23, 2025

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

@marmeladema
Copy link
Contributor

Seems similar to #84612

Cc @kornelski

@kornelski
Copy link
Contributor

kornelski commented Oct 27, 2025

IMHO this is a good idea. This is an io type returning io::Error, and users should be treating this API seriously as something that may fail.

It may be worth making this explicit in the documentation that it applies to Vec too.

PR #84612 got postponed in hope that oom=panic will make it unnecessary, but oom=panic is being removed! So the other PR should be revived too.

@Mark-Simulacrum Mark-Simulacrum added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. S-waiting-on-team needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-t-libs-api Status: Awaiting decision from T-libs-api and removed 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. S-waiting-on-team labels Nov 2, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 3, 2025

This PR was rebased onto a different master 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.

@a1phyr
Copy link
Contributor Author

a1phyr commented Nov 3, 2025

The use of these methods will most likely be in generic context anyway, where errors will be properly handled.

@rust-log-analyzer

This comment has been minimized.

@the8472
Copy link
Member

the8472 commented Dec 1, 2025

@the8472
Copy link
Member

the8472 commented Jan 20, 2026

We discussed this during today's libs-API meeting. The issue with this is that there's a lot of code out there that assumes that write!(&mut vec, ...) will never error and thus doesn't check the result.
In principle such code is wrong and we would be allowed to break it, but there's too much of it out there and the gain is too small, especially considering that linux systems will only return actually surface the error on enormous allocation requests. We also discussed whether we would retroactively bless this use by guaranteeing that this Write impl will always return Ok() or abort, but that didn't reach a conclusion and would require a separate proposal.

So instead we'll consider this a part of a future fallible-Vec API, something that remains unsolved. E.g. if we ever introduced a vec::fallible::Vec, then its Write impl would use try_reserve.
Until then people can work around this with a wrapper Write type that does call try_reserve before write.

@the8472 the8472 closed this Jan 20, 2026
@marmeladema
Copy link
Contributor

We discussed this during today's libs-API meeting. The issue with this is that there's a lot of code out there that assumes that write!(&mut vec, ...) will never error and thus doesn't check the result.

Do we have data to back that claim? Additionally, the return value of write! being a Result, the compiler should already complain in such case since the type is marked as must_use right?
I personally had to explicitly unwrap in all my write! calls in the past so I am fairly surprised by the claim that "a lot of code out there doesn't check the result". Seems like this code must be explicitly ignoring compiler warnings?

@the8472
Copy link
Member

the8472 commented Jan 20, 2026

We basically looked at code searches, https://github.com/search?q=lang%3ARust+%22_+%3D+write%21%22&type=code

@a1phyr
Copy link
Contributor Author

a1phyr commented Jan 25, 2026

How much of this actually uses Vec<u8>? While navigating your link, almost all uses seem to be on String, or stdout/stderr.

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

Labels

needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-t-libs-api Status: Awaiting decision from T-libs-api T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants