Skip to content
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

Consider unfulfilled obligations in binop errors #89323

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Sep 28, 2021

When encountering a binop where the types would have been accepted, if
all the predicates had been fulfilled, include information about the
predicates and suggest appropriate #[derive]s if possible.

Fix #84515.

@rust-highfive
Copy link
Collaborator

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 Sep 28, 2021
@petrochenkov
Copy link
Contributor

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 29, 2021
@bors
Copy link
Contributor

bors commented Sep 29, 2021

⌛ Trying commit a44c45ad10fb8966b70277036614fc7816f9cba0 with merge 5971dfc06e85c9854f851f945da5040e0b31ba48...

@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2021
@bors
Copy link
Contributor

bors commented Sep 29, 2021

☀️ Try build successful - checks-actions
Build commit: 5971dfc06e85c9854f851f945da5040e0b31ba48 (5971dfc06e85c9854f851f945da5040e0b31ba48)

@rust-timer
Copy link
Collaborator

Queued 5971dfc06e85c9854f851f945da5040e0b31ba48 with parent 50f9f78, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5971dfc06e85c9854f851f945da5040e0b31ba48): comparison url.

Summary: This change led to moderate relevant regressions 😿 in compiler performance.

  • Moderate regression in instruction counts (up to 0.6% on incr-full builds of inflate)

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

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

@rustbot rustbot added perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Sep 29, 2021
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 1, 2021
@estebank
Copy link
Contributor Author

estebank commented Oct 1, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 1, 2021
@bors
Copy link
Contributor

bors commented Oct 1, 2021

⌛ Trying commit 4d55ff29e5b641a20091ca8888ab129c192e6574 with merge 0cf952227f02aa71d77879ef66bbb3d1a477c9c2...

@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 1, 2021
@estebank
Copy link
Contributor Author

estebank commented Oct 1, 2021

@petrochenkov, sorry I hadn't realized we changed the waiting-on labels back and forth. Why should this be waiting on author? It addresses your review comments and ran a perf to verify that it has no impact (I don't see why it would).

@estebank estebank added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 1, 2021
@petrochenkov
Copy link
Contributor

I set the label after writing #89323 (comment) and before the last commit appeared, now it's waiting on perf, I guess.

@rust-timer
Copy link
Collaborator

Queued dac9ca84fd7135880faa8885c21db395c7841a6e with parent c02371c, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (dac9ca84fd7135880faa8885c21db395c7841a6e): 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.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Oct 2, 2021
@estebank estebank force-pushed the derive-binop branch 2 times, most recently from b1b3812 to 090cbb0 Compare October 4, 2021 16:48
compiler/rustc_typeck/src/check/method/suggest.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/check/method/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/check/method/mod.rs Outdated Show resolved Hide resolved
src/test/ui/issues/issue-31076.stderr Outdated Show resolved Hide resolved
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 4, 2021
@bors

This comment has been minimized.

@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 5, 2021
@petrochenkov
Copy link
Contributor

r=me with commits squashed.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 5, 2021
When encountering a binop where the types would have been accepted, if
all the predicates had been fulfilled, include information about the
predicates and suggest appropriate `#[derive]`s if possible.

Point at trait(s) that needs to be `impl`emented.
@estebank
Copy link
Contributor Author

estebank commented Oct 5, 2021

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Oct 5, 2021

📌 Commit e8fc076 has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 5, 2021
@bors
Copy link
Contributor

bors commented Oct 6, 2021

⌛ Testing commit e8fc076 with merge d7539a6...

@bors
Copy link
Contributor

bors commented Oct 6, 2021

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing d7539a6 to master...

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

Finished benchmarking commit (d7539a6): 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

@estebank estebank deleted the derive-binop branch November 9, 2023 05:14
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest implementing PartialEq for Foo when comparing Option<Foo> with Option<Foo>
6 participants