Skip to content

False positive of zero_repeat_side_effects with a const function #13110

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

Closed
powergee opened this issue Jul 16, 2024 · 4 comments · Fixed by #13116
Closed

False positive of zero_repeat_side_effects with a const function #13110

powergee opened this issue Jul 16, 2024 · 4 comments · Fixed by #13116
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@powergee
Copy link

Summary

When a zero-sized array is declared with a const initializer function, a zero_repeat_side_effects warning is generated, despite the initializer being constant.

Lint Name

zero_repeat_side_effects

Reproducer

I tried this code:

const fn const_init() -> usize {
    0
}

fn main() {
    let data = [const_init(); 0];
    std::hint::black_box(data);
}

I saw this happen:

warning: function or method calls as the initial value in zero-sized array initializers may cause side effects
 --> rust-template/src/bin/playground3.rs:6:5
  |
6 |     let data = [const_init(); 0];
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `const_init(); let data: [usize; 0] = [];`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#zero_repeat_side_effects
  = note: `#[warn(clippy::zero_repeat_side_effects)]` on by default

warning: `rust-template` (bin "playground3") generated 1 warning

I expected to see no warning from the Clippy.

Version

rustc 1.81.0-nightly (24d2ac0b5 2024-07-15)
binary: rustc
commit-hash: 24d2ac0b56fcbde13d827745f66e73efb1e17156
commit-date: 2024-07-15
host: aarch64-apple-darwin
release: 1.81.0-nightly
LLVM version: 18.1.7

Additional Labels

No response

@powergee powergee added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jul 16, 2024
@tesuji
Copy link
Contributor

tesuji commented Jul 17, 2024

I think clippy is right about this warning. let data = [const_init(); 0]; doesn't do what you expect it to do.
It creates a zero-size array (data) that has no elements inside, including value of const_init().
So const_init() is always run (we haven't talked about it being a const fn/side-effect free) but the data is empty.
@rustbot label -I-false-positive

@rustbot rustbot removed the I-false-positive Issue: The lint was triggered on code it shouldn't have label Jul 17, 2024
@powergee
Copy link
Author

Thank you for your reply.

I agree that the provided code snippet doesn't do anything meaningful, as it just creates a zero-sized array with a redundant initializer call. If a developer can easily rewrite the code, fixing it would be straightforward.

However, my initial concern was that generating a warning about a side effect itself doesn't seem reasonable because it is a compile-time constant expression, which clearly cannot cause a side effect. Additionally, I thought that it might create unnecessary Clippy CI failures on automatically generated code (e.g., macro expansion).

For example, I encountered this issue while working on a project with a macro functionality. To explain it briefly, there was a macro (select!) that might create an array (code). Sometimes, the internal array can be zero-sized due to user inputs, but it was okay because the initializer does not cause a side-effect (it can be a const fn). Clippy was complaining about it, and marking the internal initializer call as constant did not work. Finally, I had no better option than manually ignoring the lint for that line.

In summary, I think this lint might reject many old use cases of zero-sized arrays with constant initializers, and it would be great if we could allow cases where the initializer is a constant expression.

@y21
Copy link
Member

y21 commented Jul 17, 2024

const fn can still panic which could arguably be seen as a side effect (e.g. it might assert that some condition passed in as an argument is true), so not sure if I agree that we should exclude all const fn.
(They also don't necessarily have to execute at compile time unless you explicitly write [const { const_init() }; 0], which does avoid the warning here)

I do agree though that it probably still shouldn't lint in your linked PR.
We could restrict the lint to cases where the repeat count is a literal 0 expression (to avoid paths to constants like in the linked code, which can happen to have the value zero, but the developer most definitely knows and expects that the initializer function will be called), and also avoid emitting a warning if it's from an external macro (or if the length expression is from a different syntax context)

@tesuji
Copy link
Contributor

tesuji commented Jul 17, 2024

Looking at the code you linked, I think we could silent this lint when the array length initializer are from macro expansion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants