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

libsyntax/parse: fix missing kind error reporting #41282

Merged
merged 1 commit into from
Apr 18, 2017

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Apr 13, 2017

Fixes #41161.
Fixes #41239.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@arielb1
Copy link
Contributor Author

arielb1 commented Apr 13, 2017

r? @petrochenkov - how do I add error recovery?
cc @jonathandturner for help with the error message
cc @estebank

.struct_span_err(sp, "missing `fn` for method declaration");
err.span_label(sp, &"missing `fn`");
}
if !self.eat(&token::Not) {
Copy link
Contributor

@petrochenkov petrochenkov Apr 13, 2017

Choose a reason for hiding this comment

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

I'd also add path.segments.len() == 1 to the condition for mistyped impl item declaration.

As for recovery, if the current token is ( then err can be emitted immediately as non-fatal (err.emit()) and we can parse remaining tokens as a method.
If the current token is : then remaining tokens can be parsed as an associated type (statistically more likely), but there's a chance for confusing error if it's actually an associated const, and this probably doesn't matter as much as methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What to do if path.segments.len() != 1?

Copy link
Contributor

Choose a reason for hiding this comment

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

What to do if path.segments.len() != 1?

Report missing ! and continue parsing as a macro, most likely.

@@ -0,0 +1,10 @@
error: missing `fn`, `type`, or `const` for impl-item declaration
Copy link
Contributor

Choose a reason for hiding this comment

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

This file shouldn't be in the PR.

@shepmaster shepmaster added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 14, 2017
@arielb1 arielb1 force-pushed the missing-impl-item branch from ddcadbe to d648c10 Compare April 17, 2017 18:26
@arielb1
Copy link
Contributor Author

arielb1 commented Apr 17, 2017

Now with error recovery.

@arielb1 arielb1 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 17, 2017
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 17, 2017

📌 Commit d648c10 has been approved by petrochenkov

@bors
Copy link
Contributor

bors commented Apr 17, 2017

⌛ Testing commit d648c10 with merge 235fe83...

bors added a commit that referenced this pull request Apr 17, 2017
libsyntax/parse: fix missing kind error reporting

Fixes #41161.
Fixes #41239.
@bors
Copy link
Contributor

bors commented Apr 18, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing 235fe83 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

7 participants