Optimize the diagnostic generation for extern unsafe
#97172
Conversation
rustbot
added
the
T-compiler
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| let current_qual_sp = current_qual_sp.to(sp_start); | ||
| if let Ok(current_qual) = self.span_to_snippet(current_qual_sp) { | ||
| let invalid_qual_sp = self.token.uninterpolated_span(); | ||
| let invalid_qual = self.span_to_snippet(invalid_qual_sp).unwrap(); |
There was a problem hiding this comment.
Can you get rid of this unwrap? Maybe if let Ok((current_qual, invalid_qual)) = ..?
There was a problem hiding this comment.
Sure, I think we can use a let...chain here
| // If the next token is `{`, we can continue parsing items. | ||
| if self.look_ahead(1, |t| t.kind == token::OpenDelim(Delimiter::Brace)) { | ||
| err.emit(); | ||
| self.eat_keyword(kw::Unsafe); |
There was a problem hiding this comment.
do we want to modify the unsafety variable since we parsed this keyword?
| &format!("`{}` must come before `{}`", invalid_qual, current_qual), | ||
| format!("{} {}", invalid_qual, current_qual), | ||
| Applicability::MachineApplicable, | ||
| ).note("keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`"); |
There was a problem hiding this comment.
"keyword order for functions declaration"
This isn't a function declaration, though, right?
There was a problem hiding this comment.
That makes sense. And I think we can omit this note, since we just want to parse a foreign mod here.
|
Thanks for reviewing! I made some correcting here |
| Ok(items) => { | ||
| let module = ast::ForeignMod { unsafety, abi, items }; | ||
| Ok((Ident::empty(), ItemKind::ForeignMod(module))) | ||
| if self.token.is_keyword(kw::Unsafe) { |
There was a problem hiding this comment.
Actually, now that I think of it, I'm not satisfied with the fact that this if block is larger than the rest of the function.. Do we expect extern "C" unsafe to show up much in practice?
|
Thanks @SparrowLii, this is much cleaner! @bors r+ rollup |
|
📌 Commit 0be2ca9 has been approved by |
bors
added
S-waiting-on-bors
…r-errors
Optimize the diagnostic generation for `extern unsafe`
This PR does the following about diagnostic generation when parsing foreign mod:
1. Fixes the FIXME about avoiding depending on the error message text.
2. Continue parsing when `unsafe` is followed by `{` (just like `unsafe extern {...}`).
3. Add test case.
…piler-errors Rollup of 6 pull requests Successful merges: - rust-lang#89685 (refactor: VecDeques Iter fields to private) - rust-lang#97172 (Optimize the diagnostic generation for `extern unsafe`) - rust-lang#97395 (Miri call ABI check: ensure type size+align stay the same) - rust-lang#97431 (don't do `Sized` and other return type checks on RPIT's real type) - rust-lang#97555 (Source code page: line number click adds `NaN`) - rust-lang#97558 (Fix typos in comment) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This PR does the following about diagnostic generation when parsing foreign mod:
unsafeis followed by{(just likeunsafe extern {...}).