search

Cydhra / vers

Succinct data structures using very efficient rank and select
Apache License 2.0
58 stars 4 forks source link

Comparison chain #1

Closed chris-ha458 closed 1 year ago

chris-ha458 commented 1 year ago

Hi! thanks for your work.

I've been looking into the code and it seems some default lints comeup with warning.

As for the if chain I assume that this has already been tested and found that the current if chain is faster.

Although the expressiveness and safety(exhaustive matching) of match is favorable, due to https://github.com/rust-lang/rust-clippy/issues/5354 if chain is often used in performance critical code like this.

I thought maybe suppressing the lints like this and documenting why could be helpful since it reduces warnings on the default clippy settings.

Cydhra commented 1 year ago

Yes the if-chain is faster in the predecessor and successor implementation, I just never got around to suppressing the lint. I'd prefer it to be allowed at the target location, though, since allowing it for the entire crate covers a lot of methods where the lint still is useful. Also, I think a non-doc comment to explain it is enough, since this isn't a detail that needs to be exposed in public documentation.

chris-ha458 commented 1 year ago

I squashed and force pushed the previous version and made two commits that each add the lint suppression in a per function basis and add non-doc comments.