fix Expr::can_have_side_effects for [x; N] style array literal and binary expressions

[andjsrk]

AFAIK [0; 3] is basically a syntax sugar for [0, 0, 0] so it should return whether the repeat's element can have side effects, like what it does on arrays.
And it seems that the rule for unary operators and indexings can be applied to binary operators as well.

[rustbot]

r? @jdonszelmann

rustbot has assigned @jdonszelmann.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Kivooeo]

Would you like to add a test for this, to reflect new behaviour you changed here

Because on a first glance it's not obvious (at least to me)

[andjsrk]

I'm not sure where should I add a test:

• tests/ui/expr.rs - Expr::can_have_side_effects is used in a few minor(in my opinion) places and is used widely in Clippy rather than rustc. If I decide to add a test in there, I guess testing code would look pretty unintuitive (could be improved by comments though).
• compiler/rustc_hir/src/hir/tests.rs - maybe I should add tests here, but I guess runtime test is needed (not just checking successful compilation) and I have no idea about how to create an Expr...

[jdonszelmann]

I agree with @Kivooeo here: something has to be tested. A ui test will show a piece of rust code now compiling (or warning) differently. I think this is supposed to change something about the diagnostics of binop expressions and array expressions. So ideally, I'd see a uitest showing a diagnostic that is different. In fact, I see some tests that failed in CI, likely because of this diagnostics change. If you run x test tests/ui/<specific-test> --bless it will update the expected output, which you can commit, to show what diagnostics changed from this change. Maybe you want to also add a specific test for the exact changed diagnostics behavior after this PR. Let me know if that makes sense!

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[andjsrk]

Maybe you want to also add a specific test for the exact changed diagnostics behavior after this PR.

@jdonszelmann I'm going to add a test for repeats([x; N]) as it seems no test checks Expr::can_have_side_effects on repeats, but as I asked above I have no idea about where should I add a test, could you let me know?

[Kivooeo]

Add please this test to tests/ui/repeat-expr/

And I'm not sure why this changes to clippy were made?

[andjsrk]

• clippy::needless_match relies on Expr::can_have_side_effects and it warns some code in the UI test for clippy::manual_clamp.
• clippy::needless_bitwise_bool has test cases that require improvement of Expr::can_have_side_effects and some of them are resolved now.

These are the cause of test failures in CI and so I fixed them, did I something wrong?

[andjsrk]

@rustbot ready

[samueltardieu]

The Clippy part looks ok.

[jdonszelmann]

yea, I think this looks reasonable. @bors r=jdonszelmann,samueltardieu rollup

[rust-bors]

📌 Commit d848437 has been approved by jdonszelmann,samueltardieu

It is now in the queue for this repository.

[jdonszelmann]

actually, stop, @bors r-

[rust-bors]

Commit d848437 has been unapproved.

[jdonszelmann]

I realised that I would also quite like to see a test for the binary operator change here. After that r=me

[jdonszelmann]

@rustbot author

[andjsrk]

@bors r=jdonszelmann

[rust-bors]

@andjsrk: 🔑 Insufficient privileges: not in review users

[andjsrk]

@rustbot ready

[andjsrk]

@jdonszelmann I have no permission, please take action on this