search

gwenn / lemon-rs

LALR(1) parser generator for Rust based on Lemon + SQL parser
The Unlicense
48 stars 10 forks source link

fix incorrect unsafe code #25

Closed MarinPostma closed 1 year ago

MarinPostma commented 1 year ago

Fix an incorrect usage of unsafe.

The bug was that the scan method was using unsafe to return a reference to its internal buffer. This is incorrect, because the buffer can be overwritten, by either a call to consume or fill_buf.

This fix proposes to return owned slices in the form of SmallVec instead of references to the input. A default size of 16 bytes is chosen, which should prevent any allocation, most of the time.

This allows the removal of the unsafe bit, and also simplifies the code, by removing lifetime bounds. The cost of this operation is a memcpy in most case, which is cheap.

There may be a solution that only works with references, but it would probably require significant changes.

fix #23

gwenn commented 1 year ago

Thanks for your help. I would like to try dropping support for streaming / buf_redux and fix #23 and #21 (one can still use mmap if needed). Neither SQLite3 lexer nor lalrpop support streaming (but zero-copy). But don't close this PR because I am not sure how long that is going to take. Is it ok for you or you need a fix now ?

gwenn commented 1 year ago

See https://github.com/gwenn/lemon-rs/pull/26/files#diff-3f786ccab1066df60ecf2d56744fa4fbc9b94efe78a12f66d0a5f986e81038baR100 (no unsafe / transmute anymore but no streaming anymore)