Change keyword order for `impl` restrictions by CoCo-Japan-pan · Pull Request #155442 · rust-lang/rust

CoCo-Japan-pan · 2026-04-17T14:17:44Z

Based on #155222, this PR reorders keywords in trait definitions to group restrictions with visibility. It changes the order from pub(...) const unsafe auto impl(...) trait Foo {...} to pub(...) impl(...) const unsafe auto trait Foo {...}.

Tracking issue for restrictions: #105077

r? @Urgau
cc @jhpratt

rustbot · 2026-04-17T14:17:48Z

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

CoCo-Japan-pan · 2026-04-17T16:25:30Z

The auto keyword was accidentally removed from the set of possible tokens, which caused UI tests to fail.
Added check_keyword(exp!(Auto)) in check_trait_front_matter to fix this.
Also removed 2b39bea.

Urgau · 2026-04-17T17:16:42Z

    pub impl(crate::foo) trait Baz {} //~ ERROR incorrect `impl` restriction
    //[without_gate]~^ ERROR `impl` restrictions are experimental
-    pub unsafe impl(crate::foo) trait BazUnsafe {} //~ ERROR incorrect `impl` restriction
+    pub impl(crate::foo) unsafe trait BazUnsafe {} //~ ERROR incorrect `impl` restriction
    //[without_gate]~^ ERROR `impl` restrictions are experimental
-    pub auto impl(crate::foo) trait BazAuto {} //~ ERROR incorrect `impl` restriction
+    pub impl(crate::foo) auto trait BazAuto {} //~ ERROR incorrect `impl` restriction


I suspect users will be confused about the positioning of the impl(...).

Could add a test with some invalid impl(...) positions and add a fixme in it to improve the recovery and diagnostics.

View changes since the review

Urgau · 2026-04-17T17:22:05Z

+    /// Returns whether any of the given keywords are `dist` tokens ahead over the current token tree.
+    pub(crate) fn is_keyword_tree_ahead(&self, dist: usize, kws: &[Symbol]) -> bool {


I'm not sure I understand why we need this function. Can't we keep using is_keyword_ahead? or look_ahead?

View changes since the review

This is because the path in impl(in? path) can be arbitrarily long, and we need to parse it to determine how many tokens to look ahead. To handle this, we treat the (in? path) portion as a TokenTree::Delimited, so we look ahead over token trees rather than tokens.
For this reason, I have not added an explicit check for CloseParen at the moment. #155442 (comment) However, it might make sense to instead check that the Delimiter is Parenthesis.

I see thanks, that makes sense.

Would be good if you could add a comment explaining that in code, other people might wonder the same thing as me.

rustbot · 2026-04-18T16:00:48Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

rustbot · 2026-04-18T16:22:48Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Urgau · 2026-04-18T17:05:07Z

+        // `impl(`
+        if self.check_keyword(exp!(Impl)) && self.look_ahead(1, |t| t == &token::OpenParen) {
+            // `impl(..) trait`
+            self.is_keyword_tree_ahead(2, &[kw::Trait]) ||
+            // `impl(..) [const | unsafe | auto] trait`
+            (self.is_keyword_tree_ahead(2, &[kw::Const, kw::Unsafe, kw::Auto]) && self.is_keyword_tree_ahead(3, &[kw::Trait])) ||
+            // `impl(..) const [unsafe | auto] trait`
+            (self.is_keyword_tree_ahead(2, &[kw::Const]) && self.is_keyword_tree_ahead(3, &[kw::Unsafe, kw::Auto]) && self.is_keyword_tree_ahead(4, &[kw::Trait])) ||
+            // `impl(..) unsafe auto trait`
+            (self.is_keyword_tree_ahead(2, &[kw::Unsafe]) && self.is_keyword_tree_ahead(3, &[kw::Auto]) && self.is_keyword_tree_ahead(4, &[kw::Trait])) ||
+            // `impl(..) const unsafe auto trait`
+            (self.is_keyword_tree_ahead(2, &[kw::Const]) && self.is_keyword_tree_ahead(3, &[kw::Unsafe]) && self.is_keyword_tree_ahead(4, &[kw::Auto]) && self.is_keyword_tree_ahead(5, &[kw::Trait]))
+        } else {
+            // `trait`
+            self.check_keyword(exp!(Trait)) ||
+            // `auto trait`
+            self.check_keyword(exp!(Auto)) && self.is_keyword_ahead(1, &[kw::Trait]) ||
+            // `unsafe auto? trait`
+            self.check_keyword(exp!(Unsafe)) && (self.is_keyword_ahead(1, &[kw::Trait]) || self.is_keyword_ahead(1, &[kw::Auto]) && self.is_keyword_ahead(2, &[kw::Trait])) ||
+            // `const unsafe? auto? trait`
+            self.check_keyword(exp!(Const)) && (self.is_keyword_ahead(1, &[kw::Trait]) || self.is_keyword_ahead(1, &[kw::Unsafe, kw::Auto]) && self.is_keyword_ahead(2, &[kw::Trait]) || self.is_keyword_ahead(1, &[kw::Unsafe]) && self.is_keyword_ahead(2, &[kw::Auto]) && self.is_keyword_ahead(3, &[kw::Trait]))
        }


I don't this blob of code, it seems brittle and very easy to miss a case. (like impl(..) auto trait?)

I wonder if we could be cleaver enumerate all the combinations ahead and exhaustively check them.

I'm thinking of something like that. (I have not tested it, it's
Rust-ish code)

Suggested change

// `impl(`

if self.check_keyword(exp!(Impl)) && self.look_ahead(1, |t| t == &token::OpenParen) {

// `impl(..) trait`

self.is_keyword_tree_ahead(2, &[kw::Trait]) ||

// `impl(..) [const | unsafe | auto] trait`

(self.is_keyword_tree_ahead(2, &[kw::Const, kw::Unsafe, kw::Auto]) && self.is_keyword_tree_ahead(3, &[kw::Trait])) ||

// `impl(..) const [unsafe | auto] trait`

(self.is_keyword_tree_ahead(2, &[kw::Const]) && self.is_keyword_tree_ahead(3, &[kw::Unsafe, kw::Auto]) && self.is_keyword_tree_ahead(4, &[kw::Trait])) ||

// `impl(..) unsafe auto trait`

(self.is_keyword_tree_ahead(2, &[kw::Unsafe]) && self.is_keyword_tree_ahead(3, &[kw::Auto]) && self.is_keyword_tree_ahead(4, &[kw::Trait])) ||

// `impl(..) const unsafe auto trait`

(self.is_keyword_tree_ahead(2, &[kw::Const]) && self.is_keyword_tree_ahead(3, &[kw::Unsafe]) && self.is_keyword_tree_ahead(4, &[kw::Auto]) && self.is_keyword_tree_ahead(5, &[kw::Trait]))

} else {

// `trait`

self.check_keyword(exp!(Trait)) ||

// `auto trait`

self.check_keyword(exp!(Auto)) && self.is_keyword_ahead(1, &[kw::Trait]) ||

// `unsafe auto? trait`

self.check_keyword(exp!(Unsafe)) && (self.is_keyword_ahead(1, &[kw::Trait]) || self.is_keyword_ahead(1, &[kw::Auto]) && self.is_keyword_ahead(2, &[kw::Trait])) ||

// `const unsafe? auto? trait`

self.check_keyword(exp!(Const)) && (self.is_keyword_ahead(1, &[kw::Trait]) || self.is_keyword_ahead(1, &[kw::Unsafe, kw::Auto]) && self.is_keyword_ahead(2, &[kw::Trait]) || self.is_keyword_ahead(1, &[kw::Unsafe]) && self.is_keyword_ahead(2, &[kw::Auto]) && self.is_keyword_ahead(3, &[kw::Trait]))

}

const SUFFIXES: &[&[kw::Keyword]] = &[

&[kw::Trait],

&[kw::Auto, kw::Trait],

&[kw::Unsafe, kw::Trait],

&[kw::Unsafe, kw::Auto, kw::Trait],

&[kw::Const, kw::Trait],

&[kw::Const, kw::Auto, kw::Trait],

&[kw::Const, kw::Unsafe, kw::Trait],

&[kw::Const, kw::Unsafe, kw::Auto, kw::Trait],

];

// `impl(`

if self.check_keyword(exp!(Impl)) && self.look_ahead(1, |t| t == &token::OpenParen) {

SUFFIXES.iter().enumerate().any(|(idx, prefix)| self.is_keyword_tree_ahead(idx + 2, prefix));

} else {

SUFFIXES.iter().enumerate().any(|(idx, prefix)| self.is_keyword_ahead(idx, prefix));

}

}

View changes since the review

rustbot 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. labels Apr 17, 2026

rustbot assigned Urgau Apr 17, 2026

CoCo-Japan-pan force-pushed the impl-restriction-reorder branch from 8e2d081 to 5d955ca Compare April 17, 2026 14:32

This comment has been minimized.

Sign in to view

CoCo-Japan-pan force-pushed the impl-restriction-reorder branch from 5d955ca to 4178059 Compare April 17, 2026 16:21

This comment has been minimized.

Sign in to view

Urgau reviewed Apr 17, 2026

View reviewed changes

rustbot added T-clippy Relevant to the Clippy team. T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue. labels Apr 18, 2026

CoCo-Japan-pan force-pushed the impl-restriction-reorder branch from b7bec21 to 646fa9a Compare April 18, 2026 16:06

CoCo-Japan-pan added 5 commits April 19, 2026 01:17

Reorder keywords to place restrictions next to visibility

664c84c

Update UI tests

dd687da

Update rustfmt

7315e29

Update AST pretty printing

a3d6bc0

Update HIR pretty printing

78cb850

CoCo-Japan-pan force-pushed the impl-restriction-reorder branch from 646fa9a to 78cb850 Compare April 18, 2026 16:22

Urgau reviewed Apr 18, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Change keyword order for `impl` restrictions#155442

Change keyword order for `impl` restrictions#155442
CoCo-Japan-pan wants to merge 5 commits intorust-lang:mainfrom
CoCo-Japan-pan:impl-restriction-reorder

CoCo-Japan-pan commented Apr 17, 2026 •

edited by rustbot

Loading

Uh oh!

rustbot commented Apr 17, 2026

Uh oh!

This comment has been minimized.

CoCo-Japan-pan commented Apr 17, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Urgau Apr 17, 2026 •

edited by rustbot

Loading

Uh oh!

Uh oh!

Urgau Apr 17, 2026 •

edited by rustbot

Loading

Uh oh!

CoCo-Japan-pan Apr 18, 2026

Uh oh!

Urgau Apr 18, 2026

Uh oh!

rustbot commented Apr 18, 2026

Uh oh!

rustbot commented Apr 18, 2026

Uh oh!

Urgau Apr 18, 2026 •

edited by rustbot

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		/// Returns whether any of the given keywords are `dist` tokens ahead over the current token tree.
		pub(crate) fn is_keyword_tree_ahead(&self, dist: usize, kws: &[Symbol]) -> bool {

Uh oh!

Conversation

CoCo-Japan-pan commented Apr 17, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 17, 2026

Uh oh!

This comment has been minimized.

CoCo-Japan-pan commented Apr 17, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Urgau Apr 17, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Urgau Apr 17, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CoCo-Japan-pan Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Urgau Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Apr 18, 2026

Uh oh!

rustbot commented Apr 18, 2026

Uh oh!

Urgau Apr 18, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CoCo-Japan-pan commented Apr 17, 2026 •

edited by rustbot

Loading

Urgau Apr 17, 2026 •

edited by rustbot

Loading

Urgau Apr 17, 2026 •

edited by rustbot

Loading

Urgau Apr 18, 2026 •

edited by rustbot

Loading