zopfli-rs / zopfli

A Rust implementation of the Zopfli compression algorithm.
Apache License 2.0
37 stars 4 forks source link

Fix clippy::missing_const_for_fn #20

Closed Pr0methean closed 1 year ago

Pr0methean commented 1 year ago

This PR makes functions const when clippy (with the opt-in rule missing_const_for_fn) suggests they can be. This should improve performance.

Also fixes one warning that a cfg annotation can be simplified.

AlexTMjugador commented 1 year ago

According to my experience with profiling Zopfli, the Rust compiler already precomputes const data and inlines most functions. As a keyword, const is semantically closer to "this thing must be able to be evaluated at build time if parameters are known at build time, but it may be executed at runtime anyway" than to "this thing is always evaluated at build time".

Of course however, I welcome being proved wrong if this helps!

mqudsi commented 1 year ago

Basically, rust already pre-compiles anything it can at compile time. const fn just means that this function is guaranteed (now and for future semver-compatible releases) to be callable at compile-time in a const or static context. These functions you've decorated aren't used at compile-time. There's no need for them to be const and as I've mentioned, there is an implicit maintenance burden to making them const.

I'm closing this issue out, but thanks for trying.