crossbeam-rs / crossbeam

Tools for concurrent programming in Rust
Apache License 2.0
7.46k stars 469 forks source link

Fix all warnings from the latest Clippy #1123

Closed powergee closed 4 months ago

powergee commented 4 months ago

The Clippy from the stable channel now produces new warnings, which this PR addresses. Specifically, those warnings are:

taiki-e commented 4 months ago

Thanks!

  • declare_interior_mutable_const: For efficient Block initialization, a constant Slot is used, but it has interior mutability, which Clippy warns about. I believe that using the inline-const expression in Block::new is ideal, but it is experimental in the current MSRV (1.61). Other solutions either have performance overheads (e.g., array::map, array::from_fn) or require nightly features (e.g., MaybeUninit::uninit_array).

AFAIK this lint is completely useless (borrow_interior_mutable_const covers actual footguns about interior_mutable_const. see also https://github.com/rust-lang/rust-clippy/issues/7665) I would prefer ignoring this lint at workspace level like my other projects.

  • zero_repeat_side_effects: When there are no explicit cases for the select! macro, it creates a zero-sized array with initialization that calls a function. Clippy is concerned if the function has side effects. In our case, it has no side effects and can be trivially resolved.

I feel it is a clippy bug that this lint warns of a code that has no side effects...

powergee commented 4 months ago

I would prefer ignoring this lint at workspace level like my other projects.

I agree with your suggestion. I will add another commit to ignore the declare_interior_mutable_const warning at the module level. I found that there is another line that ignores the same warning 🙃.

I feel it is a clippy bug that this lint warns of a code that has no side effects...

It also seems strange to me because crossbeam_channel::never clearly does not have any side effects, and even changing that function to const does not resolve the warning. It might be worth looking into this issue more deeply if I have time.

powergee commented 4 months ago

I pushed a commit and all CI checks are passed.