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.

Sign up for GitHub

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

Jump to bottom

Tweak output for invalid negative impl errors #69649

Merged
merged 1 commit into from
Mar 23, 2020
Merged

Tweak output for invalid negative impl errors #69649

merged 1 commit into from
Mar 23, 2020
+95 −50

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Mar 2, 2020 •
edited
Loading

Follow up to #69722. Tweak negative impl errors emitted in the HIR:

error[E0192]: invalid negative impl
  --> $DIR/E0192.rs:9:6
   |
LL | impl !Trait for Foo { }
   |      ^^^^^^
   |
   = note: negative impls are only allowed for auto traits, like `Send` and `Sync`

@rust-highfive
Copy link
Collaborator

r? @ecstatic-morse

(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 Mar 2, 2020
@rust-highfive

This comment has been minimized.

Sign in to view
@Centril
Copy link
Contributor

Centril commented Mar 3, 2020 •
edited
Loading

r? @Centril

High level reviewing notes before doing a deeper review:

  • Let's split this PR into two parts, one for AST and one for HIR -- I would like to review the HIR parts in isolation as well as making sure that they don't regress perf.
  • Please avoid changing random_field: _s in favor of .. when pattern matching records, especially when it comes to visitors. The use of random_field: _ is typically intentional to make sure that we are forced to handle new cases should we add more fields.
  • Please also add a PR description.

@estebank
Copy link
Contributor Author

estebank commented Mar 5, 2020

AST part in #69722

@Centril Centril added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 8, 2020
@bors

This comment has been minimized.

Sign in to view
@Dylan-DPC-zz

This comment has been minimized.

Sign in to view
@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Mar 21, 2020
@estebank
Copy link
Contributor Author

@Centril rebased on top of master and this PR only has the HIR changes.

@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 Mar 23, 2020
Centril
Centril reviewed Mar 23, 2020
View reviewed changes
Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

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

r=me with these comments addressed and clean perf.

src/librustc_typeck/check/wfcheck.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/wfcheck.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Mar 23, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Mar 23, 2020

⌛ Trying commit 19d0186f3cead8ed2cc3741f6c2e2fbb180e81d2 with merge 12e42227516ca02418f6dbea278da1e4c0dd14b3...

@rust-highfive

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Mar 23, 2020

💔 Test failed - checks-azure

@bors bors 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 Mar 23, 2020
@estebank
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Mar 23, 2020

⌛ Trying commit b8f9866 with merge a7111c6f0c00e55b69e1eb62c1f04775705b4e0e...

@bors
Copy link
Contributor

bors commented Mar 23, 2020

☀️ Try build successful - checks-azure
Build commit: a7111c6f0c00e55b69e1eb62c1f04775705b4e0e (a7111c6f0c00e55b69e1eb62c1f04775705b4e0e)

@rust-timer
Copy link
Collaborator

Queued a7111c6f0c00e55b69e1eb62c1f04775705b4e0e with parent e4b01c7, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit a7111c6f0c00e55b69e1eb62c1f04775705b4e0e, comparison URL.

@Centril
Copy link
Contributor

Centril commented Mar 23, 2020

Seems like noise, @bors r+

@bors
Copy link
Contributor

bors commented Mar 23, 2020

📌 Commit b8f9866 has been approved by Centril

@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 Mar 23, 2020
@bors
Copy link
Contributor

bors commented Mar 23, 2020

⌛ Testing commit b8f9866 with merge 8549cfe...

@bors
Copy link
Contributor

bors commented Mar 23, 2020

☀️ Test successful - checks-azure
Approved by: Centril
Pushing 8549cfe to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 23, 2020
@bors bors merged commit 8549cfe into rust-lang:master Mar 23, 2020
@bors bors mentioned this pull request Mar 23, 2020
Merged
7 tasks
@estebank estebank deleted the negative-impl-span branch November 9, 2023 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril left review comments

Assignees

@Centril Centril

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@estebank @rust-highfive @Centril @bors @Dylan-DPC-zz @rust-timer @ecstatic-morse