Auto merge of rust-lang#70261 - Centril:angle-args-partition, r=varkor

Move arg/constraint partition check to validation & improve recovery

- In the first commit, we move the check rejecting e.g., `<'a, Item = u8, String>` from the parser into AST validation.
- We then use this to improve the code for parsing generic arguments.
- And we add recovery for e.g., `<Item = >` (missing), `<Item = 42>` (constant), and `<Item = 'a>` (lifetime).

This is also preparatory work for supporting rust-lang#70256.

r? @varkor

Author: bors

src/librustc_ast/ast.rs

@@ -214,11 +214,18 @@ impl GenericArg {
 pub struct AngleBracketedArgs {
     /// The overall span.
     pub span: Span,
-    /// The arguments for this path segment.
-    pub args: Vec<GenericArg>,
-    /// Constraints on associated types, if any.
-    /// E.g., `Foo<A = Bar, B: Baz>`.
-    pub constraints: Vec<AssocTyConstraint>,
+    /// The comma separated parts in the `<...>`.
+    pub args: Vec<AngleBracketedArg>,
+}
+
+/// Either an argument for a parameter e.g., `'a`, `Vec<u8>`, `0`,
+/// or a constraint on an associated item, e.g., `Item = String` or `Item: Bound`.
+#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
+pub enum AngleBracketedArg {
+    /// Argument for a generic parameter.
+    Arg(GenericArg),
+    /// Constraint for an associated item.
+    Constraint(AssocTyConstraint),
 }
 
 impl Into<Option<P<GenericArgs>>> for AngleBracketedArgs {
@@ -248,11 +255,13 @@ pub struct ParenthesizedArgs {
 
 impl ParenthesizedArgs {
     pub fn as_angle_bracketed_args(&self) -> AngleBracketedArgs {
-        AngleBracketedArgs {
-            span: self.span,
-            args: self.inputs.iter().cloned().map(GenericArg::Type).collect(),
-            constraints: vec![],
-        }
+        let args = self
+            .inputs
+            .iter()
+            .cloned()
+            .map(|input| AngleBracketedArg::Arg(GenericArg::Type(input)))
+            .collect();
+        AngleBracketedArgs { span: self.span, args }
     }
 }
 

src/librustc_ast/mut_visit.rs

@@ -546,9 +546,11 @@ pub fn noop_visit_angle_bracketed_parameter_data<T: MutVisitor>(
     data: &mut AngleBracketedArgs,
     vis: &mut T,
 ) {
-    let AngleBracketedArgs { args, constraints, span } = data;
-    visit_vec(args, |arg| vis.visit_generic_arg(arg));
-    visit_vec(constraints, |constraint| vis.visit_ty_constraint(constraint));
+    let AngleBracketedArgs { args, span } = data;
+    visit_vec(args, |arg| match arg {
+        AngleBracketedArg::Arg(arg) => vis.visit_generic_arg(arg),
+        AngleBracketedArg::Constraint(constraint) => vis.visit_ty_constraint(constraint),
+    });
     vis.visit_span(span);
 }
 

src/librustc_ast/visit.rs

@@ -464,8 +464,12 @@ where
 {
     match *generic_args {
         GenericArgs::AngleBracketed(ref data) => {
-            walk_list!(visitor, visit_generic_arg, &data.args);
-            walk_list!(visitor, visit_assoc_ty_constraint, &data.constraints);
+            for arg in &data.args {
+                match arg {
+                    AngleBracketedArg::Arg(a) => visitor.visit_generic_arg(a),
+                    AngleBracketedArg::Constraint(c) => visitor.visit_assoc_ty_constraint(c),
+                }
+            }
         }
         GenericArgs::Parenthesized(ref data) => {
             walk_list!(visitor, visit_ty, &data.inputs);

src/librustc_ast_lowering/lib.rs

@@ -34,6 +34,7 @@
 #![feature(crate_visibility_modifier)]
 #![feature(marker_trait_attr)]
 #![feature(specialization)]
+#![feature(or_patterns)]
 #![recursion_limit = "256"]
 
 use rustc_ast::ast;

src/librustc_ast_lowering/path.rs

@@ -366,22 +366,27 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         param_mode: ParamMode,
         mut itctx: ImplTraitContext<'_, 'hir>,
     ) -> (GenericArgsCtor<'hir>, bool) {
-        let &AngleBracketedArgs { ref args, ref constraints, .. } = data;
-        let has_non_lt_args = args.iter().any(|arg| match arg {
-            ast::GenericArg::Lifetime(_) => false,
-            ast::GenericArg::Type(_) => true,
-            ast::GenericArg::Const(_) => true,
+        let has_non_lt_args = data.args.iter().any(|arg| match arg {
+            AngleBracketedArg::Arg(ast::GenericArg::Lifetime(_))
+            | AngleBracketedArg::Constraint(_) => false,
+            AngleBracketedArg::Arg(ast::GenericArg::Type(_) | ast::GenericArg::Const(_)) => true,
         });
-        (
-            GenericArgsCtor {
-                args: args.iter().map(|a| self.lower_generic_arg(a, itctx.reborrow())).collect(),
-                bindings: self.arena.alloc_from_iter(
-                    constraints.iter().map(|b| self.lower_assoc_ty_constraint(b, itctx.reborrow())),
-                ),
-                parenthesized: false,
-            },
-            !has_non_lt_args && param_mode == ParamMode::Optional,
-        )
+        let args = data
+            .args
+            .iter()
+            .filter_map(|arg| match arg {
+                AngleBracketedArg::Arg(arg) => Some(self.lower_generic_arg(arg, itctx.reborrow())),
+                AngleBracketedArg::Constraint(_) => None,
+            })
+            .collect();
+        let bindings = self.arena.alloc_from_iter(data.args.iter().filter_map(|arg| match arg {
+            AngleBracketedArg::Constraint(c) => {
+                Some(self.lower_assoc_ty_constraint(c, itctx.reborrow()))
+            }
+            AngleBracketedArg::Arg(_) => None,
+        }));
+        let ctor = GenericArgsCtor { args, bindings, parenthesized: false };
+        (ctor, !has_non_lt_args && param_mode == ParamMode::Optional)
     }
 
     fn lower_parenthesized_parameter_data(

src/librustc_ast_passes/ast_validation.rs

@@ -639,6 +639,34 @@ impl<'a> AstValidator<'a> {
             .emit();
         }
     }
+
+    /// Enforce generic args coming before constraints in `<...>` of a path segment.
+    fn check_generic_args_before_constraints(&self, data: &AngleBracketedArgs) {
+        // Early exit in case it's partitioned as it should be.
+        if data.args.iter().is_partitioned(|arg| matches!(arg, AngleBracketedArg::Arg(_))) {
+            return;
+        }
+        // Find all generic argument coming after the first constraint...
+        let mut misplaced_args = Vec::new();
+        let mut first = None;
+        for arg in &data.args {
+            match (arg, first) {
+                (AngleBracketedArg::Arg(a), Some(_)) => misplaced_args.push(a.span()),
+                (AngleBracketedArg::Constraint(c), None) => first = Some(c.span),
+                (AngleBracketedArg::Arg(_), None) | (AngleBracketedArg::Constraint(_), Some(_)) => {
+                }
+            }
+        }
+        // ...and then error:
+        self.err_handler()
+            .struct_span_err(
+                misplaced_args.clone(),
+                "generic arguments must come before the first constraint",
+            )
+            .span_label(first.unwrap(), "the first constraint is provided here")
+            .span_labels(misplaced_args, "generic argument")
+            .emit();
+    }
 }
 
 /// Checks that generic parameters are in the correct order,
@@ -1008,17 +1036,20 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
     fn visit_generic_args(&mut self, _: Span, generic_args: &'a GenericArgs) {
         match *generic_args {
             GenericArgs::AngleBracketed(ref data) => {
-                walk_list!(self, visit_generic_arg, &data.args);
-
-                // Type bindings such as `Item = impl Debug` in `Iterator<Item = Debug>`
-                // are allowed to contain nested `impl Trait`.
-                self.with_impl_trait(None, |this| {
-                    walk_list!(
-                        this,
-                        visit_assoc_ty_constraint_from_generic_args,
-                        &data.constraints
-                    );
-                });
+                self.check_generic_args_before_constraints(data);
+
+                for arg in &data.args {
+                    match arg {
+                        AngleBracketedArg::Arg(arg) => self.visit_generic_arg(arg),
+                        // Type bindings such as `Item = impl Debug` in `Iterator<Item = Debug>`
+                        // are allowed to contain nested `impl Trait`.
+                        AngleBracketedArg::Constraint(constraint) => {
+                            self.with_impl_trait(None, |this| {
+                                this.visit_assoc_ty_constraint_from_generic_args(constraint);
+                            });
+                        }
+                    }
+                }
             }
             GenericArgs::Parenthesized(ref data) => {
                 walk_list!(self, visit_ty, &data.inputs);

src/librustc_ast_passes/lib.rs

@@ -1,10 +1,12 @@
-#![feature(bindings_after_at)]
 //! The `rustc_ast_passes` crate contains passes which validate the AST in `syntax`
 //! parsed by `rustc_parse` and then lowered, after the passes in this crate,
 //! by `rustc_ast_lowering`.
 //!
 //! The crate also contains other misc AST visitors, e.g. `node_count` and `show_span`.
 
+#![feature(bindings_after_at)]
+#![feature(iter_is_partitioned)]
+
 pub mod ast_validation;
 pub mod feature_gate;
 pub mod node_count;

src/librustc_ast_pretty/pprust.rs

@@ -796,31 +796,10 @@ impl<'a> PrintState<'a> for State<'a> {
         match *args {
             ast::GenericArgs::AngleBracketed(ref data) => {
                 self.s.word("<");
-
-                self.commasep(Inconsistent, &data.args, |s, generic_arg| {
-                    s.print_generic_arg(generic_arg)
+                self.commasep(Inconsistent, &data.args, |s, arg| match arg {
+                    ast::AngleBracketedArg::Arg(a) => s.print_generic_arg(a),
+                    ast::AngleBracketedArg::Constraint(c) => s.print_assoc_constraint(c),
                 });
-
-                let mut comma = !data.args.is_empty();
-
-                for constraint in data.constraints.iter() {
-                    if comma {
-                        self.word_space(",")
-                    }
-                    self.print_ident(constraint.ident);
-                    self.s.space();
-                    match constraint.kind {
-                        ast::AssocTyConstraintKind::Equality { ref ty } => {
-                            self.word_space("=");
-                            self.print_type(ty);
-                        }
-                        ast::AssocTyConstraintKind::Bound { ref bounds } => {
-                            self.print_type_bounds(":", &*bounds);
-                        }
-                    }
-                    comma = true;
-                }
-
                 self.s.word(">")
             }
 
@@ -891,6 +870,20 @@ impl<'a> State<'a> {
         }
     }
 
+    fn print_assoc_constraint(&mut self, constraint: &ast::AssocTyConstraint) {
+        self.print_ident(constraint.ident);
+        self.s.space();
+        match &constraint.kind {
+            ast::AssocTyConstraintKind::Equality { ty } => {
+                self.word_space("=");
+                self.print_type(ty);
+            }
+            ast::AssocTyConstraintKind::Bound { bounds } => {
+                self.print_type_bounds(":", &*bounds);
+            }
+        }
+    }
+
     crate fn print_generic_arg(&mut self, generic_arg: &GenericArg) {
         match generic_arg {
             GenericArg::Lifetime(lt) => self.print_lifetime(*lt),

src/librustc_expand/build.rs

@@ -36,7 +36,8 @@ impl<'a> ExtCtxt<'a> {
             idents.into_iter().map(|ident| ast::PathSegment::from_ident(ident.with_span_pos(span))),
         );
         let args = if !args.is_empty() {
-            ast::AngleBracketedArgs { args, constraints: Vec::new(), span }.into()
+            let args = args.into_iter().map(ast::AngleBracketedArg::Arg).collect();
+            ast::AngleBracketedArgs { args, span }.into()
         } else {
             None
         };

src/librustc_interface/util.rs

@@ -634,17 +634,19 @@ impl<'a, 'b> ReplaceBodyWithLoop<'a, 'b> {
                         match seg.args.as_ref().map(|generic_arg| &**generic_arg) {
                             None => false,
                             Some(&ast::GenericArgs::AngleBracketed(ref data)) => {
-                                let types = data.args.iter().filter_map(|arg| match arg {
-                                    ast::GenericArg::Type(ty) => Some(ty),
-                                    _ => None,
-                                });
-                                any_involves_impl_trait(types)
-                                    || data.constraints.iter().any(|c| match c.kind {
+                                data.args.iter().any(|arg| match arg {
+                                    ast::AngleBracketedArg::Arg(arg) => match arg {
+                                        ast::GenericArg::Type(ty) => involves_impl_trait(ty),
+                                        ast::GenericArg::Lifetime(_)
+                                        | ast::GenericArg::Const(_) => false,
+                                    },
+                                    ast::AngleBracketedArg::Constraint(c) => match c.kind {
                                         ast::AssocTyConstraintKind::Bound { .. } => true,
                                         ast::AssocTyConstraintKind::Equality { ref ty } => {
                                             involves_impl_trait(ty)
                                         }
-                                    })
+                                    },
+                                })
                             }
                             Some(&ast::GenericArgs::Parenthesized(ref data)) => {
                                 any_involves_impl_trait(data.inputs.iter())