Skip to content

Suggest deriving traits if possible#86943

Merged
bors merged 1 commit intorust-lang:masterfrom
ptrojahn:suggest_derive
Sep 8, 2021
Merged

Suggest deriving traits if possible#86943
bors merged 1 commit intorust-lang:masterfrom
ptrojahn:suggest_derive

Conversation

@ptrojahn
Copy link
Contributor

@ptrojahn ptrojahn commented Jul 7, 2021

This only applies to builtin derives as I don't think there is a
clean way to get the available derives in typeck.

Closes #85851

@rust-highfive
Copy link
Contributor

rust-highfive commented Jul 7, 2021

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2021
@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 7, 2021

r? @estebank for review or re-assignment

@bors
Copy link
Collaborator

bors commented Jul 9, 2021

☔ The latest upstream changes (presumably #85263) made this pull request unmergeable. Please resolve the merge conflicts.

estebank
estebank reviewed Jul 18, 2021
View reviewed changes
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, I had the review in pending and never "finished" the review.

I have some nitpicks. Do you think you could deal with them?

@ptrojahn
Copy link
Contributor Author

ptrojahn commented Jul 27, 2021

Thanks for the great suggestions!

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 15, 2021
@inquisitivecrystal inquisitivecrystal added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 24, 2021
@ptrojahn ptrojahn requested a review from estebank September 1, 2021 16:17
@estebank
Copy link
Contributor

estebank commented Sep 2, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 2, 2021

📌 Commit d552e65e2a4e065cf98ac4703c3833f90ab5d6b3 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 2, 2021
@bors
Copy link
Collaborator

bors commented Sep 2, 2021

⌛ Testing commit d552e65e2a4e065cf98ac4703c3833f90ab5d6b3 with merge b3d10b04488f013cd3ea7d01a6a3de215883a68e...

@bors
Copy link
Collaborator

bors commented Sep 2, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 2, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
estebank
estebank reviewed Sep 2, 2021
View reviewed changes
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

You will need to rebase on top of a recent master. I also left a nitpick.

compiler/rustc_typeck/src/check/method/suggest.rs Outdated
Comment on lines 999 to 1028
Copy link
Contributor

@estebank estebank Sep 2, 2021

Choose a reason for hiding this comment

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

This has quite a bit of rightwards drift. Could we instead use let x = if let ... else { return; } to early return the closure and make the contents look more linear?

Copy link
Contributor Author

@ptrojahn ptrojahn Sep 6, 2021

Choose a reason for hiding this comment

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

Like this?

@ptrojahn
Suggest deriving traits if possible
50e5f90 
This only applies to builtin derives as I don't think there is a
clean way to get the available derives in typeck.

Closes rust-lang#85851
@ptrojahn ptrojahn requested a review from estebank September 6, 2021 11:30
@estebank
Copy link
Contributor

estebank commented Sep 6, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 6, 2021

📌 Commit 50e5f90 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2021
@bors
Copy link
Collaborator

bors commented Sep 8, 2021

⌛ Testing commit 50e5f90 with merge 434cb43...

@bors
Copy link
Collaborator

bors commented Sep 8, 2021

☀️ Test successful - checks-actions
Approved by: estebank
Pushing 434cb43 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 8, 2021
@bors bors merged commit 434cb43 into rust-lang:master Sep 8, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 8, 2021
@rust-timer
Copy link
Collaborator

rust-timer commented Sep 8, 2021

Finished benchmarking commit (434cb43): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

This was referenced Oct 23, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estebank estebank Awaiting requested review from estebank

Assignees

@estebank estebank

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.57.0

Development

Successfully merging this pull request may close these issues.

Suggest missing Debug Impl when unwrapping

10 participants

@ptrojahn @rust-highfive @petrochenkov @bors @estebank @rust-log-analyzer @rust-timer @JohnCSimon @inquisitivecrystal @rustbot