-
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
suggest !
for erroneous identifier not
#49258
Conversation
src/libsyntax/parse/parser.rs
Outdated
let mut err = self.diagnostic() | ||
.struct_span_err(self.span, | ||
&format!("unexpected {} after identifier", | ||
found.to_string())); |
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 can use format!("unexpected {} after identifier", self.this_token_descr())
here.
src/libsyntax/parse/parser.rs
Outdated
// but as `rustc`-the-compiler, we can issue clever diagnostics | ||
// for confused users who really want to say `!` | ||
let token_cannot_continue_expr = |t: &token::Token| match *t { | ||
// These tokens can't continue an expression after an ident |
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.
Comment nit: Tokens that can start an expression after !
, but can't continue an expression after an ident
src/libsyntax/parse/parser.rs
Outdated
// for confused users who really want to say `!` | ||
let token_cannot_continue_expr = |t: &token::Token| match *t { | ||
// These tokens can't continue an expression after an ident | ||
token::Ident(..) | token::Literal(..) | token::Pound => true, |
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.
Also, it should be easier now to add the Nt
variants from #48858 (comment)
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.
Also, for token::Ident
the correct condition is t is Token(ident) && ident_can_begin_expr(ident)
Meta: opening a new PR moves it into the end of @bors queue, it's usually better to rework an original PR, then it will land sooner. |
Ping from triage, @zackmdavis ! It's been a week since we heard from you; will you be able to address the review feedback you've received? |
@shepmaster yes, but next week; very busy lately |
be654c3
to
cea7c69
Compare
updated to address feedback, except that I don't know how to write a test example that exercises the nonterminals (and it's more efficient to push this and admit that I don't know if I can't spare the effort to figure it out myself now)
I usually add commits or force-push existing PRs, but in this case, I don't think there are any lines of code in common between the original submission and the post-reviewer-feedback version; semantically, it feels like a "different PR addressing the same issue" rather than "a revision of the same PR". For something nonessential like this, I'm not that interested in jostling for position in the buildbot queue: that's a zero-sum game among outstanding PRs! |
Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
r=me after making Travis green (updating UI tests) |
Thanks to the inestimably inimitable Esteban "Estebank" Küber for pointing this out. This is relevant to rust-lang#46836.
Impressing confused Python users with magical diagnostics is perhaps worth this not-grossly-unreasonable (only 40ish lines) extra complexity in the parser? Thanks to Vadim Petrochenkov for guidance. This resolves rust-lang#46836.
cea7c69
to
ba0dd8e
Compare
(I wish the check for |
@bors r+ |
📌 Commit ba0dd8e has been approved by |
⌛ Testing commit ba0dd8e with merge 6f0cad9687f69e5dbb3a643515a419997bc50912... |
💔 Test failed - status-appveyor |
@bors retry |
suggest `!` for erroneous identifier `not` ![not_recovery](https://user-images.githubusercontent.com/1076988/37753255-3b669c42-2d59-11e8-9071-efad8eaf3086.png) This supersedes #48858. r? @petrochenkov
☀️ Test successful - status-appveyor, status-travis |
This supersedes #48858.
r? @petrochenkov