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

parser: show a helpful note on unexpected inner comment #33333

Merged
merged 1 commit into from
May 7, 2016

Conversation

birkenfeld
Copy link
Contributor

Fixes: #30318.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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


//! Misplaced comment...
//~^ ERROR expected outer comment
//~| inner comments like this must be placed before any items
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this say NOTE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess :) will fix with the new message.

Copy link
Member

Choose a reason for hiding this comment

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

(btw, ping me when you need re-review, I'm not actively watching the PRs)

@Manishearth
Copy link
Member

@wafflespeanut had attempted a more comprehensive diagnostics overhaul of the parser when it comes to unexpected comments-that-aren't, but it didn't work out so well IIRC.

@birkenfeld
Copy link
Contributor Author

r? @Manishearth

let mut err = self.fatal("expected outer doc comment");
err.fileline_note(self.span, "inner doc comments like this \
(starting with `//!` or `/*!`) \
must be placed before any items");
Copy link
Member

Choose a reason for hiding this comment

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

s/must be placed before any items/can only appear before items/

@Manishearth
Copy link
Member

@bors delegate+ with minor nit

@bors
Copy link
Contributor

bors commented May 2, 2016

✌️ @birkenfeld can now approve this pull request

@birkenfeld
Copy link
Contributor Author

@bors r=Manishearth

@bors
Copy link
Contributor

bors commented May 3, 2016

📌 Commit b446c99 has been approved by Manishearth

@bors
Copy link
Contributor

bors commented May 3, 2016

⌛ Testing commit b446c99 with merge 9cce34c...

@bors
Copy link
Contributor

bors commented May 3, 2016

💔 Test failed - auto-linux-64-cross-armhf

@@ -35,7 +35,11 @@ impl<'a> Parser<'a> {
self.span.hi
);
if attr.node.style != ast::AttrStyle::Outer {
return Err(self.fatal("expected outer comment"));
let mut err = self.fatal("expected outer doc comment");
err.fileline_note(self.span, "inner doc comments like this \
Copy link
Member

Choose a reason for hiding this comment

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

won't work anymore

@birkenfeld
Copy link
Contributor Author

@bors r=Manishearth 72560e1

bors added a commit that referenced this pull request May 3, 2016
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 3, 2016

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented May 3, 2016

📌 Commit 72560e1 has been approved by Manishearth

bors added a commit that referenced this pull request May 3, 2016
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
bors added a commit that referenced this pull request May 4, 2016
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
bors added a commit that referenced this pull request May 4, 2016
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
@bors
Copy link
Contributor

bors commented May 7, 2016

⌛ Testing commit 72560e1 with merge 0d61bb3...

bors added a commit that referenced this pull request May 7, 2016
parser: show a helpful note on unexpected inner comment

Fixes: #30318.
@bors bors merged commit 72560e1 into rust-lang:master May 7, 2016
@birkenfeld birkenfeld deleted the issue-30318 branch May 7, 2016 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants