unnecessary_lazy_evaluations suggests running `unsafe` code eagerly

[kornelski]

Summary

For unsafe {} blocks it matters when they're executed, even if they're cheap.

Lint Name

unnecessary_lazy_evaluations

Reproducer

I tried this code:

is_safe_to_access.then_some(|| unsafe { &a_union.field });

Clippy suggests:

is_safe_to_access.then(unsafe { &a_union.field });

which runs unsafe code unconditionally, bypassing the safety check.

Version

rustc 1.81.0-nightly (bcf94dec5 2024-06-23)
binary: rustc
commit-hash: bcf94dec5ba6838e435902120c0384c360126a26
commit-date: 2024-06-23
host: aarch64-apple-darwin
release: 1.81.0-nightly
LLVM version: 18.1.7

Additional Labels

No response

[Jarcho]

Note for anyone taking this: Do not check for the presence of an unsafe block. This particular issue is about union field access.

[kornelski]

I see Clippy tries to find unsafe side effects, but I'm uncomfortable with this idea. I wouldn't want b.then(|| unsafe { }) to be changed to b.then_some(unsafe { }) under any circumstances. Even if the unsafe {} block has no side effects currently, that could change in the future. The b.then_some(unsafe { }) construct is needlessly high risk.

[Jarcho]

Looking for an unsafe block in the expression doesn't fix the problem. The unsafe block could come from a parent block/fn. e.g.

unsafe { is_safe_to_access.then_some(|| &a_union.field ); }

[rzvxa]

I'd like to claim this one, I'm not sure if there is a way to check if union fields are accessed through the type system or if I should walk all fields and look for union access.

[Jarcho]

When walking the expression use the adjusted type of the base expression.

[rzvxa]

When walking the expression use the adjusted type of the base expression.

I'm implementing this check in the expr_eagerness function. Is that the right place for it? Or should it be local to this rule?

Here's what I have in mind, For any ExprKind::Field(a, b) I use expr_ty on the a expression and I have to find a way to make sure the returned type isn't a union otherwise I should prevent eager evaluations.
I'd appreciate it if you point me in the right direction, I'm not sure how I should approach it.

Note to anyone interested in fixing this issue: I'm not claiming this since I'm familiarizing myself with the codebase and I might be slow to submit a PR, If you need this fixed faster please feel free to claim before I do. I'll self-assign it only once I'm confident that I can implement it.