-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Librustc_resolve diagnostics fixes #33493
Conversation
} | ||
``` | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra empty line.
cc #32777 |
let x = (0,2); | ||
match x { | ||
(0, ref y) | (y, 0) => { /* use y */} // error: variable `y` is bound with | ||
// different mode in pattern #2 than |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't be aligned with "error:". If it is too long, write it to next line.
☔ The latest upstream changes (presumably #33494) made this pull request unmergeable. Please resolve the merge conflicts. |
addressed |
Example of erroneous code: | ||
|
||
```compile_fail | ||
let x = (0,2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You put a whitespace after the comma everywhere else. I think to be consistent you should put it here as well.
Thanks! r=me |
@bors r=GuillaumeGomez |
📌 Commit a795419 has been approved by |
…omez Librustc_resolve diagnostics fixes r? @GuillaumeGomez
``` | ||
let x = (0, 2); | ||
match x { | ||
(0, ref y) | (ref y, 0) => { /* use y */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http://buildbot.rust-lang.org/builders/auto-linux-64-opt-rustbuild/builds/1038/steps/test/logs/stdio <- it looks like this fails the rollup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an ICE. I already filed it. We need to add "ignore" on this one or to find a workaround...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. r?
(I didn't mention a bug number here since this is supposed to be read by the user. I've left a note on the ICE bug.)
@bors: r- |
Seems fixed. cc @steveklabnik |
@bors r+ p=20 |
📌 Commit 84843b7 has been approved by |
@bors rollup p=0 oops |
r? @GuillaumeGomez