parse: refactor fun ret ty & param ty

Centril · Centril · commit 34d91709b672 · 2019-12-12T17:54:49.000+01:00
diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
@@ -2092,29 +2092,19 @@ impl<'a> LoweringContext<'a> {
                     .iter()
                     .map(|ty| this.lower_ty_direct(ty, ImplTraitContext::disallowed()))
                     .collect();
+                let output_ty = match output {
+                    FunctionRetTy::Ty(ty) => this.lower_ty(&ty, ImplTraitContext::disallowed()),
+                    FunctionRetTy::Default(_) => P(this.ty_tup(span, hir::HirVec::new())),
+                };
+                let args = hir_vec![GenericArg::Type(this.ty_tup(span, inputs))];
+                let binding = hir::TypeBinding {
+                    hir_id: this.next_id(),
+                    ident: Ident::with_dummy_span(FN_OUTPUT_NAME),
+                    span: output_ty.span,
+                    kind: hir::TypeBindingKind::Equality { ty: output_ty },
+                };
                 (
-                    hir::GenericArgs {
-                        args: hir_vec![GenericArg::Type(this.ty_tup(span, inputs))],
-                        bindings: hir_vec![
-                            hir::TypeBinding {
-                                hir_id: this.next_id(),
-                                ident: Ident::with_dummy_span(FN_OUTPUT_NAME),
-                                kind: hir::TypeBindingKind::Equality {
-                                    ty: output
-                                        .as_ref()
-                                        .map(|ty| this.lower_ty(
-                                            &ty,
-                                            ImplTraitContext::disallowed()
-                                        ))
-                                        .unwrap_or_else(||
-                                            P(this.ty_tup(span, hir::HirVec::new()))
-                                        ),
-                                },
-                                span: output.as_ref().map_or(span, |ty| ty.span),
-                            }
-                        ],
-                        parenthesized: true,
-                    },
+                    hir::GenericArgs { args, bindings: hir_vec![binding], parenthesized: true },
                     false,
                 )
             }
diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs
@@ -712,8 +712,8 @@ impl<'a, 'b> ReplaceBodyWithLoop<'a, 'b> {
         ret
     }
 
-    fn should_ignore_fn(ret_ty: &ast::FnDecl) -> bool {
-        if let ast::FunctionRetTy::Ty(ref ty) = ret_ty.output {
+    fn should_ignore_fn(ret_ty: &ast::FunctionRetTy) -> bool {
+        if let ast::FunctionRetTy::Ty(ref ty) = ret_ty {
             fn involves_impl_trait(ty: &ast::Ty) -> bool {
                 match ty.kind {
                     ast::TyKind::ImplTrait(..) => true,
@@ -742,7 +742,7 @@ impl<'a, 'b> ReplaceBodyWithLoop<'a, 'b> {
                             },
                             Some(&ast::GenericArgs::Parenthesized(ref data)) => {
                                 any_involves_impl_trait(data.inputs.iter()) ||
-                                any_involves_impl_trait(data.output.iter())
+                                ReplaceBodyWithLoop::should_ignore_fn(&data.output)
                             }
                         }
                     }),
@@ -762,7 +762,7 @@ impl<'a, 'b> ReplaceBodyWithLoop<'a, 'b> {
 
     fn is_sig_const(sig: &ast::FnSig) -> bool {
         sig.header.constness.node == ast::Constness::Const ||
-            ReplaceBodyWithLoop::should_ignore_fn(&sig.decl)
+            ReplaceBodyWithLoop::should_ignore_fn(&sig.decl.output)
     }
 }
 
diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs
@@ -1381,7 +1381,7 @@ impl<'a> Parser<'a> {
                 args
             }
         };
-        let output = self.parse_ret_ty(true)?;
+        let output = self.parse_ret_ty(true, true)?;
 
         Ok(P(FnDecl {
             inputs: inputs_captures,
diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs
@@ -1900,7 +1900,7 @@ impl<'a> Parser<'a> {
     ) -> PResult<'a, P<FnDecl>> {
         Ok(P(FnDecl {
             inputs: self.parse_fn_params(cfg)?,
-            output: self.parse_ret_ty(ret_allow_plus)?,
+            output: self.parse_ret_ty(ret_allow_plus, true)?,
         }))
     }
 
@@ -2002,12 +2002,12 @@ impl<'a> Parser<'a> {
             }
 
             self.eat_incorrect_doc_comment_for_param_type();
-            (pat, self.parse_ty_common(true, true, cfg.allow_c_variadic)?)
+            (pat, self.parse_ty_for_param(cfg.allow_c_variadic)?)
         } else {
             debug!("parse_param_general ident_to_pat");
             let parser_snapshot_before_ty = self.clone();
             self.eat_incorrect_doc_comment_for_param_type();
-            let mut ty = self.parse_ty_common(true, true, cfg.allow_c_variadic);
+            let mut ty = self.parse_ty_for_param(cfg.allow_c_variadic);
             if ty.is_ok() && self.token != token::Comma &&
                self.token != token::CloseDelim(token::Paren) {
                 // This wasn't actually a type, but a pattern looking like a type,
diff --git a/src/librustc_parse/parser/path.rs b/src/librustc_parse/parser/path.rs
@@ -182,11 +182,7 @@ impl<'a> Parser<'a> {
                 // `(T, U) -> R`
                 let (inputs, _) = self.parse_paren_comma_seq(|p| p.parse_ty())?;
                 let span = ident.span.to(self.prev_span);
-                let output = if self.eat(&token::RArrow) {
-                    Some(self.parse_ty_common(false, false, false)?)
-                } else {
-                    None
-                };
+                let output = self.parse_ret_ty(false, false)?;
                 ParenthesizedArgs { inputs, output, span }.into()
             };
 
diff --git a/src/librustc_parse/parser/ty.rs b/src/librustc_parse/parser/ty.rs
@@ -30,6 +30,13 @@ impl<'a> Parser<'a> {
         self.parse_ty_common(true, true, false)
     }
 
+    /// Parse a type suitable for a function or function pointer parameter.
+    /// The difference from `parse_ty` is that this version allows `...`
+    /// (`CVarArgs`) at the top level of the the type.
+    pub(super) fn parse_ty_for_param(&mut self, allow_c_variadic: bool) -> PResult<'a, P<Ty>> {
+        self.parse_ty_common(true, true, allow_c_variadic)
+    }
+
     /// Parses a type in restricted contexts where `+` is not permitted.
     ///
     /// Example 1: `&'a TYPE`
@@ -41,17 +48,26 @@ impl<'a> Parser<'a> {
     }
 
     /// Parses an optional return type `[ -> TY ]` in a function declaration.
-    pub(super) fn parse_ret_ty(&mut self, allow_plus: bool) -> PResult<'a, FunctionRetTy> {
+    pub(super) fn parse_ret_ty(
+        &mut self,
+        allow_plus: bool,
+        allow_qpath_recovery: bool,
+    ) -> PResult<'a, FunctionRetTy> {
         Ok(if self.eat(&token::RArrow) {
             // FIXME(Centril): Can we unconditionally `allow_plus`?
-            FunctionRetTy::Ty(self.parse_ty_common(allow_plus, true, false)?)
+            FunctionRetTy::Ty(self.parse_ty_common(allow_plus, allow_qpath_recovery, false)?)
         } else {
             FunctionRetTy::Default(self.token.span.shrink_to_lo())
         })
     }
 
-    pub(super) fn parse_ty_common(&mut self, allow_plus: bool, allow_qpath_recovery: bool,
-                       allow_c_variadic: bool) -> PResult<'a, P<Ty>> {
+    fn parse_ty_common(
+        &mut self,
+        allow_plus: bool,
+        allow_qpath_recovery: bool,
+        // Is `...` (`CVarArgs`) legal in the immediate top level call?
+        allow_c_variadic: bool,
+    ) -> PResult<'a, P<Ty>> {
         maybe_recover_from_interpolated_ty_qpath!(self, allow_qpath_recovery);
         maybe_whole!(self, NtTy, |x| x);
 
@@ -198,6 +214,8 @@ impl<'a> Parser<'a> {
                 self.eat(&token::DotDotDot);
                 TyKind::CVarArgs
             } else {
+                // FIXME(Centril): Should we just allow `...` syntactically
+                // anywhere in a type and use semantic restrictions instead?
                 return Err(struct_span_fatal!(
                     self.sess.span_diagnostic,
                     self.token.span,
diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs
@@ -674,10 +674,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
             }
             GenericArgs::Parenthesized(ref data) => {
                 walk_list!(self, visit_ty, &data.inputs);
-                if let Some(ref type_) = data.output {
+                if let FunctionRetTy::Ty(ty) = &data.output {
                     // `-> Foo` syntax is essentially an associated type binding,
                     // so it is also allowed to contain nested `impl Trait`.
-                    self.with_impl_trait(None, |this| this.visit_ty(type_));
+                    self.with_impl_trait(None, |this| this.visit_ty(ty));
                 }
             }
         }
diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs
@@ -811,18 +811,17 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> {
                 match **generic_args {
                     ast::GenericArgs::AngleBracketed(ref data) => {
                         for arg in &data.args {
-                            match arg {
-                                ast::GenericArg::Type(ty) => self.visit_ty(ty),
-                                _ => {}
+                            if let ast::GenericArg::Type(ty) = arg {
+                                self.visit_ty(ty);
                             }
                         }
                     }
                     ast::GenericArgs::Parenthesized(ref data) => {
                         for t in &data.inputs {
                             self.visit_ty(t);
                         }
-                        if let Some(ref t) = data.output {
-                            self.visit_ty(t);
+                        if let ast::FunctionRetTy::Ty(ty) = &data.output {
+                            self.visit_ty(ty);
                         }
                     }
                 }
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
@@ -254,7 +254,7 @@ pub struct ParenthesizedArgs {
     pub inputs: Vec<P<Ty>>,
 
     /// `C`
-    pub output: Option<P<Ty>>,
+    pub output: FunctionRetTy,
 }
 
 impl ParenthesizedArgs {
@@ -2185,7 +2185,7 @@ impl fmt::Debug for ImplPolarity {
 }
 
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
-pub enum FunctionRetTy {
+pub enum FunctionRetTy { // FIXME(Centril): Rename to `FnRetTy` and in HIR also.
     /// Returns type is not specified.
     ///
     /// Functions default to `()` and closures default to inference.
diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs
@@ -553,7 +553,7 @@ pub fn noop_visit_parenthesized_parameter_data<T: MutVisitor>(args: &mut Parenth
                                                               vis: &mut T) {
     let ParenthesizedArgs { inputs, output, span } = args;
     visit_vec(inputs, |input| vis.visit_ty(input));
-    visit_opt(output, |output| vis.visit_ty(output));
+    noop_visit_fn_ret_ty(output, vis);
     vis.visit_span(span);
 }
 
@@ -742,7 +742,11 @@ pub fn noop_visit_asyncness<T: MutVisitor>(asyncness: &mut IsAsync, vis: &mut T)
 pub fn noop_visit_fn_decl<T: MutVisitor>(decl: &mut P<FnDecl>, vis: &mut T) {
     let FnDecl { inputs, output } = decl.deref_mut();
     inputs.flat_map_in_place(|param| vis.flat_map_param(param));
-    match output {
+    noop_visit_fn_ret_ty(output, vis);
+}
+
+pub fn noop_visit_fn_ret_ty<T: MutVisitor>(fn_ret_ty: &mut FunctionRetTy, vis: &mut T) {
+    match fn_ret_ty {
         FunctionRetTy::Default(span) => vis.visit_span(span),
         FunctionRetTy::Ty(ty) => vis.visit_ty(ty),
     }
diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs
@@ -887,17 +887,9 @@ impl<'a> PrintState<'a> for State<'a> {
 
             ast::GenericArgs::Parenthesized(ref data) => {
                 self.s.word("(");
-                self.commasep(
-                    Inconsistent,
-                    &data.inputs,
-                    |s, ty| s.print_type(ty));
+                self.commasep(Inconsistent, &data.inputs, |s, ty| s.print_type(ty));
                 self.s.word(")");
-
-                if let Some(ref ty) = data.output {
-                    self.space_if_not_bol();
-                    self.word_space("->");
-                    self.print_type(ty);
-                }
+                self.print_fn_ret_ty(&data.output);
             }
         }
     }
@@ -1579,6 +1571,7 @@ impl<'a> State<'a> {
         self.ann.post(self, AnnNode::SubItem(ti.id))
     }
 
+    // FIXME(Centril): merge with function above.
     crate fn print_impl_item(&mut self, ii: &ast::ImplItem) {
         self.ann.pre(self, AnnNode::SubItem(ii.id));
         self.hardbreak_if_not_bol();
@@ -2104,7 +2097,7 @@ impl<'a> State<'a> {
                 self.print_asyncness(asyncness);
                 self.print_capture_clause(capture_clause);
 
-                self.print_fn_block_params(decl);
+                self.print_fn_params_and_ret(decl, true);
                 self.s.space();
                 self.print_expr(body);
                 self.end(); // need to close a box
@@ -2535,36 +2528,16 @@ impl<'a> State<'a> {
             self.print_ident(name);
         }
         self.print_generic_params(&generics.params);
-        self.print_fn_params_and_ret(decl);
+        self.print_fn_params_and_ret(decl, false);
         self.print_where_clause(&generics.where_clause)
     }
 
-    crate fn print_fn_params_and_ret(&mut self, decl: &ast::FnDecl) {
-        self.popen();
-        self.commasep(Inconsistent, &decl.inputs, |s, param| s.print_param(param, false));
-        self.pclose();
-
-        self.print_fn_output(decl)
-    }
-
-    crate fn print_fn_block_params(&mut self, decl: &ast::FnDecl) {
-        self.s.word("|");
-        self.commasep(Inconsistent, &decl.inputs, |s, param| s.print_param(param, true));
-        self.s.word("|");
-
-        if let ast::FunctionRetTy::Default(..) = decl.output {
-            return;
-        }
-
-        self.space_if_not_bol();
-        self.word_space("->");
-        match decl.output {
-            ast::FunctionRetTy::Ty(ref ty) => {
-                self.print_type(ty);
-                self.maybe_print_comment(ty.span.lo())
-            }
-            ast::FunctionRetTy::Default(..) => unreachable!(),
-        }
+    crate fn print_fn_params_and_ret(&mut self, decl: &ast::FnDecl, is_closure: bool) {
+        let (open, close) = if is_closure { ("|", "|") } else { ("(", ")") };
+        self.word(open);
+        self.commasep(Inconsistent, &decl.inputs, |s, param| s.print_param(param, is_closure));
+        self.word(close);
+        self.print_fn_ret_ty(&decl.output)
     }
 
     crate fn print_movability(&mut self, movability: ast::Movability) {
@@ -2786,24 +2759,14 @@ impl<'a> State<'a> {
         self.end();
     }
 
-    crate fn print_fn_output(&mut self, decl: &ast::FnDecl) {
-        if let ast::FunctionRetTy::Default(..) = decl.output {
-            return;
-        }
-
-        self.space_if_not_bol();
-        self.ibox(INDENT_UNIT);
-        self.word_space("->");
-        match decl.output {
-            ast::FunctionRetTy::Default(..) => unreachable!(),
-            ast::FunctionRetTy::Ty(ref ty) =>
-                self.print_type(ty),
-        }
-        self.end();
-
-        match decl.output {
-            ast::FunctionRetTy::Ty(ref output) => self.maybe_print_comment(output.span.lo()),
-            _ => {}
+    crate fn print_fn_ret_ty(&mut self, fn_ret_ty: &ast::FunctionRetTy) {
+        if let ast::FunctionRetTy::Ty(ty) = fn_ret_ty {
+            self.space_if_not_bol();
+            self.ibox(INDENT_UNIT);
+            self.word_space("->");
+            self.print_type(ty);
+            self.end();
+            self.maybe_print_comment(ty.span.lo());
         }
     }
 
diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs
@@ -420,7 +420,7 @@ pub fn walk_generic_args<'a, V>(visitor: &mut V,
         }
         GenericArgs::Parenthesized(ref data) => {
             walk_list!(visitor, visit_ty, &data.inputs);
-            walk_list!(visitor, visit_ty, &data.output);
+            walk_fn_ret_ty(visitor, &data.output);
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -712,8 +712,8 @@ impl<'a, 'b> ReplaceBodyWithLoop<'a, 'b> {`
`712`	`712`	`ret`
`713`	`713`	`}`
`714`	`714`
`715`		`- fn should_ignore_fn(ret_ty: &ast::FnDecl) -> bool {`
`716`		`- if let ast::FunctionRetTy::Ty(ref ty) = ret_ty.output {`
	`715`	`+ fn should_ignore_fn(ret_ty: &ast::FunctionRetTy) -> bool {`
	`716`	`+ if let ast::FunctionRetTy::Ty(ref ty) = ret_ty {`
`717`	`717`	`fn involves_impl_trait(ty: &ast::Ty) -> bool {`
`718`	`718`	`match ty.kind {`
`719`	`719`	`ast::TyKind::ImplTrait(..) => true,`
`@@ -742,7 +742,7 @@ impl<'a, 'b> ReplaceBodyWithLoop<'a, 'b> {`
`742`	`742`	`},`
`743`	`743`	`Some(&ast::GenericArgs::Parenthesized(ref data)) => {`
`744`	`744`	`any_involves_impl_trait(data.inputs.iter()) \|\|`
`745`		`- any_involves_impl_trait(data.output.iter())`
	`745`	`+ ReplaceBodyWithLoop::should_ignore_fn(&data.output)`
`746`	`746`	`}`
`747`	`747`	`}`
`748`	`748`	`}),`
`@@ -762,7 +762,7 @@ impl<'a, 'b> ReplaceBodyWithLoop<'a, 'b> {`
`762`	`762`
`763`	`763`	`fn is_sig_const(sig: &ast::FnSig) -> bool {`
`764`	`764`	`sig.header.constness.node == ast::Constness::Const \|\|`
`765`		`- ReplaceBodyWithLoop::should_ignore_fn(&sig.decl)`
	`765`	`+ ReplaceBodyWithLoop::should_ignore_fn(&sig.decl.output)`
`766`	`766`	`}`
`767`	`767`	`}`
`768`	`768`
Original file line number	Diff line number	Diff line change
`@@ -1381,7 +1381,7 @@ impl<'a> Parser<'a> {`
`1381`	`1381`	`args`
`1382`	`1382`	`}`
`1383`	`1383`	`};`
`1384`		`- let output = self.parse_ret_ty(true)?;`
	`1384`	`+ let output = self.parse_ret_ty(true, true)?;`
`1385`	`1385`
`1386`	`1386`	`Ok(P(FnDecl {`
`1387`	`1387`	`inputs: inputs_captures,`
Original file line number	Diff line number	Diff line change
`@@ -674,10 +674,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {`
`674`	`674`	`}`
`675`	`675`	`GenericArgs::Parenthesized(ref data) => {`
`676`	`676`	`walk_list!(self, visit_ty, &data.inputs);`
`677`		`- if let Some(ref type_) = data.output {`
	`677`	`+ if let FunctionRetTy::Ty(ty) = &data.output {`
`678`	`678`	// `-> Foo` syntax is essentially an associated type binding,
`679`	`679`	// so it is also allowed to contain nested `impl Trait`.
`680`		`- self.with_impl_trait(None, \|this\| this.visit_ty(type_));`
	`680`	`+ self.with_impl_trait(None, \|this\| this.visit_ty(ty));`
`681`	`681`	`}`
`682`	`682`	`}`
`683`	`683`	`}`
Original file line number	Diff line number	Diff line change
`@@ -811,18 +811,17 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> {`
`811`	`811`	`match **generic_args {`
`812`	`812`	`ast::GenericArgs::AngleBracketed(ref data) => {`
`813`	`813`	`for arg in &data.args {`
`814`		`- match arg {`
`815`		`- ast::GenericArg::Type(ty) => self.visit_ty(ty),`
`816`		`- _ => {}`
	`814`	`+ if let ast::GenericArg::Type(ty) = arg {`
	`815`	`+ self.visit_ty(ty);`
`817`	`816`	`}`
`818`	`817`	`}`
`819`	`818`	`}`
`820`	`819`	`ast::GenericArgs::Parenthesized(ref data) => {`
`821`	`820`	`for t in &data.inputs {`
`822`	`821`	`self.visit_ty(t);`
`823`	`822`	`}`
`824`		`- if let Some(ref t) = data.output {`
`825`		`- self.visit_ty(t);`
	`823`	`+ if let ast::FunctionRetTy::Ty(ty) = &data.output {`
	`824`	`+ self.visit_ty(ty);`
`826`	`825`	`}`
`827`	`826`	`}`
`828`	`827`	`}`
Original file line number	Diff line number	Diff line change
`@@ -254,7 +254,7 @@ pub struct ParenthesizedArgs {`
`254`	`254`	`pub inputs: Vec<P<Ty>>,`
`255`	`255`
`256`	`256`	/// `C`
`257`		`- pub output: Option<P<Ty>>,`
	`257`	`+ pub output: FunctionRetTy,`
`258`	`258`	`}`
`259`	`259`
`260`	`260`	`impl ParenthesizedArgs {`
`@@ -2185,7 +2185,7 @@ impl fmt::Debug for ImplPolarity {`
`2185`	`2185`	`}`
`2186`	`2186`
`2187`	`2187`	`#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]`
`2188`		`-pub enum FunctionRetTy {`
	`2188`	+pub enum FunctionRetTy { // FIXME(Centril): Rename to `FnRetTy` and in HIR also.
`2189`	`2189`	`/// Returns type is not specified.`
`2190`	`2190`	`///`
`2191`	`2191`	/// Functions default to `()` and closures default to inference.