Rollup merge of #155442 - CoCo-Japan-pan:impl-restriction-reorder, r=Urgau,fmease,jhpratt

Change keyword order for `impl` restrictions

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

Author: GuillaumeGomez

compiler/rustc_ast/src/ast.rs

@@ -3786,10 +3786,10 @@ pub struct TraitAlias {
 
 #[derive(Clone, Encodable, Decodable, Debug, Walkable)]
 pub struct Trait {
+    pub impl_restriction: ImplRestriction,
     pub constness: Const,
     pub safety: Safety,
     pub is_auto: IsAuto,
-    pub impl_restriction: ImplRestriction,
     pub ident: Ident,
     pub generics: Generics,
     #[visitable(extra = BoundKind::SuperTraits)]

compiler/rustc_ast_lowering/src/item.rs

@@ -556,10 +556,10 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                 })
             }
             ItemKind::Trait(box Trait {
+                impl_restriction,
                 constness,
                 is_auto,
                 safety,
-                impl_restriction,
                 ident,
                 generics,
                 bounds,
@@ -586,10 +586,10 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
                     },
                 );
                 hir::ItemKind::Trait(
+                    impl_restriction,
                     constness,
                     *is_auto,
                     safety,
-                    impl_restriction,
                     ident,
                     generics,
                     bounds,

compiler/rustc_ast_pretty/src/pprust/state/item.rs

@@ -371,21 +371,21 @@ impl<'a> State<'a> {
                 self.bclose(item.span, empty, cb);
             }
             ast::ItemKind::Trait(box ast::Trait {
+                impl_restriction,
                 constness,
                 safety,
                 is_auto,
-                impl_restriction,
                 ident,
                 generics,
                 bounds,
                 items,
             }) => {
                 let (cb, ib) = self.head("");
                 self.print_visibility(&item.vis);
+                self.print_impl_restriction(impl_restriction);
                 self.print_constness(*constness);
                 self.print_safety(*safety);
                 self.print_is_auto(*is_auto);
-                self.print_impl_restriction(impl_restriction);
                 self.word_nbsp("trait");
                 self.print_ident(*ident);
                 self.print_generic_params(&generics.params);

compiler/rustc_hir/src/hir.rs

@@ -4457,17 +4457,17 @@ impl<'hir> Item<'hir> {
 
         expect_trait,
             (
+                &'hir ImplRestriction<'hir>,
                 Constness,
                 IsAuto,
                 Safety,
-                &'hir ImplRestriction<'hir>,
                 Ident,
                 &'hir Generics<'hir>,
                 GenericBounds<'hir>,
                 &'hir [TraitItemId]
             ),
-            ItemKind::Trait(constness, is_auto, safety, impl_restriction, ident, generics, bounds, items),
-            (*constness, *is_auto, *safety, impl_restriction, *ident, generics, bounds, items);
+            ItemKind::Trait(impl_restriction, constness, is_auto, safety, ident, generics, bounds, items),
+            (impl_restriction, *constness, *is_auto, *safety, *ident, generics, bounds, items);
 
         expect_trait_alias, (Constness, Ident, &'hir Generics<'hir>, GenericBounds<'hir>),
             ItemKind::TraitAlias(constness, ident, generics, bounds), (*constness, *ident, generics, bounds);
@@ -4659,10 +4659,10 @@ pub enum ItemKind<'hir> {
     Union(Ident, &'hir Generics<'hir>, VariantData<'hir>),
     /// A trait definition.
     Trait(
+        &'hir ImplRestriction<'hir>,
         Constness,
         IsAuto,
         Safety,
-        &'hir ImplRestriction<'hir>,
         Ident,
         &'hir Generics<'hir>,
         GenericBounds<'hir>,

compiler/rustc_hir/src/intravisit.rs

@@ -619,10 +619,10 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V::
             try_visit!(visitor.visit_variant_data(struct_definition));
         }
         ItemKind::Trait(
+            ref impl_restriction,
             _constness,
             _is_auto,
             _safety,
-            ref impl_restriction,
             ident,
             ref generics,
             bounds,

compiler/rustc_hir_analysis/src/collect.rs

@@ -895,7 +895,7 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {
     let item = tcx.hir_expect_item(def_id);
 
     let (constness, is_alias, is_auto, safety, impl_restriction) = match item.kind {
-        hir::ItemKind::Trait(constness, is_auto, safety, impl_restriction, ..) => (
+        hir::ItemKind::Trait(impl_restriction, constness, is_auto, safety, ..) => (
             constness,
             false,
             is_auto == hir::IsAuto::Yes,
@@ -959,9 +959,9 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {
 
     ty::TraitDef {
         def_id: def_id.to_def_id(),
+        impl_restriction,
         safety,
         constness,
-        impl_restriction,
         paren_sugar,
         has_auto_impl: is_auto,
         is_marker,

compiler/rustc_hir_pretty/src/lib.rs

@@ -758,20 +758,20 @@ impl<'a> State<'a> {
                 self.bclose(item.span, cb);
             }
             hir::ItemKind::Trait(
+                impl_restriction,
                 constness,
                 is_auto,
                 safety,
-                impl_restriction,
                 ident,
                 generics,
                 bounds,
                 trait_items,
             ) => {
                 let (cb, ib) = self.head("");
+                self.print_impl_restriction(impl_restriction);
                 self.print_constness(constness);
                 self.print_is_auto(is_auto);
                 self.print_safety(safety);
-                self.print_impl_restriction(impl_restriction);
                 self.word_nbsp("trait");
                 self.print_ident(ident);
                 self.print_generic_params(generics.params);

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -1900,7 +1900,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     entry.1.insert((self_ty.span, String::new()));
                 }
                 Some(Node::Item(hir::Item {
-                    kind: hir::ItemKind::Trait(_, rustc_ast::ast::IsAuto::Yes, ..),
+                    kind: hir::ItemKind::Trait(_, _, rustc_ast::ast::IsAuto::Yes, ..),
                     span: item_span,
                     ..
                 })) => {

compiler/rustc_middle/src/ty/trait_def.rs

@@ -20,14 +20,14 @@ use crate::ty::{Ident, Ty, TyCtxt};
 pub struct TraitDef {
     pub def_id: DefId,
 
+    /// Restrictions on trait implementations.
+    pub impl_restriction: ImplRestrictionKind,
+
     pub safety: hir::Safety,
 
     /// Whether this trait is `const`.
     pub constness: hir::Constness,
 
-    /// Restrictions on trait implementations.
-    pub impl_restriction: ImplRestrictionKind,
-
     /// If `true`, then this trait had the `#[rustc_paren_sugar]`
     /// attribute, indicating that it should be used with `Foo()`
     /// sugar. This is a temporary thing -- eventually any trait will

compiler/rustc_parse/src/parser/item.rs

@@ -1052,80 +1052,69 @@ impl<'a> Parser<'a> {
         }
     }
 
-    /// Is there an `[ impl(in? path) ]? trait` item `dist` tokens ahead?
-    fn is_trait_with_maybe_impl_restriction_in_front(&self, dist: usize) -> bool {
-        // `trait`
-        if self.is_keyword_ahead(dist, &[kw::Trait]) {
-            return true;
-        }
-        // `impl(`
-        if !self.is_keyword_ahead(dist, &[kw::Impl])
-            || !self.look_ahead(dist + 1, |t| t == &token::OpenParen)
-        {
-            return false;
-        }
-        // `crate | super | self) trait`
-        if self.is_keyword_ahead(dist + 2, &[kw::Crate, kw::Super, kw::SelfLower])
-            && self.look_ahead(dist + 3, |t| t == &token::CloseParen)
-            && self.is_keyword_ahead(dist + 4, &[kw::Trait])
-        {
-            return true;
-        }
-        // `impl(in? something) trait`
-        // We catch cases where the `in` keyword is missing to provide a
-        // better error message. This is handled later in
-        // `self.recover_incorrect_impl_restriction`.
-        self.tree_look_ahead(dist + 2, |t| {
-            if let TokenTree::Token(token, _) = t { token.is_keyword(kw::Trait) } else { false }
-        })
-        .unwrap_or(false)
-    }
-
-    /// Is this an `(const unsafe? auto? [ impl(in? path) ]? | unsafe auto? [ impl(in? path) ]? | auto [ impl(in? path) ]? | [ impl(in? path) ]?) trait` item?
+    /// Is this an `[impl(in? path)]? const? unsafe? auto? trait` item?
     fn check_trait_front_matter(&mut self) -> bool {
-        // `[ impl(in? path) ]? trait`
-        if self.is_trait_with_maybe_impl_restriction_in_front(0) {
-            return true;
-        }
-        // `auto [ impl(in? path) ]? trait`
-        if self.check_keyword(exp!(Auto)) && self.is_trait_with_maybe_impl_restriction_in_front(1) {
-            return true;
-        }
-        // `unsafe auto? [ impl(in? path) ]? trait`
-        if self.check_keyword(exp!(Unsafe))
-            && (self.is_trait_with_maybe_impl_restriction_in_front(1)
-                || self.is_keyword_ahead(1, &[kw::Auto])
-                    && self.is_trait_with_maybe_impl_restriction_in_front(2))
-        {
-            return true;
-        }
-        // `const` ...
-        if !self.check_keyword(exp!(Const)) {
-            return false;
-        }
-        // `const [ impl(in? path) ]? trait`
-        if self.is_trait_with_maybe_impl_restriction_in_front(1) {
-            return true;
-        }
-        // `const (unsafe | auto) [ impl(in? path) ]? trait`
-        if self.is_keyword_ahead(1, &[kw::Unsafe, kw::Auto])
-            && self.is_trait_with_maybe_impl_restriction_in_front(2)
-        {
-            return true;
+        const SUFFIXES: &[&[Symbol]] = &[
+            &[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) {
+            // `impl(in` unambiguously introduces an `impl` restriction
+            if self.is_keyword_ahead(2, &[kw::In]) {
+                return true;
+            }
+            // `impl(crate | self | super)` + SUFFIX
+            if self.is_keyword_ahead(2, &[kw::Crate, kw::SelfLower, kw::Super])
+                && self.look_ahead(3, |t| t == &token::CloseParen)
+                && SUFFIXES.iter().any(|suffix| {
+                    suffix.iter().enumerate().all(|(i, kw)| self.is_keyword_ahead(i + 4, &[*kw]))
+                })
+            {
+                return true;
+            }
+            // Recover cases like `impl(path::to::module)` + SUFFIX to suggest inserting `in`.
+            SUFFIXES.iter().any(|suffix| {
+                suffix.iter().enumerate().all(|(i, kw)| {
+                    self.tree_look_ahead(i + 2, |t| {
+                        if let TokenTree::Token(token, _) = t {
+                            token.is_keyword(*kw)
+                        } else {
+                            false
+                        }
+                    })
+                    .unwrap_or(false)
+                })
+            })
+        } else {
+            SUFFIXES.iter().any(|suffix| {
+                suffix.iter().enumerate().all(|(i, kw)| {
+                    // We use `check_keyword` for the first token to include it in the expected tokens.
+                    if i == 0 {
+                        match *kw {
+                            kw::Const => self.check_keyword(exp!(Const)),
+                            kw::Unsafe => self.check_keyword(exp!(Unsafe)),
+                            kw::Auto => self.check_keyword(exp!(Auto)),
+                            kw::Trait => self.check_keyword(exp!(Trait)),
+                            _ => unreachable!(),
+                        }
+                    } else {
+                        self.is_keyword_ahead(i, &[*kw])
+                    }
+                })
+            })
         }
-        // `const unsafe auto [ impl(in? path) ]? trait`
-        self.is_keyword_ahead(1, &[kw::Unsafe])
-            && self.is_keyword_ahead(2, &[kw::Auto])
-            && self.is_trait_with_maybe_impl_restriction_in_front(3)
     }
 
-    /// Parses `const? unsafe? auto? [impl(in? path)]? trait Foo { ... }` or `trait Foo = Bar;`.
-    ///
-    /// FIXME(restrictions): The current keyword order follows the grammar specified in RFC 3323.
-    /// However, whether the restriction should be grouped closer to the visibility modifier
-    /// (e.g., `pub impl(crate) const unsafe auto trait`) remains an unresolved design question.
-    /// This ordering must be kept in sync with the logic in `check_trait_front_matter`.
+    /// Parses `[impl(in? path)]? const? unsafe? auto? trait Foo { ... }` or `trait Foo = Bar;`.
     fn parse_item_trait(&mut self, attrs: &mut AttrVec, lo: Span) -> PResult<'a, ItemKind> {
+        let impl_restriction = self.parse_impl_restriction()?;
         let constness = self.parse_constness(Case::Sensitive);
         if let Const::Yes(span) = constness {
             self.psess.gated_spans.gate(sym::const_trait_impl, span);
@@ -1139,8 +1128,6 @@ impl<'a> Parser<'a> {
             IsAuto::No
         };
 
-        let impl_restriction = self.parse_impl_restriction()?;
-
         self.expect_keyword(exp!(Trait))?;
         let ident = self.parse_ident()?;
         let mut generics = self.parse_generics()?;
@@ -1181,10 +1168,10 @@ impl<'a> Parser<'a> {
             generics.where_clause = self.parse_where_clause()?;
             let items = self.parse_item_list(attrs, |p| p.parse_trait_item(ForceCollect::No))?;
             Ok(ItemKind::Trait(Box::new(Trait {
+                impl_restriction,
                 constness,
                 is_auto,
                 safety,
-                impl_restriction,
                 ident,
                 generics,
                 bounds,
@@ -2966,7 +2953,7 @@ impl<'a> Parser<'a> {
                         && !self.is_unsafe_foreign_mod()
                         // Rule out `async gen {` and `async gen move {`
                         && !self.is_async_gen_block()
-                        // Rule out `const unsafe auto` and `const unsafe trait` and `const unsafe impl`.
+                        // Rule out `const unsafe auto` and `const unsafe trait` and `const unsafe impl`
                         && !self.is_keyword_ahead(2, &[kw::Auto, kw::Trait, kw::Impl])
                     )
                 })