2

consider autoderef through user-defined `Deref` in `eager_or_lazy` by y21 · Pull...

 1 year ago
source link: https://github.com/rust-lang/rust-clippy/pull/10896
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Conversation

Contributor

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.

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)

xFrednet reacted with heart emoji

rustbot

added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label

Jun 5, 2023

Member

Thank you for the PR and review request :)

@blyxyas Do you first want to take a look at this PR? You also reviewed the previous one

Contributor

Yep, will take it! cat

xFrednet and y21 reacted with heart emoji

Contributor

@blyxyas blyxyas

left a comment

I don't think there's anything wrong with this. Let's hope it doesn't make a big toll on performance (By my benchmarks, it doesn't)! Thanks heart!

(Forgot to ping @xFrednet on the other reviews, sorry)

y21 reacted with thumbs up emoji

Member

Looks good to me as well! Thank you for the update @y21!


Let's hope it doesn't make a big toll on performance (By my benchmarks, it doesn't)!

This change looks pretty simple, I would be surprised if it was even noticeable. Especially, since the condition is nested below other ones. But At the same time, it's good to check.

(Forgot to ping @xFrednet on the other reviews, sorry)

Usually, I still look at the ongoing conversation, in the other PR I just missed it. You're always welcome to ping me, though. Thank you for the review!


Now without further of due: Lord bors, would you mind merging this PR in the name of princess blyxyas and me?

flip1995 reacted with laugh emojiy21 and blyxyas reacted with heart emoji

Contributor

pushpin Commit e70dd55 has been approved by blyxyas,xFrednet

It is now in the queue for this repository.

Contributor

hourglass Testing commit e70dd55 with merge 7c34ec8...

Contributor

bors

merged commit 7c34ec8 into

rust-lang:master

Jun 7, 2023

5 checks passed

Contributor

princess blyxyas

I'm genuine dying right now
Dies

xFrednet reacted with laugh emojixFrednet reacted with heart emoji

Member

Now without further of due: Lord bors, would you mind merging this PR in the name of princess blyxyas and me?

How did you do it this time? How did you trigger bors, you magician? smile

Member

princess blyxyas

I'm genuine dying right now
Dies

Please don't I haven't learned any spells for resurrection. This kingdom still needs you


How did you do it this time?

Should a wizard reveal their power?

(It's the same magic trick I used a few months ago. GitHub still processes pings in markdown doc comments <!-- -->)

y21, flip1995, and blyxyas reacted with eyes emoji

Member

Ah, I already forgot about this trick :D

Member

Maybe I can use it again in a few months, to confuse you. Using it is always a lot of fun, and that's one of the reasons we're here, right? ^^

flip1995 reacted with laugh emojiflip1995 and blyxyas reacted with heart emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

xFrednet

xFrednet approved these changes

blyxyas

blyxyas approved these changes
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

unnecessary_lazy_evaluations and once_cell::sync::Lazy

6 participants

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK