Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix all warnings from the latest Clippy #1123

Merged
merged 7 commits into from
Jul 16, 2024

Conversation

powergee
Copy link
Contributor

@powergee powergee commented Jul 4, 2024

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

  • 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).

  • doc_lazy_continuation: Some indentations are missing in the documentation.

  • 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.

@taiki-e
Copy link
Member

taiki-e commented Jul 4, 2024

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 rust-lang/rust-clippy#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
Copy link
Contributor Author

powergee commented Jul 4, 2024

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.

* Clippy's `declare_interior_mutable_const` is considered redundant
* See also: crossbeam-rs#1123 (comment)
@powergee
Copy link
Contributor Author

powergee commented Jul 5, 2024

I pushed a commit and all CI checks are passed.

* This reverts commit 5602a6c.
* `zero_repeat_side_effect` seems to be a false positive in this circumstance.
* See: crossbeam-rs#1123
@taiki-e taiki-e merged commit 2a82b61 into crossbeam-rs:master Jul 16, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants