Skip to content

Don't panic in VecInner::extend and return a Result - #642

Open
sirhcel wants to merge 1 commit into
rust-embedded:mainfrom
sirhcel:vec-extend-with-result
Open

Don't panic in VecInner::extend and return a Result#642
sirhcel wants to merge 1 commit into
rust-embedded:mainfrom
sirhcel:vec-extend-with-result

Conversation

@sirhcel

Copy link
Copy Markdown
Member

Other methods for extending VecInner already return results instead of panicking. In case of VecInner::extend, avoiding a panic by checking that the actual amount of elements to extend with fits beforehand is not generally possible.

So attempt to add the elements from the iterator and restore the original length in case of an error.

This is a semver-breaking change.

@sirhcel
sirhcelforce-pushed the vec-extend-with-result branch from fcec20e to 28542e2CompareJanuary 22, 2026 20:58
Other methods for extending VecInner already return results instead of
panicking. In case of VecInner::extend, avoiding a panic by checking
that the actual amount of elements to extend with fits beforehand is not
generally possible.
So attempt to add the elements from the iterator and restore the
original length in case of an error.
@sirhcel
sirhcelforce-pushed the vec-extend-with-result branch from 28542e2 to f6f8aa5CompareJanuary 22, 2026 21:05
@sgued

Copy link
Copy Markdown
Contributor

This goes hand in hand with the panicking FromIterator implementations.

There was some discussion of the topic in #442.

My opinion is that for the next breaking release we should aim to remove implicitly panicking APIs.

@sguedsgued added the API Break Breaking changes label Jan 23, 2026
@sirhcel

Copy link
Copy Markdown
MemberAuthor

This goes hand in hand with the panicking FromIterator implementations.

There was some discussion of the topic in #442.

Thank you for the references! The latter is a great read on the pros and cons. Do you mean the former in the sense that there should be a fallible conversion like TryFromIterator instead?

My opinion is that for the next breaking release we should aim to remove implicitly panicking APIs.

I would prefer this too. Especially for operations involving iterators where getting to know the actual length/number of items to return look non-trivial to me and opting in to panicking with unwrapping the result seems way less annoying to me than writing all the pre-checks for ensuring that an operation won't panic.

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

Labels

API BreakBreaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@sirhcel@sgued