jam1garner / binrw

A Rust crate for helping parse and rebuild binary data using ✨macro magic✨.
https://binrw.rs
MIT License
545 stars 35 forks source link

Consider declaring Minimum Supported Rust Version #268

Closed marxin closed 1 month ago

marxin commented 1 month ago

I noticed that with clippy for an older Rust release (warning: this expression creates a reference which is immediately dereferenced by the compiler):

#[derive(BinWrite)]
struct Foo {
    value: i32,
    value2: i32,
}
$ cargo +1.71 clippy
    Checking binrw-demo v0.1.0 (/home/marxin/Programming/binrw-demo)
warning: this expression creates a reference which is immediately dereferenced by the compiler
 --> src/main.rs:8:5
  |
8 |     value: i32,
  |     ^^^^^ help: change this to: `value`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
  = note: `#[warn(clippy::needless_borrow)]` on by default

warning: this expression creates a reference which is immediately dereferenced by the compiler
 --> src/main.rs:9:5
  |
9 |     value2: i32,
  |     ^^^^^^ help: change this to: `value2`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
csnover commented 1 month ago

Hi, thanks for your report! MSRV for binrw is already defined, and CI runs tests against the MSRV to make sure that binrw compiles and works correctly.

At this point I can’t justify ever spending time on fixing lints from old versions of clippy, so I’m going to close this ticket as ‘not planned’. Presumably there is some reason this code doesn’t emit warnings in newer versions and I am curious what that is, but I don’t have time to look and try to figure out why. However, if you do have the time, and you want to submit some patch that resolves the lints in the version of Rust you are using, without suppressing true positives in newer versions of Rust, and without adding extra dependencies (i.e. please don’t add some version detection library dependency just so old Rust versions don’t get warnings), please go for it. Thanks!

marxin commented 1 month ago

Thanks for the reply. Apparently, it's already fixed in clippy, so it does not make sense to investigate that further.