From 90a6954327bb4f018eab155f43299d4bf67abe41 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 4 Aug 2018 02:23:21 +0100 Subject: [PATCH 1/7] Emit error for pattern arguments in trait methods The error and check for this already existed, but the parser didn't try to parse trait method arguments as patterns, so the error was never emitted. This surfaces the error, so we get better errors than simple parse errors. --- src/librustc_passes/diagnostics.rs | 14 +++++++- src/libsyntax/lib.rs | 1 + src/libsyntax/parse/parser.rs | 53 +++++++++++++++++++----------- src/test/ui/E0642.rs | 15 +++++++++ src/test/ui/E0642.stderr | 9 +++++ 5 files changed, 71 insertions(+), 21 deletions(-) create mode 100644 src/test/ui/E0642.rs create mode 100644 src/test/ui/E0642.stderr diff --git a/src/librustc_passes/diagnostics.rs b/src/librustc_passes/diagnostics.rs index f1ec3371c3b9a..b78f2ca676d2f 100644 --- a/src/librustc_passes/diagnostics.rs +++ b/src/librustc_passes/diagnostics.rs @@ -261,6 +261,19 @@ let result = loop { // ok! ``` "##, +E0642: r##" +Trait methods currently cannot take patterns as arguments. + +Example of erroneous code: + +```compile_fail,E0642 +trait Foo { + fn foo((x, y): (i32, i32)); // error: patterns aren't allowed + // in methods without bodies +} +``` +"##, + E0695: r##" A `break` statement without a label appeared inside a labeled block. @@ -306,7 +319,6 @@ register_diagnostics! { E0561, // patterns aren't allowed in function pointer types E0567, // auto traits can not have generic parameters E0568, // auto traits can not have super traits - E0642, // patterns aren't allowed in methods without bodies E0666, // nested `impl Trait` is illegal E0667, // `impl Trait` in projections E0696, // `continue` pointing to a labeled block diff --git a/src/libsyntax/lib.rs b/src/libsyntax/lib.rs index c8e60620248b3..76035a73d1a15 100644 --- a/src/libsyntax/lib.rs +++ b/src/libsyntax/lib.rs @@ -26,6 +26,7 @@ #![feature(slice_sort_by_cached_key)] #![feature(str_escape)] #![feature(unicode_internals)] +#![feature(catch_expr)] #![recursion_limit="256"] diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 9011b6e48b974..5a2fd5f0145fc 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1371,7 +1371,7 @@ impl<'a> Parser<'a> { let ident = self.parse_ident()?; let mut generics = self.parse_generics()?; - let d = self.parse_fn_decl_with_self(|p: &mut Parser<'a>|{ + let d = self.parse_fn_decl_with_self(|p: &mut Parser<'a>| { // This is somewhat dubious; We don't want to allow // argument names to be left off if there is a // definition... @@ -1744,30 +1744,43 @@ impl<'a> Parser<'a> { fn parse_arg_general(&mut self, require_name: bool) -> PResult<'a, Arg> { maybe_whole!(self, NtArg, |x| x); - let (pat, ty) = if require_name || self.is_named_argument() { - debug!("parse_arg_general parse_pat (require_name:{})", - require_name); - let pat = self.parse_pat()?; + let parser_snapshot_before_pat = self.clone(); + // We're going to try parsing the argument as a pattern (even if it's not + // allowed, such as for trait methods without bodies). This way we can provide + // better errors to the user. + let pat_arg: PResult<'a, (P, P)> = do catch { + let pat = self.parse_pat()?; self.expect(&token::Colon)?; (pat, self.parse_ty()?) - } else { - debug!("parse_arg_general ident_to_pat"); - let ident = Ident::new(keywords::Invalid.name(), self.prev_span); - let ty = self.parse_ty()?; - let pat = P(Pat { - id: ast::DUMMY_NODE_ID, - node: PatKind::Ident(BindingMode::ByValue(Mutability::Immutable), ident, None), - span: ty.span, - }); - (pat, ty) }; - Ok(Arg { - ty, - pat, - id: ast::DUMMY_NODE_ID, - }) + let is_named_argument = self.is_named_argument(); + match pat_arg { + Ok((pat, ty)) => { + Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID }) + } + Err(mut err) => { + if require_name || is_named_argument { + Err(err) + } else { + err.cancel(); + // Recover from attempting to parse the argument as a pattern. This means + // the type is alone, with no name, e.g. `fn foo(u32)`. + mem::replace(self, parser_snapshot_before_pat); + debug!("parse_arg_general ident_to_pat"); + let ident = Ident::new(keywords::Invalid.name(), self.prev_span); + let ty = self.parse_ty()?; + let pat = P(Pat { + id: ast::DUMMY_NODE_ID, + node: PatKind::Ident( + BindingMode::ByValue(Mutability::Immutable), ident, None), + span: ty.span, + }); + Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID }) + } + } + } } /// Parse a single function argument diff --git a/src/test/ui/E0642.rs b/src/test/ui/E0642.rs new file mode 100644 index 0000000000000..a09846cb3a1e8 --- /dev/null +++ b/src/test/ui/E0642.rs @@ -0,0 +1,15 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +trait Foo { + fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in methods without bodies +} + +fn main() {} diff --git a/src/test/ui/E0642.stderr b/src/test/ui/E0642.stderr new file mode 100644 index 0000000000000..edc430d578ba9 --- /dev/null +++ b/src/test/ui/E0642.stderr @@ -0,0 +1,9 @@ +error[E0642]: patterns aren't allowed in methods without bodies + --> $DIR/E0642.rs:12:12 + | +LL | fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in methods without bodies + | ^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0642`. From b05f0bec1a6f576cd275e52a8a0a0165fb25f77a Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 4 Aug 2018 11:48:33 +0100 Subject: [PATCH 2/7] Suggest replacing patterns with underscores --- src/librustc_passes/ast_validation.rs | 7 +++++-- src/test/ui/E0642.stderr | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 0ea90e7453190..c75ae07fe6618 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -344,8 +344,11 @@ impl<'a> Visitor<'a> for AstValidator<'a> { trait_item.id, span, "patterns aren't allowed in methods without bodies"); } else { - struct_span_err!(self.session, span, E0642, - "patterns aren't allowed in methods without bodies").emit(); + let mut err = struct_span_err!(self.session, span, E0642, + "patterns aren't allowed in methods without bodies"); + err.span_suggestion(span, + "use an underscore to ignore the name", "_".to_owned()); + err.emit(); } }); } diff --git a/src/test/ui/E0642.stderr b/src/test/ui/E0642.stderr index edc430d578ba9..5291c016c7faf 100644 --- a/src/test/ui/E0642.stderr +++ b/src/test/ui/E0642.stderr @@ -2,7 +2,7 @@ error[E0642]: patterns aren't allowed in methods without bodies --> $DIR/E0642.rs:12:12 | LL | fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in methods without bodies - | ^^^^^^ + | ^^^^^^ help: use an underscore to ignore the name: `_` error: aborting due to previous error From 235905c080bf953a522ff86d4fec6134ac4fb371 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 6 Aug 2018 18:14:57 +0100 Subject: [PATCH 3/7] Fix handling of trait methods with bodies and improve efficiency --- src/librustc_passes/ast_validation.rs | 26 +++++----- src/libsyntax/parse/parser.rs | 49 ++++++++++++------- .../compile-fail/no-patterns-in-args-2.rs | 4 +- .../compile-fail/no-patterns-in-args-macro.rs | 2 +- src/test/ui/E0642.rs | 6 ++- src/test/ui/E0642.stderr | 22 +++++++-- 6 files changed, 70 insertions(+), 39 deletions(-) diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index c75ae07fe6618..7022136f23984 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -336,22 +336,24 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if let TraitItemKind::Method(ref sig, ref block) = trait_item.node { self.check_trait_fn_not_async(trait_item.span, sig.header.asyncness); self.check_trait_fn_not_const(sig.header.constness); - if block.is_none() { - self.check_decl_no_pat(&sig.decl, |span, mut_ident| { - if mut_ident { + self.check_decl_no_pat(&sig.decl, |span, mut_ident| { + if mut_ident { + if block.is_none() { self.session.buffer_lint( lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY, trait_item.id, span, - "patterns aren't allowed in methods without bodies"); - } else { - let mut err = struct_span_err!(self.session, span, E0642, - "patterns aren't allowed in methods without bodies"); - err.span_suggestion(span, - "use an underscore to ignore the name", "_".to_owned()); - err.emit(); + "patterns aren't allowed in trait methods"); } - }); - } + } else { + let mut err = struct_span_err!(self.session, span, E0642, + "patterns aren't allowed in trait methods"); + let suggestion = "give this argument a name or use an \ + underscore to ignore it, instead of a \ + tuple pattern"; + err.span_suggestion(span, suggestion, "_".to_owned()); + err.emit(); + } + }); } } } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 5a2fd5f0145fc..57eb1f52fb703 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1744,7 +1744,16 @@ impl<'a> Parser<'a> { fn parse_arg_general(&mut self, require_name: bool) -> PResult<'a, Arg> { maybe_whole!(self, NtArg, |x| x); - let parser_snapshot_before_pat = self.clone(); + // If we see `ident :`, then we know that the argument is just of the + // form `type`, which means we won't need to recover from parsing a + // pattern and so we don't need to store a parser snapshot. + let parser_snapshot_before_pat = if + self.look_ahead(1, |t| t.is_ident()) && + self.look_ahead(2, |t| t == &token::Colon) { + None + } else { + Some(self.clone()) + }; // We're going to try parsing the argument as a pattern (even if it's not // allowed, such as for trait methods without bodies). This way we can provide @@ -1755,29 +1764,31 @@ impl<'a> Parser<'a> { (pat, self.parse_ty()?) }; - let is_named_argument = self.is_named_argument(); match pat_arg { Ok((pat, ty)) => { Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID }) } Err(mut err) => { - if require_name || is_named_argument { - Err(err) - } else { - err.cancel(); - // Recover from attempting to parse the argument as a pattern. This means - // the type is alone, with no name, e.g. `fn foo(u32)`. - mem::replace(self, parser_snapshot_before_pat); - debug!("parse_arg_general ident_to_pat"); - let ident = Ident::new(keywords::Invalid.name(), self.prev_span); - let ty = self.parse_ty()?; - let pat = P(Pat { - id: ast::DUMMY_NODE_ID, - node: PatKind::Ident( - BindingMode::ByValue(Mutability::Immutable), ident, None), - span: ty.span, - }); - Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID }) + match (require_name || self.is_named_argument(), parser_snapshot_before_pat) { + (true, _) | (_, None) => { + Err(err) + } + (false, Some(parser_snapshot_before_pat)) => { + err.cancel(); + // Recover from attempting to parse the argument as a pattern. This means + // the type is alone, with no name, e.g. `fn foo(u32)`. + mem::replace(self, parser_snapshot_before_pat); + debug!("parse_arg_general ident_to_pat"); + let ident = Ident::new(keywords::Invalid.name(), self.prev_span); + let ty = self.parse_ty()?; + let pat = P(Pat { + id: ast::DUMMY_NODE_ID, + node: PatKind::Ident( + BindingMode::ByValue(Mutability::Immutable), ident, None), + span: ty.span, + }); + Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID }) + } } } } diff --git a/src/test/compile-fail/no-patterns-in-args-2.rs b/src/test/compile-fail/no-patterns-in-args-2.rs index 4d2412c34a5fa..80e6967980119 100644 --- a/src/test/compile-fail/no-patterns-in-args-2.rs +++ b/src/test/compile-fail/no-patterns-in-args-2.rs @@ -11,9 +11,9 @@ #![deny(patterns_in_fns_without_body)] trait Tr { - fn f1(mut arg: u8); //~ ERROR patterns aren't allowed in methods without bodies + fn f1(mut arg: u8); //~ ERROR patterns aren't allowed in trait methods //~^ WARN was previously accepted - fn f2(&arg: u8); //~ ERROR patterns aren't allowed in methods without bodies + fn f2(&arg: u8); //~ ERROR patterns aren't allowed in trait methods fn g1(arg: u8); // OK fn g2(_: u8); // OK #[allow(anonymous_parameters)] diff --git a/src/test/compile-fail/no-patterns-in-args-macro.rs b/src/test/compile-fail/no-patterns-in-args-macro.rs index f85ce8f57ea71..546c40ecbd045 100644 --- a/src/test/compile-fail/no-patterns-in-args-macro.rs +++ b/src/test/compile-fail/no-patterns-in-args-macro.rs @@ -30,7 +30,7 @@ mod bad_pat { m!((bad, pat)); //~^ ERROR patterns aren't allowed in function pointer types //~| ERROR patterns aren't allowed in foreign function declarations - //~| ERROR patterns aren't allowed in methods without bodies + //~| ERROR patterns aren't allowed in trait methods } fn main() {} diff --git a/src/test/ui/E0642.rs b/src/test/ui/E0642.rs index a09846cb3a1e8..837a9365271e2 100644 --- a/src/test/ui/E0642.rs +++ b/src/test/ui/E0642.rs @@ -9,7 +9,11 @@ // except according to those terms. trait Foo { - fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in methods without bodies + fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in trait methods +} + +trait Bar { + fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods } fn main() {} diff --git a/src/test/ui/E0642.stderr b/src/test/ui/E0642.stderr index 5291c016c7faf..07ec8b4cc2c8f 100644 --- a/src/test/ui/E0642.stderr +++ b/src/test/ui/E0642.stderr @@ -1,9 +1,23 @@ -error[E0642]: patterns aren't allowed in methods without bodies +error[E0642]: patterns aren't allowed in trait methods --> $DIR/E0642.rs:12:12 | -LL | fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in methods without bodies - | ^^^^^^ help: use an underscore to ignore the name: `_` +LL | fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in trait methods + | ^^^^^^ +help: give this argument a name or use an underscore to ignore it, instead of a tuple pattern + | +LL | fn foo(_: (i32, i32)); //~ ERROR patterns aren't allowed in trait methods + | ^ + +error[E0642]: patterns aren't allowed in trait methods + --> $DIR/E0642.rs:16:12 + | +LL | fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods + | ^^^^^^ +help: give this argument a name or use an underscore to ignore it, instead of a tuple pattern + | +LL | fn bar(_: (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods + | ^ -error: aborting due to previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0642`. From a478cd41e3f203ec531bfce7efb8fc602aad5c7d Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 7 Aug 2018 00:03:26 +0100 Subject: [PATCH 4/7] Improve diagnostics --- src/librustc_passes/ast_validation.rs | 2 +- src/librustc_passes/diagnostics.rs | 10 +++++++++- src/libsyntax/parse/parser.rs | 2 +- src/test/ui/E0642.stderr | 4 ++-- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 7022136f23984..2195331f465ba 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -348,7 +348,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { let mut err = struct_span_err!(self.session, span, E0642, "patterns aren't allowed in trait methods"); let suggestion = "give this argument a name or use an \ - underscore to ignore it, instead of a \ + underscore to ignore it instead of using a \ tuple pattern"; err.span_suggestion(span, suggestion, "_".to_owned()); err.emit(); diff --git a/src/librustc_passes/diagnostics.rs b/src/librustc_passes/diagnostics.rs index b78f2ca676d2f..f1d0a4fee341e 100644 --- a/src/librustc_passes/diagnostics.rs +++ b/src/librustc_passes/diagnostics.rs @@ -269,7 +269,15 @@ Example of erroneous code: ```compile_fail,E0642 trait Foo { fn foo((x, y): (i32, i32)); // error: patterns aren't allowed - // in methods without bodies + // in trait methods +} +``` + +You can instead use a single name for the argument: + +``` +trait Foo { + fn foo(x_and_y: (i32, i32)); // ok! } ``` "##, diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 57eb1f52fb703..a1dbe93fdfe35 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1744,7 +1744,7 @@ impl<'a> Parser<'a> { fn parse_arg_general(&mut self, require_name: bool) -> PResult<'a, Arg> { maybe_whole!(self, NtArg, |x| x); - // If we see `ident :`, then we know that the argument is just of the + // If we see `ident :`, then we know that the argument is not just of the // form `type`, which means we won't need to recover from parsing a // pattern and so we don't need to store a parser snapshot. let parser_snapshot_before_pat = if diff --git a/src/test/ui/E0642.stderr b/src/test/ui/E0642.stderr index 07ec8b4cc2c8f..8c16b8b30cd53 100644 --- a/src/test/ui/E0642.stderr +++ b/src/test/ui/E0642.stderr @@ -3,7 +3,7 @@ error[E0642]: patterns aren't allowed in trait methods | LL | fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in trait methods | ^^^^^^ -help: give this argument a name or use an underscore to ignore it, instead of a tuple pattern +help: give this argument a name or use an underscore to ignore it instead of using a tuple pattern | LL | fn foo(_: (i32, i32)); //~ ERROR patterns aren't allowed in trait methods | ^ @@ -13,7 +13,7 @@ error[E0642]: patterns aren't allowed in trait methods | LL | fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods | ^^^^^^ -help: give this argument a name or use an underscore to ignore it, instead of a tuple pattern +help: give this argument a name or use an underscore to ignore it instead of using a tuple pattern | LL | fn bar(_: (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods | ^ From e4c3b49fe790dd23f32acace8157d897d31c0cb3 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 9 Aug 2018 23:23:08 +0100 Subject: [PATCH 5/7] Emit an error during parsing --- src/librustc_passes/ast_validation.rs | 21 ++---- src/libsyntax/parse/parser.rs | 104 +++++++++++++++----------- src/test/ui/E0642.stderr | 9 +-- 3 files changed, 74 insertions(+), 60 deletions(-) diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 2195331f465ba..e15dab404f478 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -336,24 +336,19 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if let TraitItemKind::Method(ref sig, ref block) = trait_item.node { self.check_trait_fn_not_async(trait_item.span, sig.header.asyncness); self.check_trait_fn_not_const(sig.header.constness); - self.check_decl_no_pat(&sig.decl, |span, mut_ident| { - if mut_ident { - if block.is_none() { + if block.is_none() { + self.check_decl_no_pat(&sig.decl, |span, mut_ident| { + if mut_ident { self.session.buffer_lint( lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY, trait_item.id, span, "patterns aren't allowed in trait methods"); + } else { + struct_span_err!(self.session, span, E0642, + "patterns aren't allowed in trait methods").emit(); } - } else { - let mut err = struct_span_err!(self.session, span, E0642, - "patterns aren't allowed in trait methods"); - let suggestion = "give this argument a name or use an \ - underscore to ignore it instead of using a \ - tuple pattern"; - err.span_suggestion(span, suggestion, "_".to_owned()); - err.emit(); - } - }); + }); + } } } } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index a1dbe93fdfe35..9a49d705c464b 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1744,54 +1744,74 @@ impl<'a> Parser<'a> { fn parse_arg_general(&mut self, require_name: bool) -> PResult<'a, Arg> { maybe_whole!(self, NtArg, |x| x); - // If we see `ident :`, then we know that the argument is not just of the - // form `type`, which means we won't need to recover from parsing a - // pattern and so we don't need to store a parser snapshot. - let parser_snapshot_before_pat = if - self.look_ahead(1, |t| t.is_ident()) && - self.look_ahead(2, |t| t == &token::Colon) { - None - } else { - Some(self.clone()) - }; - - // We're going to try parsing the argument as a pattern (even if it's not - // allowed, such as for trait methods without bodies). This way we can provide - // better errors to the user. - let pat_arg: PResult<'a, (P, P)> = do catch { + let (pat, ty) = if require_name || self.is_named_argument() { + debug!("parse_arg_general parse_pat (require_name:{})", + require_name); let pat = self.parse_pat()?; + self.expect(&token::Colon)?; (pat, self.parse_ty()?) - }; + } else { + debug!("parse_arg_general ident_to_pat"); + + // If we see `ident :`, then we know that the argument is not just of the + // form `type`, which means we won't need to recover from parsing a + // pattern and so we don't need to store a parser snapshot. + let parser_snapshot_before_pat = if + self.look_ahead(1, |t| t.is_ident()) && + self.look_ahead(2, |t| t == &token::Colon) { + None + } else { + Some(self.clone()) + }; - match pat_arg { - Ok((pat, ty)) => { - Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID }) - } - Err(mut err) => { - match (require_name || self.is_named_argument(), parser_snapshot_before_pat) { - (true, _) | (_, None) => { - Err(err) - } - (false, Some(parser_snapshot_before_pat)) => { - err.cancel(); - // Recover from attempting to parse the argument as a pattern. This means - // the type is alone, with no name, e.g. `fn foo(u32)`. - mem::replace(self, parser_snapshot_before_pat); - debug!("parse_arg_general ident_to_pat"); - let ident = Ident::new(keywords::Invalid.name(), self.prev_span); - let ty = self.parse_ty()?; - let pat = P(Pat { - id: ast::DUMMY_NODE_ID, - node: PatKind::Ident( - BindingMode::ByValue(Mutability::Immutable), ident, None), - span: ty.span, - }); - Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID }) - } + // We're going to try parsing the argument as a pattern (even though it's not + // allowed). This way we can provide better errors to the user. + let pat_arg: PResult<'a, _> = do catch { + let pat = self.parse_pat()?; + self.expect(&token::Colon)?; + (pat, self.parse_ty()?) + }; + + match pat_arg { + Ok((pat, ty)) => { + let mut err = self.diagnostic() + .struct_span_err(pat.span, "patterns aren't allowed in trait methods"); + err.span_suggestion_short_with_applicability( + pat.span, + "give this argument a name or use an underscore to ignore it", + "_".to_owned(), + Applicability::MachineApplicable, + ); + err.emit(); + // Pretend the pattern is `_`, to avoid duplicate errors from AST validation. + let pat = P(Pat { + node: PatKind::Wild, + span: pat.span, + id: ast::DUMMY_NODE_ID + }); + (pat, ty) + } + Err(mut err) => { + err.cancel(); + // Recover from attempting to parse the argument as a pattern. This means + // the type is alone, with no name, e.g. `fn foo(u32)`. + mem::replace(self, parser_snapshot_before_pat.unwrap()); + debug!("parse_arg_general ident_to_pat"); + let ident = Ident::new(keywords::Invalid.name(), self.prev_span); + let ty = self.parse_ty()?; + let pat = P(Pat { + id: ast::DUMMY_NODE_ID, + node: PatKind::Ident( + BindingMode::ByValue(Mutability::Immutable), ident, None), + span: ty.span, + }); + (pat, ty) } } - } + }; + + Ok(Arg { ty, pat, id: ast::DUMMY_NODE_ID }) } /// Parse a single function argument diff --git a/src/test/ui/E0642.stderr b/src/test/ui/E0642.stderr index 8c16b8b30cd53..b8e0496945a15 100644 --- a/src/test/ui/E0642.stderr +++ b/src/test/ui/E0642.stderr @@ -1,23 +1,22 @@ -error[E0642]: patterns aren't allowed in trait methods +error: patterns aren't allowed in trait methods --> $DIR/E0642.rs:12:12 | LL | fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in trait methods | ^^^^^^ -help: give this argument a name or use an underscore to ignore it instead of using a tuple pattern +help: give this argument a name or use an underscore to ignore it | LL | fn foo(_: (i32, i32)); //~ ERROR patterns aren't allowed in trait methods | ^ -error[E0642]: patterns aren't allowed in trait methods +error: patterns aren't allowed in trait methods --> $DIR/E0642.rs:16:12 | LL | fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods | ^^^^^^ -help: give this argument a name or use an underscore to ignore it instead of using a tuple pattern +help: give this argument a name or use an underscore to ignore it | LL | fn bar(_: (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods | ^ error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0642`. From 49e9c5fe90db9a70697da8a3bf4237492376c541 Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 10 Aug 2018 01:49:45 +0100 Subject: [PATCH 6/7] Add E0642 to parser error --- src/libsyntax/parse/parser.rs | 9 ++++++--- src/test/ui/E0642.stderr | 5 +++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 9a49d705c464b..14026c5bede68 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -44,7 +44,7 @@ use ast::{RangeEnd, RangeSyntax}; use {ast, attr}; use codemap::{self, CodeMap, Spanned, respan}; use syntax_pos::{self, Span, MultiSpan, BytePos, FileName, edition::Edition}; -use errors::{self, Applicability, DiagnosticBuilder}; +use errors::{self, Applicability, DiagnosticBuilder, DiagnosticId}; use parse::{self, SeqSep, classify, token}; use parse::lexer::TokenAndSpan; use parse::lexer::comments::{doc_comment_style, strip_doc_comment_decoration}; @@ -1775,8 +1775,11 @@ impl<'a> Parser<'a> { match pat_arg { Ok((pat, ty)) => { - let mut err = self.diagnostic() - .struct_span_err(pat.span, "patterns aren't allowed in trait methods"); + let mut err = self.diagnostic().struct_span_err_with_code( + pat.span, + "patterns aren't allowed in trait methods", + DiagnosticId::Error("E0642".into()), + ); err.span_suggestion_short_with_applicability( pat.span, "give this argument a name or use an underscore to ignore it", diff --git a/src/test/ui/E0642.stderr b/src/test/ui/E0642.stderr index b8e0496945a15..1723e97b45e42 100644 --- a/src/test/ui/E0642.stderr +++ b/src/test/ui/E0642.stderr @@ -1,4 +1,4 @@ -error: patterns aren't allowed in trait methods +error[E0642]: patterns aren't allowed in trait methods --> $DIR/E0642.rs:12:12 | LL | fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in trait methods @@ -8,7 +8,7 @@ help: give this argument a name or use an underscore to ignore it LL | fn foo(_: (i32, i32)); //~ ERROR patterns aren't allowed in trait methods | ^ -error: patterns aren't allowed in trait methods +error[E0642]: patterns aren't allowed in trait methods --> $DIR/E0642.rs:16:12 | LL | fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods @@ -20,3 +20,4 @@ LL | fn bar(_: (i32, i32)) {} //~ ERROR patterns aren't allowed in trait met error: aborting due to 2 previous errors +For more information about this error, try `rustc --explain E0642`. From 5c814e2e4e0649972ec6a18c7dbf57259edf2210 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 11 Aug 2018 21:25:48 +0100 Subject: [PATCH 7/7] Clean up and add extra tests --- src/librustc_passes/ast_validation.rs | 4 ++-- src/libsyntax/parse/parser.rs | 15 +++------------ src/test/compile-fail/no-patterns-in-args-2.rs | 4 ++-- .../compile-fail/no-patterns-in-args-macro.rs | 2 +- src/test/ui/E0642.rs | 15 ++++++++++----- src/test/ui/E0642.stderr | 16 ++++++++-------- 6 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index e15dab404f478..0ea90e7453190 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -342,10 +342,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> { self.session.buffer_lint( lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY, trait_item.id, span, - "patterns aren't allowed in trait methods"); + "patterns aren't allowed in methods without bodies"); } else { struct_span_err!(self.session, span, E0642, - "patterns aren't allowed in trait methods").emit(); + "patterns aren't allowed in methods without bodies").emit(); } }); } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 14026c5bede68..746e03d771a88 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1754,16 +1754,7 @@ impl<'a> Parser<'a> { } else { debug!("parse_arg_general ident_to_pat"); - // If we see `ident :`, then we know that the argument is not just of the - // form `type`, which means we won't need to recover from parsing a - // pattern and so we don't need to store a parser snapshot. - let parser_snapshot_before_pat = if - self.look_ahead(1, |t| t.is_ident()) && - self.look_ahead(2, |t| t == &token::Colon) { - None - } else { - Some(self.clone()) - }; + let parser_snapshot_before_pat = self.clone(); // We're going to try parsing the argument as a pattern (even though it's not // allowed). This way we can provide better errors to the user. @@ -1777,7 +1768,7 @@ impl<'a> Parser<'a> { Ok((pat, ty)) => { let mut err = self.diagnostic().struct_span_err_with_code( pat.span, - "patterns aren't allowed in trait methods", + "patterns aren't allowed in methods without bodies", DiagnosticId::Error("E0642".into()), ); err.span_suggestion_short_with_applicability( @@ -1799,7 +1790,7 @@ impl<'a> Parser<'a> { err.cancel(); // Recover from attempting to parse the argument as a pattern. This means // the type is alone, with no name, e.g. `fn foo(u32)`. - mem::replace(self, parser_snapshot_before_pat.unwrap()); + mem::replace(self, parser_snapshot_before_pat); debug!("parse_arg_general ident_to_pat"); let ident = Ident::new(keywords::Invalid.name(), self.prev_span); let ty = self.parse_ty()?; diff --git a/src/test/compile-fail/no-patterns-in-args-2.rs b/src/test/compile-fail/no-patterns-in-args-2.rs index 80e6967980119..4d2412c34a5fa 100644 --- a/src/test/compile-fail/no-patterns-in-args-2.rs +++ b/src/test/compile-fail/no-patterns-in-args-2.rs @@ -11,9 +11,9 @@ #![deny(patterns_in_fns_without_body)] trait Tr { - fn f1(mut arg: u8); //~ ERROR patterns aren't allowed in trait methods + fn f1(mut arg: u8); //~ ERROR patterns aren't allowed in methods without bodies //~^ WARN was previously accepted - fn f2(&arg: u8); //~ ERROR patterns aren't allowed in trait methods + fn f2(&arg: u8); //~ ERROR patterns aren't allowed in methods without bodies fn g1(arg: u8); // OK fn g2(_: u8); // OK #[allow(anonymous_parameters)] diff --git a/src/test/compile-fail/no-patterns-in-args-macro.rs b/src/test/compile-fail/no-patterns-in-args-macro.rs index 546c40ecbd045..f85ce8f57ea71 100644 --- a/src/test/compile-fail/no-patterns-in-args-macro.rs +++ b/src/test/compile-fail/no-patterns-in-args-macro.rs @@ -30,7 +30,7 @@ mod bad_pat { m!((bad, pat)); //~^ ERROR patterns aren't allowed in function pointer types //~| ERROR patterns aren't allowed in foreign function declarations - //~| ERROR patterns aren't allowed in trait methods + //~| ERROR patterns aren't allowed in methods without bodies } fn main() {} diff --git a/src/test/ui/E0642.rs b/src/test/ui/E0642.rs index 837a9365271e2..58ccfc56ab79a 100644 --- a/src/test/ui/E0642.rs +++ b/src/test/ui/E0642.rs @@ -8,12 +8,17 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -trait Foo { - fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in trait methods -} +#[derive(Clone, Copy)] +struct S; + +trait T { + fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in methods without bodies + + fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in methods without bodies -trait Bar { - fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods + fn f(&ident: &S) {} // ok + fn g(&&ident: &&S) {} // ok + fn h(mut ident: S) {} // ok } fn main() {} diff --git a/src/test/ui/E0642.stderr b/src/test/ui/E0642.stderr index 1723e97b45e42..34c163e210970 100644 --- a/src/test/ui/E0642.stderr +++ b/src/test/ui/E0642.stderr @@ -1,21 +1,21 @@ -error[E0642]: patterns aren't allowed in trait methods - --> $DIR/E0642.rs:12:12 +error[E0642]: patterns aren't allowed in methods without bodies + --> $DIR/E0642.rs:15:12 | -LL | fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in trait methods +LL | fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in methods without bodies | ^^^^^^ help: give this argument a name or use an underscore to ignore it | -LL | fn foo(_: (i32, i32)); //~ ERROR patterns aren't allowed in trait methods +LL | fn foo(_: (i32, i32)); //~ ERROR patterns aren't allowed in methods without bodies | ^ -error[E0642]: patterns aren't allowed in trait methods - --> $DIR/E0642.rs:16:12 +error[E0642]: patterns aren't allowed in methods without bodies + --> $DIR/E0642.rs:17:12 | -LL | fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods +LL | fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in methods without bodies | ^^^^^^ help: give this argument a name or use an underscore to ignore it | -LL | fn bar(_: (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods +LL | fn bar(_: (i32, i32)) {} //~ ERROR patterns aren't allowed in methods without bodies | ^ error: aborting due to 2 previous errors