From caee195bdd922c25946276625993b93a5bc0eb41 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 08:42:43 +1000 Subject: Invert early exit conditions in `collect_tokens_trailing_token`. This has been bugging me for a while. I find complex "if any of these are true" conditions easier to think about than complex "if all of these are true" conditions, because you can stop as soon as one is true. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 58 ++++++++++++------------- 1 file changed, 29 insertions(+), 29 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index dc5f98f7be8..6413aa09241 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -199,20 +199,20 @@ impl<'a> Parser<'a> { force_collect: ForceCollect, f: impl FnOnce(&mut Self, ast::AttrVec) -> PResult<'a, (R, bool)>, ) -> PResult<'a, R> { - // Skip collection when nothing could observe the collected tokens, i.e. - // all of the following conditions hold. - // - We are not force collecting tokens (because force collection - // requires tokens by definition). - if matches!(force_collect, ForceCollect::No) - // - None of our outer attributes require tokens. - && attrs.is_complete() - // - Our target doesn't support custom inner attributes (custom + // We must collect if anything could observe the collected tokens, i.e. + // if any of the following conditions hold. + // - We are force collecting tokens (because force collection requires + // tokens by definition). + let needs_collection = matches!(force_collect, ForceCollect::Yes) + // - Any of our outer attributes require tokens. + || !attrs.is_complete() + // - Our target supports custom inner attributes (custom // inner attribute invocation might require token capturing). - && !R::SUPPORTS_CUSTOM_INNER_ATTRS - // - We are not in `capture_cfg` mode (which requires tokens if + || R::SUPPORTS_CUSTOM_INNER_ATTRS + // - We are in `capture_cfg` mode (which requires tokens if // the parsed node has `#[cfg]` or `#[cfg_attr]` attributes). - && !self.capture_cfg - { + || self.capture_cfg; + if !needs_collection { return Ok(f(self, attrs.attrs)?.0); } @@ -250,28 +250,28 @@ impl<'a> Parser<'a> { return Ok(ret); } - // This is similar to the "skip collection" check at the start of this - // function, but now that we've parsed an AST node we have more + // This is similar to the `needs_collection` check at the start of this + // function, but now that we've parsed an AST node we have complete // information available. (If we return early here that means the // setup, such as cloning the token cursor, was unnecessary. That's // hard to avoid.) // - // Skip collection when nothing could observe the collected tokens, i.e. - // all of the following conditions hold. - // - We are not force collecting tokens. - if matches!(force_collect, ForceCollect::No) - // - None of our outer *or* inner attributes require tokens. + // We must collect if anything could observe the collected tokens, i.e. + // if any of the following conditions hold. + // - We are force collecting tokens. + let needs_collection = matches!(force_collect, ForceCollect::Yes) + // - Any of our outer *or* inner attributes require tokens. // (`attrs` was just outer attributes, but `ret.attrs()` is outer - // and inner attributes. That makes this check more precise than - // `attrs.is_complete()` at the start of the function, and we can - // skip the subsequent check on `R::SUPPORTS_CUSTOM_INNER_ATTRS`. - && crate::parser::attr::is_complete(ret.attrs()) - // - We are not in `capture_cfg` mode, or we are but there are no - // `#[cfg]` or `#[cfg_attr]` attributes. (During normal - // non-`capture_cfg` parsing, we don't need any special capturing - // for those attributes, because they're builtin.) - && (!self.capture_cfg || !has_cfg_or_cfg_attr(ret.attrs())) - { + // and inner attributes. So this check is more precise than the + // earlier `attrs.is_complete()` check, and we don't need to + // check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.) + || !crate::parser::attr::is_complete(ret.attrs()) + // - We are in `capture_cfg` mode and there are `#[cfg]` or + // `#[cfg_attr]` attributes. (During normal non-`capture_cfg` + // parsing, we don't need any special capturing for those + // attributes, because they're builtin.) + || (self.capture_cfg && has_cfg_or_cfg_attr(ret.attrs())); + if !needs_collection { return Ok(ret); } -- cgit 1.4.1-3-g733a5 From 4288edb219fb288af524b490bd3830fc7cfafe02 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 09:00:17 +1000 Subject: Inline and remove `AttrWrapper::is_complete`. It has a single call site. This change makes the two `needs_collect` conditions more similar to each other, and therefore easier to understand. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 6413aa09241..27b899300e7 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -60,10 +60,6 @@ impl AttrWrapper { pub fn is_empty(&self) -> bool { self.attrs.is_empty() } - - pub fn is_complete(&self) -> bool { - crate::parser::attr::is_complete(&self.attrs) - } } /// Returns `true` if `attrs` contains a `cfg` or `cfg_attr` attribute @@ -205,7 +201,7 @@ impl<'a> Parser<'a> { // tokens by definition). let needs_collection = matches!(force_collect, ForceCollect::Yes) // - Any of our outer attributes require tokens. - || !attrs.is_complete() + || !crate::parser::attr::is_complete(&attrs.attrs) // - Our target supports custom inner attributes (custom // inner attribute invocation might require token capturing). || R::SUPPORTS_CUSTOM_INNER_ATTRS @@ -261,9 +257,9 @@ impl<'a> Parser<'a> { // - We are force collecting tokens. let needs_collection = matches!(force_collect, ForceCollect::Yes) // - Any of our outer *or* inner attributes require tokens. - // (`attrs` was just outer attributes, but `ret.attrs()` is outer - // and inner attributes. So this check is more precise than the - // earlier `attrs.is_complete()` check, and we don't need to + // (`attr.attrs` was just outer attributes, but `ret.attrs()` is + // outer and inner attributes. So this check is more precise than + // the earlier `is_complete()` check, and we don't need to // check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.) || !crate::parser::attr::is_complete(ret.attrs()) // - We are in `capture_cfg` mode and there are `#[cfg]` or -- cgit 1.4.1-3-g733a5 From 3d363c3d99a5e27fa0b604aae4ca745b4cb4c43a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 09:03:30 +1000 Subject: Move `is_complete` to the module that uses it. And make it non-`pub`. --- compiler/rustc_parse/src/parser/attr.rs | 11 ----------- compiler/rustc_parse/src/parser/attr_wrapper.rs | 15 +++++++++++++-- 2 files changed, 13 insertions(+), 13 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index 0b2c3044039..535b53a836e 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -457,14 +457,3 @@ impl<'a> Parser<'a> { Err(self.dcx().create_err(err)) } } - -/// The attributes are complete if all attributes are either a doc comment or a -/// builtin attribute other than `cfg_attr`. -pub fn is_complete(attrs: &[ast::Attribute]) -> bool { - attrs.iter().all(|attr| { - attr.is_doc_comment() - || attr.ident().is_some_and(|ident| { - ident.name != sym::cfg_attr && rustc_feature::is_builtin_attr_name(ident.name) - }) - }) -} diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 27b899300e7..82fd2257481 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -201,7 +201,7 @@ impl<'a> Parser<'a> { // tokens by definition). let needs_collection = matches!(force_collect, ForceCollect::Yes) // - Any of our outer attributes require tokens. - || !crate::parser::attr::is_complete(&attrs.attrs) + || !is_complete(&attrs.attrs) // - Our target supports custom inner attributes (custom // inner attribute invocation might require token capturing). || R::SUPPORTS_CUSTOM_INNER_ATTRS @@ -261,7 +261,7 @@ impl<'a> Parser<'a> { // outer and inner attributes. So this check is more precise than // the earlier `is_complete()` check, and we don't need to // check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.) - || !crate::parser::attr::is_complete(ret.attrs()) + || !is_complete(ret.attrs()) // - We are in `capture_cfg` mode and there are `#[cfg]` or // `#[cfg_attr]` attributes. (During normal non-`capture_cfg` // parsing, we don't need any special capturing for those @@ -457,6 +457,17 @@ fn make_attr_token_stream( AttrTokenStream::new(stack_top.inner) } +/// The attributes are complete if all attributes are either a doc comment or a +/// builtin attribute other than `cfg_attr`. +fn is_complete(attrs: &[ast::Attribute]) -> bool { + attrs.iter().all(|attr| { + attr.is_doc_comment() + || attr.ident().is_some_and(|ident| { + ident.name != sym::cfg_attr && rustc_feature::is_builtin_attr_name(ident.name) + }) + }) +} + // Some types are used a lot. Make sure they don't unintentionally get bigger. #[cfg(target_pointer_width = "64")] mod size_asserts { -- cgit 1.4.1-3-g733a5 From e631b1ebfa273fc4703fa2fd60fde281340d4909 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 09:06:19 +1000 Subject: Invert the sense of `is_complete` and rename it as `needs_tokens`. I have always found `is_complete` an unhelpful name. The new name (and inverted sense) fits in better with the conditions at its call sites. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 82fd2257481..1bc41e68cf3 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -201,7 +201,7 @@ impl<'a> Parser<'a> { // tokens by definition). let needs_collection = matches!(force_collect, ForceCollect::Yes) // - Any of our outer attributes require tokens. - || !is_complete(&attrs.attrs) + || needs_tokens(&attrs.attrs) // - Our target supports custom inner attributes (custom // inner attribute invocation might require token capturing). || R::SUPPORTS_CUSTOM_INNER_ATTRS @@ -259,9 +259,9 @@ impl<'a> Parser<'a> { // - Any of our outer *or* inner attributes require tokens. // (`attr.attrs` was just outer attributes, but `ret.attrs()` is // outer and inner attributes. So this check is more precise than - // the earlier `is_complete()` check, and we don't need to + // the earlier `needs_tokens` check, and we don't need to // check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.) - || !is_complete(ret.attrs()) + || needs_tokens(ret.attrs()) // - We are in `capture_cfg` mode and there are `#[cfg]` or // `#[cfg_attr]` attributes. (During normal non-`capture_cfg` // parsing, we don't need any special capturing for those @@ -457,14 +457,16 @@ fn make_attr_token_stream( AttrTokenStream::new(stack_top.inner) } -/// The attributes are complete if all attributes are either a doc comment or a -/// builtin attribute other than `cfg_attr`. -fn is_complete(attrs: &[ast::Attribute]) -> bool { - attrs.iter().all(|attr| { - attr.is_doc_comment() - || attr.ident().is_some_and(|ident| { - ident.name != sym::cfg_attr && rustc_feature::is_builtin_attr_name(ident.name) - }) +/// Tokens are needed if: +/// - any non-single-segment attributes (other than doc comments) are present; or +/// - any `cfg_attr` attributes are present; +/// - any single-segment, non-builtin attributes are present. +fn needs_tokens(attrs: &[ast::Attribute]) -> bool { + attrs.iter().any(|attr| match attr.ident() { + None => !attr.is_doc_comment(), + Some(ident) => { + ident.name == sym::cfg_attr || !rustc_feature::is_builtin_attr_name(ident.name) + } }) } -- cgit 1.4.1-3-g733a5 From a560810a69a09452b4cd0c3173b6af98496dba35 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 10:54:56 +1000 Subject: Don't include inner attribute ranges in `CaptureState`. The current code is this: ``` self.capture_state.replace_ranges.push((start_pos..end_pos, Some(target))); self.capture_state.replace_ranges.extend(inner_attr_replace_ranges); ``` What's not obvious is that every range in `inner_attr_replace_ranges` must be a strict sub-range of `start_pos..end_pos`. Which means, in `LazyAttrTokenStreamImpl::to_attr_token_stream`, they will be done first, and then the `start_pos..end_pos` replacement will just overwrite them. So they aren't needed. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index dc5f98f7be8..1fd0654e0ab 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -337,8 +337,7 @@ impl<'a> Parser<'a> { // When parsing `m`: // - `start_pos..end_pos` is `0..34` (`mod m`, excluding the `#[cfg_eval]` attribute). // - `inner_attr_replace_ranges` is empty. - // - `replace_range_start..replace_ranges_end` has two entries. - // - One to delete the inner attribute (`17..27`), obtained when parsing `g` (see above). + // - `replace_range_start..replace_ranges_end` has one entry. // - One `AttrsTarget` (added below when parsing `g`) to replace all of `g` (`3..33`, // including its outer attribute), with: // - `attrs`: includes the outer and the inner attr. @@ -369,12 +368,10 @@ impl<'a> Parser<'a> { // What is the status here when parsing the example code at the top of this method? // - // When parsing `g`, we add two entries: + // When parsing `g`, we add one entry: // - The `start_pos..end_pos` (`3..33`) entry has a new `AttrsTarget` with: // - `attrs`: includes the outer and the inner attr. // - `tokens`: lazy tokens for `g` (with its inner attr deleted). - // - `inner_attr_replace_ranges` contains the one entry to delete the inner attr's - // tokens (`17..27`). // // When parsing `m`, we do nothing here. @@ -384,7 +381,6 @@ impl<'a> Parser<'a> { let start_pos = if has_outer_attrs { attrs.start_pos } else { start_pos }; let target = AttrsTarget { attrs: ret.attrs().iter().cloned().collect(), tokens }; self.capture_state.replace_ranges.push((start_pos..end_pos, Some(target))); - self.capture_state.replace_ranges.extend(inner_attr_replace_ranges); } else if matches!(self.capture_state.capturing, Capturing::No) { // Only clear the ranges once we've finished capturing entirely, i.e. we've finished // the outermost call to this method. -- cgit 1.4.1-3-g733a5 From 6e87858f26bcb9fadbd60f9e8ee24816a55faa80 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 14:02:15 +1000 Subject: Fix a comment. Imagine you have replace ranges (2..20,X) and (5..15,Y), and these tokens: ``` a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x ``` If we replace (5..15,Y) first, then (2..20,X) we get this sequence ``` a,b,c,d,e,Y,_,_,_,_,_,_,_,_,_,p,q,r,s,t,u,v,w,x a,b,X,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,u,v,w,x ``` which is what we want. If we do it in the other order, we get this: ``` a,b,X,_,_,_,_,_,_,_,_,_,_,_,_,p,q,r,s,t,u,v,w,x a,b,X,_,_,Y,_,_,_,_,_,_,_,_,_,_,_,_,_,_,u,v,w,x ``` which is wrong. So it's true that we need the `.rev()` but the comment is wrong about why. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 1fd0654e0ab..a90bfb574e2 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -137,9 +137,9 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl { // `#[cfg(FALSE)] struct Foo { #[cfg(FALSE)] field: bool }` // // By starting processing from the replace range with the greatest - // start position, we ensure that any replace range which encloses - // another replace range will capture the *replaced* tokens for the inner - // range, not the original tokens. + // start position, we ensure that any (outer) replace range which + // encloses another (inner) replace range will fully overwrite the + // inner range's replacement. for (range, target) in replace_ranges.into_iter().rev() { assert!(!range.is_empty(), "Cannot replace an empty range: {range:?}"); -- cgit 1.4.1-3-g733a5 From 6ea2da5a28580dba214b32831d0a6952e2ebce91 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 13:20:27 +1000 Subject: Tweak a loop. A fully imperative style is easier to read than a half-iterator, half-imperative style. Also, rename `inner_attr` as `attr` because it might be an outer attribute. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index a90bfb574e2..abee668cc6c 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -297,11 +297,13 @@ impl<'a> Parser<'a> { // with `None`, which means the relevant tokens will be removed. (More // details below.) let mut inner_attr_replace_ranges = Vec::new(); - for inner_attr in ret.attrs().iter().filter(|a| a.style == ast::AttrStyle::Inner) { - if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&inner_attr.id) { - inner_attr_replace_ranges.push((attr_range, None)); - } else { - self.dcx().span_delayed_bug(inner_attr.span, "Missing token range for attribute"); + for attr in ret.attrs() { + if attr.style == ast::AttrStyle::Inner { + if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&attr.id) { + inner_attr_replace_ranges.push((attr_range, None)); + } else { + self.dcx().span_delayed_bug(attr.span, "Missing token range for attribute"); + } } } -- cgit 1.4.1-3-g733a5 From 55d37ae711b1a1ab82ef02d0594f92167e18af90 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Jul 2024 14:00:29 +1000 Subject: Remove an unnecessary block. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index abee668cc6c..389e4a6267d 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -114,17 +114,15 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl { replace_ranges.sort_by_key(|(range, _)| range.start); #[cfg(debug_assertions)] - { - for [(range, tokens), (next_range, next_tokens)] in replace_ranges.array_windows() { - assert!( - range.end <= next_range.start || range.end >= next_range.end, - "Replace ranges should either be disjoint or nested: ({:?}, {:?}) ({:?}, {:?})", - range, - tokens, - next_range, - next_tokens, - ); - } + for [(range, tokens), (next_range, next_tokens)] in replace_ranges.array_windows() { + assert!( + range.end <= next_range.start || range.end >= next_range.end, + "Replace ranges should either be disjoint or nested: ({:?}, {:?}) ({:?}, {:?})", + range, + tokens, + next_range, + next_tokens, + ); } // Process the replace ranges, starting from the highest start -- cgit 1.4.1-3-g733a5 From 3fdc99193e95a29b6fd2228b0acc5f8b4a81feda Mon Sep 17 00:00:00 2001 From: Tamme Dittrich Date: Fri, 26 Jul 2024 15:14:05 +0200 Subject: Improve error message for `extern "C" unsafe fn()` This was handled correctly already for `extern unsafe fn()`. Co-authored-by: Folkert --- compiler/rustc_parse/src/parser/item.rs | 9 ++++++--- tests/ui/parser/issues/issue-19398.rs | 6 +++++- tests/ui/parser/issues/issue-19398.stderr | 14 +++++++------- .../issues/issue-87217-keyword-order/wrong-unsafe-abi.rs | 6 +++++- .../issue-87217-keyword-order/wrong-unsafe-abi.stderr | 9 +++++++-- 5 files changed, 30 insertions(+), 14 deletions(-) (limited to 'compiler/rustc_parse/src/parser') diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 9aaf4b99243..112855e6d1f 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -2483,12 +2483,15 @@ impl<'a> Parser<'a> { /// `check_pub` adds additional `pub` to the checks in case users place it /// wrongly, can be used to ensure `pub` never comes after `default`. pub(super) fn check_fn_front_matter(&mut self, check_pub: bool, case: Case) -> bool { + const ALL_QUALS: &[Symbol] = + &[kw::Pub, kw::Gen, kw::Const, kw::Async, kw::Unsafe, kw::Safe, kw::Extern]; + // We use an over-approximation here. // `const const`, `fn const` won't parse, but we're not stepping over other syntax either. // `pub` is added in case users got confused with the ordering like `async pub fn`, // only if it wasn't preceded by `default` as `default pub` is invalid. let quals: &[Symbol] = if check_pub { - &[kw::Pub, kw::Gen, kw::Const, kw::Async, kw::Unsafe, kw::Safe, kw::Extern] + ALL_QUALS } else { &[kw::Gen, kw::Const, kw::Async, kw::Unsafe, kw::Safe, kw::Extern] }; @@ -2518,9 +2521,9 @@ impl<'a> Parser<'a> { || self.check_keyword_case(kw::Extern, case) && self.look_ahead(1, |t| t.can_begin_string_literal()) && (self.look_ahead(2, |t| t.is_keyword_case(kw::Fn, case)) || - // this branch is only for better diagnostic in later, `pub` is not allowed here + // this branch is only for better diagnostics; `pub`, `unsafe`, etc. are not allowed here (self.may_recover() - && self.look_ahead(2, |t| t.is_keyword(kw::Pub)) + && self.look_ahead(2, |t| ALL_QUALS.iter().any(|&kw| t.is_keyword(kw))) && self.look_ahead(3, |t| t.is_keyword_case(kw::Fn, case)))) } diff --git a/tests/ui/parser/issues/issue-19398.rs b/tests/ui/parser/issues/issue-19398.rs index 46eb320a172..358f65f1da5 100644 --- a/tests/ui/parser/issues/issue-19398.rs +++ b/tests/ui/parser/issues/issue-19398.rs @@ -1,6 +1,10 @@ trait T { extern "Rust" unsafe fn foo(); - //~^ ERROR expected `{`, found keyword `unsafe` + //~^ ERROR expected `fn`, found keyword `unsafe` + //~| NOTE expected `fn` + //~| HELP `unsafe` must come before `extern "Rust"` + //~| SUGGESTION unsafe extern "Rust" + //~| NOTE keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern` } fn main() {} diff --git a/tests/ui/parser/issues/issue-19398.stderr b/tests/ui/parser/issues/issue-19398.stderr index 236fac673b6..2b97ec50c91 100644 --- a/tests/ui/parser/issues/issue-19398.stderr +++ b/tests/ui/parser/issues/issue-19398.stderr @@ -1,13 +1,13 @@ -error: expected `{`, found keyword `unsafe` +error: expected `fn`, found keyword `unsafe` --> $DIR/issue-19398.rs:2:19 | -LL | trait T { - | - while parsing this item list starting here LL | extern "Rust" unsafe fn foo(); - | ^^^^^^ expected `{` -LL | -LL | } - | - the item list ends here + | --------------^^^^^^ + | | | + | | expected `fn` + | help: `unsafe` must come before `extern "Rust"`: `unsafe extern "Rust"` + | + = note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern` error: aborting due to 1 previous error diff --git a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs index c97425d8ade..794ac635c76 100644 --- a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs +++ b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.rs @@ -7,6 +7,10 @@ // Visibilities are tested elsewhere. extern "C" unsafe fn test() {} -//~^ ERROR +//~^ ERROR expected `fn`, found keyword `unsafe` +//~| NOTE expected `fn` +//~| HELP `unsafe` must come before `extern "C"` +//~| SUGGESTION unsafe extern "C" +//~| NOTE keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern` fn main() {} diff --git a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr index 42c9764cc1e..8ed037869c8 100644 --- a/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr +++ b/tests/ui/parser/issues/issue-87217-keyword-order/wrong-unsafe-abi.stderr @@ -1,8 +1,13 @@ -error: expected `{`, found keyword `unsafe` +error: expected `fn`, found keyword `unsafe` --> $DIR/wrong-unsafe-abi.rs:9:12 | LL | extern "C" unsafe fn test() {} - | ^^^^^^ expected `{` + | -----------^^^^^^ + | | | + | | expected `fn` + | help: `unsafe` must come before `extern "C"`: `unsafe extern "C"` + | + = note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern` error: aborting due to 1 previous error -- cgit 1.4.1-3-g733a5