Skip to content

[unnecessary_lazy_eval]: don't lint on types with deref impl #10864

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

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

y21
Copy link
Member

@y21 y21 commented Jun 1, 2023

Fixes #10437.
This PR changes clippy's util module eager_or_lazy to also consider deref expressions whose type has a non-builtin deref impl and not suggest replacing it as that might have observable side effects.
A prominent example might be the lazy_static macro, which creates a newtype with a Deref impl that you need to go through to get access to the inner value. Going from lazy to eager can make a difference there.

changelog: [unnecessary_lazy_eval]: don't lint on types with non-builtin deref impl

@rustbot
Copy link
Collaborator

rustbot commented Jun 1, 2023

r? @giraffate

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 1, 2023
@xFrednet
Copy link
Member

xFrednet commented Jun 3, 2023

Assuming it's fine if I steal the PR review :)

@blyxyas Would you mind reviewing this PR?

r? @xFrednet

@rustbot rustbot assigned xFrednet and unassigned giraffate Jun 3, 2023
@blyxyas
Copy link
Member

blyxyas commented Jun 3, 2023

Yep, Gonna review it right now ^^

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

@y21
Copy link
Member Author

y21 commented Jun 5, 2023

thanks for the review! not sure if @blyxyas has r+ rights, so maybe @xFrednet you need to if the changes look good? :)

@xFrednet
Copy link
Member

xFrednet commented Jun 5, 2023

Yes, sorry for the delay. This looks good to me as well! Thank you!

@bors r=blyxyas,xFrednet

@bors
Copy link
Contributor

bors commented Jun 5, 2023

📌 Commit a239c8b has been approved by blyxyas,xFrednet

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 5, 2023

⌛ Testing commit a239c8b with merge b033883...

@bors
Copy link
Contributor

bors commented Jun 5, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas,xFrednet
Pushing b033883 to master...

@bors bors merged commit b033883 into rust-lang:master Jun 5, 2023
bors added a commit that referenced this pull request Jun 7, 2023
consider autoderef through user-defined `Deref` in `eager_or_lazy`

Fixes #10462

This PR handles autoderef in the `eager_or_lazy` util module and stops suggesting to change lazy to eager if autoderef in an expression goes through user defined `Deref` impls, e.g.
```rs
struct S;
impl Deref for S {
  type Target = ();
  fn deref(&self) -> &Self::Target { &() }
}

let _ = Some(()).as_ref().unwrap_or_else(|| &S); // autoderef `&S` -> `&()`
```

changelog: [`unnecessary_lazy_evaluations`]: don't suggest changing lazy evaluation to eager if autoderef goes through user-defined `Deref`

r? `@xFrednet`  (because of the earlier review in #10864, might help for context here)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unnecessary_lazy_evaluations and Deref
6 participants