rust-bakery / nom

Rust parser combinator framework
MIT License
9.18k stars 792 forks source link

Suggestion: fallible allocation #1629

Closed edgarogh closed 1 year ago

edgarogh commented 1 year ago

Let's start by being clear: this suggestion may be seen as overly pedantic to nom's standards. It's up to the maintainer to judge it based on his idea of nom. But apart from adding a few lines of code, I'm pretty sure it doesn't affect runtime performance at all, so it may still be preferable adding it to upstream nom instead of a separate crate.

What ?

The idea would be to make all allocating parsers (most if not all parsers from nom::multi + maybe others) use fallible allocation, so the parser can gracefully error in case a memory allocation is unsuccessful.

Why ?

On modern systems (notably Linux), fallible allocation is pretty much useless because the OS tends to let processes allocate more memory than there is physically and may trigger an OOM at any time, often only when the memory is accessed.

However, on smaller computers and microcontrollers, allocators tend to allocate "live". That's also typically the kind of systems where memory is the most limited.

How ?

The Vec::try_reserve method can be called before each Vec::push with 1 as its parameter, while its result error can be mapped to an ErrorKind::TooLarge — for instance — and thrown (?) immediately with an Err::Failure.

Drawbacks

epage commented 1 year ago

One challenge with this is the multis were switched to be generic over any Extend in #1621.

An alternative when it comes to custom collection operations is using the fold combiantors. The downside is they don't support the accumulate operation to fail. try_fold functions are needed to support that (see #1411).

edgarogh commented 1 year ago

One challenge with this is the multis were switched to be generic over any Extend

Very good point. I should've checked the latest commits instead of the latest published version :grimacing:

In this case, my suggestion isn't relevant anymore, and fallible allocators do actually belong in a separate crate. Maybe in a distant future Extend will have provided methods to try to allocate fallibly for implementors that support it...