Skip to content

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

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

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.

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

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.

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.

Comment on lines 999 to 1028
if let ty::PredicateKind::Trait(trait_pred, _) = pred.kind().skip_binder() {
let trait_ref = trait_pred.trait_ref;
let self_ty = trait_ref.self_ty();
if let ty::Adt(adt_def, _) = self_ty.kind() {
if adt_def.did.is_local() {
let diagnostic_items =
self.tcx.diagnostic_items(trait_ref.def_id.krate);
return derivables.iter().find_map(|trait_derivable| {
if let Some(item_def_id) = diagnostic_items.get(trait_derivable) {
if item_def_id == &trait_ref.def_id
&& !(adt_def.is_enum() && *trait_derivable == sym::Default)
{
return Some((
format!("{}", self_ty),
self.tcx.def_span(adt_def.did),
format!("{}", trait_ref.print_only_trait_path()),
));
}
}
None
});
}
}
}
None
Copy link
Contributor

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

Choose a reason for hiding this comment

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

Like this?

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
@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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

Suggest missing Debug Impl when unwrapping
10 participants