Make Clippy stricter #239
Make Clippy stricter #239
Conversation
…want to make the GIL lifetime explicit.
It seems preferable to have a clean CI based on stable Clippy, than to accomodate a not-yet-released version.
|
Sorry for the back and forth w.r.t. Clippy lints, I am running nightly locally but I do not think this is something that should be encouraged by the CI as the Clippy lints change quite often. |
| @@ -448,7 +448,7 @@ impl<T: Element, D: Dimension> PyArray<T, D> { | |||
| ID: IntoDimension<Dim = D>, | |||
| { | |||
| let dims = dims.into_dimension(); | |||
| let data_ptr = data_ptr.unwrap_or(boxed_slice.as_ptr()); | |||
| let data_ptr = data_ptr.unwrap_or_else(|| boxed_slice.as_ptr()); | |||
There was a problem hiding this comment.
I should have pointed out when reviewing (I thought recent clippy is OK with this...)
There was a problem hiding this comment.
Exactly, current nightly Clippy actually wants the version without the closure whereas current stable Clippy complains about it. I think nightly Clippy is right, as boxed_slice.as_ptr() does not cost anything that should be delayed by wrapping it in a closure. So I expect to change this again with the next stable Clippy release. But as written above, the most important thing is to have a systematic check of this stuff via the CI.
|
Can the clippy config be added to Might not be the nicest way to handle it, but it's quite common. This way any contributor can just run |
The allow lifetimes part, yes. The deny warnings part not as this might break builds. Will add this to #251 as I touch the Clippy flags there anyway. |
|
What I found useful in the crates that I maintain is doing something like this at crate root: #![warn(clippy::pedantic, clippy::nursery, clippy::all)]
#![allow(clippy:this, clippy:that, clippy:whatever)]There's tons of useful lints out there that are not enabled by default (but there's also some e.g. related to crate metadata that can be disabled). |
But disable needless-lifetimes lint as we often want to make the GIL lifetime explicit.