-
-
Notifications
You must be signed in to change notification settings - Fork 15.7k
Parser: Uninterpolate when checking for const closures, try bikeshed blocks & in relevant diagnostic code #162704
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -337,66 +337,46 @@ impl<'a> Parser<'a> { | |
| HelpIdentifierStartsWithNumber { num_span: invalid } | ||
| }); | ||
|
|
||
| let err = ExpectedIdentifier { | ||
| let mut err = self.dcx().create_err(ExpectedIdentifier { | ||
| span: bad_token.span, | ||
| token: bad_token, | ||
| suggest_raw, | ||
| suggest_remove_comma, | ||
| help_cannot_start_number, | ||
| }; | ||
| let mut err = self.dcx().create_err(err); | ||
| }); | ||
|
|
||
| // if the token we have is a `<` | ||
| // it *might* be a misplaced generic | ||
| // FIXME: could we recover with this? | ||
| // If the token we have is a `<` it *might* be a misplaced generic | ||
| // parameter list as in `fn <T>id(x: T) -> T { x }`. | ||
| // FIXME: Could we recover with this? | ||
| if self.token == token::Lt { | ||
| // all keywords that could have generic applied | ||
| let valid_prev_keywords = | ||
| [kw::Fn, kw::Type, kw::Struct, kw::Enum, kw::Union, kw::Trait]; | ||
|
|
||
| // If we've expected an identifier, | ||
| // and the current token is a '<' | ||
| // if the previous token is a valid keyword | ||
| // that might use a generic, then suggest a correct | ||
| // generic placement (later on) | ||
| let maybe_keyword = self.prev_token; | ||
| if valid_prev_keywords.into_iter().any(|x| maybe_keyword.is_keyword(x)) { | ||
| // if we have a valid keyword, attempt to parse generics | ||
| // also obtain the keywords symbol | ||
| // Let's check if the previous token could denote the start of an item | ||
| // whose kind can have generics. | ||
| if let Some((Ident { name, .. }, IdentIsRaw::No)) = self.prev_token.ident() | ||
| && let kw::Fn | kw::Type | kw::Struct | kw::Enum | kw::Union | kw::Trait = name | ||
| { | ||
| match self.parse_generics() { | ||
| Ok(generic) => { | ||
| if let TokenKind::Ident(symbol, _) = maybe_keyword.kind { | ||
| let ident_name = symbol; | ||
| // at this point, we've found something like | ||
| // `fn <T>id` | ||
| // and current token should be Ident with the item name (i.e. the function name) | ||
| // if there is a `<` after the fn name, then don't show a suggestion, show help | ||
|
|
||
| if !self.look_ahead(1, |t| *t == token::Lt) | ||
| && let Ok(snippet) = | ||
| self.psess.source_map().span_to_snippet(generic.span) | ||
| { | ||
| err.multipart_suggestion( | ||
| format!("place the generic parameter name after the {ident_name} name"), | ||
| vec![ | ||
| (self.token.span.shrink_to_hi(), snippet), | ||
| (generic.span, String::new()) | ||
| ], | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| } else { | ||
| err.help(format!( | ||
| "place the generic parameter name after the {ident_name} name" | ||
| )); | ||
| } | ||
| Ok(generics) => { | ||
| if !self.look_ahead(1, |t| *t == token::Lt) | ||
| && let Ok(snippet) = | ||
| self.psess.source_map().span_to_snippet(generics.span) | ||
| { | ||
| err.multipart_suggestion( | ||
| format!("place the generic parameter name after the {name} name"), | ||
| vec![ | ||
| (self.token.span.shrink_to_hi(), snippet), | ||
| (generics.span, String::new()), | ||
| ], | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| } else { | ||
| err.help(format!( | ||
| "place the generic parameter name after the {name} name" | ||
| )); | ||
| } | ||
| } | ||
| Err(err) => { | ||
| // if there's an error parsing the generics, | ||
| // then don't do a misplaced generics suggestion | ||
| // and emit the expected ident error instead; | ||
| err.cancel(); | ||
| } | ||
| // It's unlikely that the user meant to write a generic parameter list. | ||
| // Let's not show them errors specific to generics. | ||
| Err(err) => err.cancel(), | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -602,15 +582,15 @@ impl<'a> Parser<'a> { | |
| ); | ||
| } | ||
|
|
||
| if let TokenKind::Ident(symbol, _) = &self.prev_token.kind { | ||
| if ["def", "fun", "func", "function"].contains(&symbol.as_str()) { | ||
| err.span_suggestion_short( | ||
| self.prev_token.span, | ||
| format!("write `fn` instead of `{symbol}` to declare a function"), | ||
| "fn", | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| } | ||
| if let Some((ident, IdentIsRaw::No)) = self.prev_token.ident() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apart from the "interpolated identifier" business, this now makes this code aware of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes us now also emit the suggestion for code like below: macro_rules! make { ($kw:ident) => { $kw f() {} } }
make!(function);Compiler Output |
||
| && let "def" | "fun" | "func" | "function" = ident.name.as_str() | ||
| { | ||
| err.span_suggestion_short( | ||
| self.prev_token.span, | ||
| format!("write `fn` instead of `{}` to declare a function", ident.name), | ||
| "fn", | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| } | ||
|
|
||
| if let TokenKind::Ident(prev, _) = &self.prev_token.kind | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1349,16 +1349,16 @@ impl<'a> Parser<'a> { | |
| self.bump(); // `[` | ||
| let index = self.parse_expr()?; | ||
| self.suggest_missing_semicolon_before_array(prev_token.span, open_delim_span)?; | ||
| self.expect(exp!(CloseBracket)).map_err(|mut e| { | ||
| if let TokenKind::Ident(_, _) = prev_token.kind { | ||
| e.span_suggestion_verbose( | ||
| self.expect(exp!(CloseBracket)).map_err(|mut err| { | ||
| if prev_token.is_non_reserved_ident() { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes us now also emit the suggestion for code like below: macro_rules! make { ($ident:ident) => { $ident[0;] } }
fn scope() { make!(ident); }Compiler Output |
||
| err.span_suggestion_verbose( | ||
| prev_token.span.shrink_to_hi(), | ||
| "you might have meant to call a macro", | ||
| "!".to_string(), | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| } | ||
| e | ||
| err | ||
| })?; | ||
| Ok(self.mk_expr( | ||
| lo.to(self.prev_token.span), | ||
|
|
@@ -3688,9 +3688,7 @@ impl<'a> Parser<'a> { | |
| fn is_try_block(&self) -> bool { | ||
| self.token.is_keyword(kw::Try) | ||
| && self.look_ahead(1, |t| { | ||
| *t == token::OpenBrace | ||
| || t.is_metavar_block() | ||
| || t.kind == TokenKind::Ident(sym::bikeshed, IdentIsRaw::No) | ||
| *t == token::OpenBrace || t.is_metavar_block() || t.is_keyword(sym::bikeshed) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As the commit message already states this makes us accept code like: #![feature(try_blocks_heterogeneous)]
macro_rules! make { ($kw:ident) => { try $kw Option<()> {} } }
fn scope() { make!(bikeshed); }Previously, we would error out with |
||
| }) | ||
| && self.token_uninterpolated_span().at_least_rust_2018() | ||
| } | ||
|
|
@@ -3875,12 +3873,8 @@ impl<'a> Parser<'a> { | |
| // Peek the field's ident before parsing its expr in order to emit better diagnostics. | ||
| let peek = self | ||
| .token | ||
| .ident() | ||
| .filter(|(ident, is_raw)| { | ||
| (!ident.is_reserved() || matches!(is_raw, IdentIsRaw::Yes)) | ||
| && self.look_ahead(1, |tok| *tok == token::Colon) | ||
| }) | ||
| .map(|(ident, _)| ident); | ||
| .non_reserved_ident() | ||
| .filter(|_| self.look_ahead(1, |&tok| tok == token::Colon)); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Drive-by cleanup. Taken straight from PR #161775 which incidentally also introduces |
||
|
|
||
| // We still want a field even if its expr didn't parse. | ||
| let field_ident = |this: &Self, guar: ErrorGuaranteed| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -714,9 +714,11 @@ impl<'a> Parser<'a> { | |
| } | ||
|
|
||
| fn check_const_closure(&self) -> bool { | ||
| // FIXME(#146122): Parse `const async ...`, `const gen ...` & `const async gen ...` | ||
| // closures. We already parse `const static async ...` ones etc. | ||
|
|
||
| self.is_keyword_ahead(0, &[kw::Const]) | ||
| && self.look_ahead(1, |t| match &t.kind { | ||
| // async closures do not work with const closures, so we do not parse that here. | ||
| && self.look_ahead(1, |t| match t.uninterpolate().kind { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the added UI test for what this makes us accept. |
||
| token::Ident(kw::Move | kw::Use | kw::Static, IdentIsRaw::No) | ||
| | token::OrOr | ||
| | token::Or => true, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1014,47 +1014,39 @@ impl<'a> Parser<'a> { | |
| break 'break_recover None; | ||
| } | ||
|
|
||
| match &expr.kind { | ||
| ExprKind::Path(None, ast::Path { segments, .. }) | ||
| if let [segment] = segments.as_slice() => | ||
| { | ||
| if self.token == token::Colon | ||
| && self.look_ahead(1, |token| { | ||
| token.is_metavar_block() | ||
| || matches!( | ||
| token.kind, | ||
| token::Ident( | ||
| kw::For | kw::Loop | kw::While, | ||
| token::IdentIsRaw::No | ||
| ) | token::OpenBrace | ||
| ) | ||
| if self.token == token::Colon | ||
| && let ExprKind::Path(None, ast::Path { segments, .. }) = &expr.kind | ||
| && let [segment] = segments.as_slice() | ||
| && self.look_ahead(1, |t| { | ||
| t.is_metavar_block() | ||
| || t.kind == token::OpenBrace | ||
| || t.is_non_raw_ident_where(|ident| { | ||
| matches!(ident.name, kw::For | kw::Loop | kw::While) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes us now also emit this customized diagnostic plus its suggestion for code like: macro_rules! make { ($kw:ident) => { label: $kw {} } }
fn scope() { make!(loop); }Compiler Output |
||
| }) | ||
| { | ||
| let snapshot = self.create_snapshot_for_diagnostic(); | ||
| let label = Label { | ||
| ident: Ident::from_str_and_span( | ||
| &format!("'{}", segment.ident), | ||
| segment.ident.span, | ||
| ), | ||
| }; | ||
| match self.parse_expr_labeled(label, false) { | ||
| Ok(labeled_expr) => { | ||
| e.cancel(); | ||
| self.dcx().emit_err(MalformedLoopLabel { | ||
| span: label.ident.span, | ||
| suggestion: label.ident.span.shrink_to_lo(), | ||
| }); | ||
| *expr = labeled_expr; | ||
| break 'break_recover None; | ||
| } | ||
| Err(err) => { | ||
| err.cancel(); | ||
| self.restore_snapshot(snapshot); | ||
| } | ||
| } | ||
| }) | ||
| { | ||
| let snapshot = self.create_snapshot_for_diagnostic(); | ||
| let label = Label { | ||
| ident: Ident::from_str_and_span( | ||
| &format!("'{}", segment.ident), | ||
| segment.ident.span, | ||
| ), | ||
| }; | ||
| match self.parse_expr_labeled(label, false) { | ||
| Ok(labeled_expr) => { | ||
| e.cancel(); | ||
| self.dcx().emit_err(MalformedLoopLabel { | ||
| span: label.ident.span, | ||
| suggestion: label.ident.span.shrink_to_lo(), | ||
| }); | ||
| *expr = labeled_expr; | ||
| break 'break_recover None; | ||
| } | ||
| Err(err) => { | ||
| err.cancel(); | ||
| self.restore_snapshot(snapshot); | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
|
|
||
| let res = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -915,18 +915,15 @@ impl<'a> Parser<'a> { | |
|
|
||
| /// Parses an `impl B0 + ... + Bn` type. | ||
| fn parse_impl_ty(&mut self, impl_dyn_multi: &mut bool) -> PResult<'a, TyKind> { | ||
| if self.token.is_lifetime() { | ||
| self.look_ahead(1, |t| { | ||
| if let token::Ident(sym, _) = t.kind { | ||
| // parse pattern with "'a Sized" we're supposed to give suggestion like | ||
| // "'a + Sized" | ||
| self.dcx().emit_err(diagnostics::MissingPlusBounds { | ||
| span: self.token.span, | ||
| hi: self.token.span.shrink_to_hi(), | ||
| sym, | ||
| }); | ||
| } | ||
| }) | ||
| // If we encounter a type like `impl 'a Sized`, suggest `impl 'a + Sized`. | ||
| if self.token.is_lifetime() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apart from the "interpolated identifier" business this makes the diagnostic aware of keywords. Previously, it would also trigger on inputs like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes us now also emit this custom diagnostic for code like: macro_rules! make { ($ident:ident) => { impl 'static $ident; } }
fn scope() -> make!(Trait) {}Compiler Output (Excerpt) |
||
| && let Some(ident) = self.look_ahead(1, |t| t.non_reserved_ident()) | ||
| { | ||
| self.dcx().emit_err(diagnostics::MissingPlusBounds { | ||
| span: self.token.span, | ||
| hi: self.token.span.shrink_to_hi(), | ||
| sym: ident.name, | ||
| }); | ||
| } | ||
|
|
||
| // Always parse bounds greedily for better error recovery. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| // Ensure that we can still recognize const closures in the parser even if some of the other closure | ||
| // qualifiers were interpolated. | ||
|
|
||
| //@ check-pass | ||
| #![feature(const_closures, const_destruct, const_trait_impl)] | ||
|
|
||
| use std::marker::Destruct; | ||
|
|
||
| macro_rules! make { | ||
| ($qual:ident $local:ident) => { | ||
| const $qual || { let _ = $local.len(); } | ||
| } | ||
| } | ||
|
|
||
| const fn scope() { | ||
| let local = String::new(); | ||
| call(make!(move local)) | ||
| } | ||
|
|
||
| const fn call(_: impl [const] FnOnce() + [const] Destruct + 'static) {} | ||
|
|
||
| fn main() {} |
Uh oh!
There was an error while loading. Please reload this page.
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.
Makes us now also emit the suggestion for code like below:
Compiler Output
error: expected identifier, found `<` --> t.rs:12:41 | 12 | macro_rules! make { ($kw:ident) => { $kw<T> f() {} } } | ^ expected identifier 13 | make!(fn); | --------- in this macro invocation | = note: this error originates in the macro `make` (in Nightly builds, run with -Z macro-backtrace for more info) help: place the generic parameter name after the fn name | 12 - macro_rules! make { ($kw:ident) => { $kw<T> f() {} } } 12 + macro_rules! make { ($kw:ident) => { $kw f<T>() {} } }View changes since the review