Skip to content

Commit

Permalink
syntax: fix rust-lang#64682.
Browse files Browse the repository at this point in the history
Fuse parsing of `self` into `parse_param_general`.
  • Loading branch information
Centril committed Sep 29, 2019
1 parent b61e694 commit 8fd03b1
Show file tree
Hide file tree
Showing 15 changed files with 391 additions and 127 deletions.
73 changes: 23 additions & 50 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -974,15 +974,22 @@ impl<'a> Parser<'a> {
/// This version of parse param doesn't necessarily require identifier names.
fn parse_param_general(
&mut self,
is_self_allowed: bool,
is_trait_item: bool,
allow_c_variadic: bool,
is_name_required: impl Fn(&token::Token) -> bool,
) -> PResult<'a, Param> {
let lo = self.token.span;
let attrs = self.parse_outer_attributes()?;

// Possibly parse `self`. Recover if we parsed it and it wasn't allowed here.
if let Some(mut param) = self.parse_self_param()? {
param.attrs = attrs.into();
return self.recover_bad_self_param(param, is_trait_item);
return if is_self_allowed {
Ok(param)
} else {
self.recover_bad_self_param(param, is_trait_item)
};
}

let is_name_required = is_name_required(&self.token);
Expand Down Expand Up @@ -1207,6 +1214,7 @@ impl<'a> Parser<'a> {
}
};
match p.parse_param_general(
false,
false,
allow_c_variadic,
do_not_enforce_named_arguments_for_c_variadic
Expand Down Expand Up @@ -1359,60 +1367,25 @@ impl<'a> Parser<'a> {
Ok(Some(Param::from_self(ThinVec::default(), eself, eself_ident)))
}

/// Returns the parsed optional self parameter with attributes and whether a self
/// shortcut was used.
fn parse_self_parameter_with_attrs(&mut self) -> PResult<'a, Option<Param>> {
let attrs = self.parse_outer_attributes()?;
let param_opt = self.parse_self_param()?;
Ok(param_opt.map(|mut param| {
param.attrs = attrs.into();
param
}))
}

/// Parses the parameter list and result type of a function that may have a `self` parameter.
fn parse_fn_decl_with_self<F>(&mut self, parse_param_fn: F) -> PResult<'a, P<FnDecl>>
where F: FnMut(&mut Parser<'a>) -> PResult<'a, Param>,
{
self.expect(&token::OpenDelim(token::Paren))?;

// Parse optional self argument.
let self_param = self.parse_self_parameter_with_attrs()?;

// Parse the rest of the function parameter list.
let sep = SeqSep::trailing_allowed(token::Comma);
let (mut fn_inputs, recovered) = if let Some(self_param) = self_param {
if self.check(&token::CloseDelim(token::Paren)) {
(vec![self_param], false)
} else if self.eat(&token::Comma) {
let mut fn_inputs = vec![self_param];
let (mut input, _, recovered) = self.parse_seq_to_before_end(
&token::CloseDelim(token::Paren), sep, parse_param_fn)?;
fn_inputs.append(&mut input);
(fn_inputs, recovered)
} else {
match self.expect_one_of(&[], &[]) {
Err(err) => return Err(err),
Ok(recovered) => (vec![self_param], recovered),
}
}
} else {
let (input, _, recovered) =
self.parse_seq_to_before_end(&token::CloseDelim(token::Paren),
sep,
parse_param_fn)?;
(input, recovered)
};
fn parse_fn_decl_with_self(
&mut self,
is_name_required: impl Copy + Fn(&token::Token) -> bool,
) -> PResult<'a, P<FnDecl>> {
// Parse the arguments, starting out with `self` being allowed...
let mut is_self_allowed = true;
let (mut inputs, _): (Vec<_>, _) = self.parse_paren_comma_seq(|p| {
let res = p.parse_param_general(is_self_allowed, true, false, is_name_required);
// ...but now that we've parsed the first argument, `self` is no longer allowed.
is_self_allowed = false;
res
})?;

if !recovered {
// Parse closing paren and return type.
self.expect(&token::CloseDelim(token::Paren))?;
}
// Replace duplicated recovered params with `_` pattern to avoid unecessary errors.
self.deduplicate_recovered_params_names(&mut fn_inputs);
self.deduplicate_recovered_params_names(&mut inputs);

Ok(P(FnDecl {
inputs: fn_inputs,
inputs,
output: self.parse_ret_ty(true)?,
c_variadic: false
}))
Expand Down
44 changes: 18 additions & 26 deletions src/libsyntax/parse/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,13 +424,7 @@ impl<'a> Parser<'a> {
} else if self.look_ahead(1, |t| *t == token::OpenDelim(token::Paren)) {
let ident = self.parse_ident().unwrap();
self.bump(); // `(`
let kw_name = if let Ok(Some(_)) = self.parse_self_parameter_with_attrs()
.map_err(|mut e| e.cancel())
{
"method"
} else {
"function"
};
let kw_name = self.recover_first_param();
self.consume_block(token::Paren);
let (kw, kw_name, ambiguous) = if self.check(&token::RArrow) {
self.eat_to_tokens(&[&token::OpenDelim(token::Brace)]);
Expand Down Expand Up @@ -477,13 +471,7 @@ impl<'a> Parser<'a> {
self.eat_to_tokens(&[&token::Gt]);
self.bump(); // `>`
let (kw, kw_name, ambiguous) = if self.eat(&token::OpenDelim(token::Paren)) {
if let Ok(Some(_)) = self.parse_self_parameter_with_attrs()
.map_err(|mut e| e.cancel())
{
("fn", "method", false)
} else {
("fn", "function", false)
}
("fn", self.recover_first_param(), false)
} else if self.check(&token::OpenDelim(token::Brace)) {
("struct", "struct", false)
} else {
Expand All @@ -505,6 +493,16 @@ impl<'a> Parser<'a> {
self.parse_macro_use_or_failure(attrs, macros_allowed, attributes_allowed, lo, visibility)
}

fn recover_first_param(&mut self) -> &'static str {
match self.parse_outer_attributes()
.and_then(|_| self.parse_self_param())
.map_err(|mut e| e.cancel())
{
Ok(Some(_)) => "method",
_ => "function",
}
}

/// This is the fall-through for parsing items.
fn parse_macro_use_or_failure(
&mut self,
Expand Down Expand Up @@ -861,9 +859,7 @@ impl<'a> Parser<'a> {
let (constness, unsafety, asyncness, abi) = self.parse_fn_front_matter()?;
let ident = self.parse_ident()?;
let mut generics = self.parse_generics()?;
let decl = self.parse_fn_decl_with_self(|p| {
p.parse_param_general(true, false, |_| true)
})?;
let decl = self.parse_fn_decl_with_self(|_| true)?;
generics.where_clause = self.parse_where_clause()?;
*at_end = true;
let (inner_attrs, body) = self.parse_inner_attrs_and_block()?;
Expand Down Expand Up @@ -1034,15 +1030,11 @@ impl<'a> Parser<'a> {
let ident = self.parse_ident()?;
let mut generics = self.parse_generics()?;

let decl = 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...

// We don't allow argument names to be left off in edition 2018.
let is_name_required = p.token.span.rust_2018();
p.parse_param_general(true, false, |_| is_name_required)
})?;
// This is somewhat dubious; We don't want to allow
// argument names to be left off if there is a definition...
//
// We don't allow argument names to be left off in edition 2018.
let decl = self.parse_fn_decl_with_self(|t| t.span.rust_2018())?;
generics.where_clause = self.parse_where_clause()?;

let sig = ast::MethodSig {
Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/lint/lint-unused-variables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ impl RefStruct {
b: i32,
//~^ ERROR unused variable: `b`
) {}
fn issue_64682_associated_fn(
#[allow(unused_variables)] a: i32,
b: i32,
//~^ ERROR unused variable: `b`
) {}
}
trait RefTrait {
fn bar(
Expand All @@ -37,6 +42,11 @@ trait RefTrait {
b: i32,
//~^ ERROR unused variable: `b`
) {}
fn issue_64682_associated_fn(
#[allow(unused_variables)] a: i32,
b: i32,
//~^ ERROR unused variable: `b`
) {}
}
impl RefTrait for RefStruct {
fn bar(
Expand All @@ -45,6 +55,11 @@ impl RefTrait for RefStruct {
b: i32,
//~^ ERROR unused variable: `b`
) {}
fn issue_64682_associated_fn(
#[allow(unused_variables)] a: i32,
b: i32,
//~^ ERROR unused variable: `b`
) {}
}

fn main() {
Expand Down
28 changes: 23 additions & 5 deletions src/test/ui/lint/lint-unused-variables.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,25 @@ LL | b: i32,
| ^ help: consider prefixing with an underscore: `_b`

error: unused variable: `a`
--> $DIR/lint-unused-variables.rs:53:9
--> $DIR/lint-unused-variables.rs:68:9
|
LL | a: i32,
| ^ help: consider prefixing with an underscore: `_a`

error: unused variable: `b`
--> $DIR/lint-unused-variables.rs:59:9
--> $DIR/lint-unused-variables.rs:74:9
|
LL | b: i32,
| ^ help: consider prefixing with an underscore: `_b`

error: unused variable: `b`
--> $DIR/lint-unused-variables.rs:37:9
--> $DIR/lint-unused-variables.rs:42:9
|
LL | b: i32,
| ^ help: consider prefixing with an underscore: `_b`

error: unused variable: `b`
--> $DIR/lint-unused-variables.rs:47:9
|
LL | b: i32,
| ^ help: consider prefixing with an underscore: `_b`
Expand All @@ -47,10 +53,22 @@ LL | b: i32,
| ^ help: consider prefixing with an underscore: `_b`

error: unused variable: `b`
--> $DIR/lint-unused-variables.rs:45:9
--> $DIR/lint-unused-variables.rs:34:9
|
LL | b: i32,
| ^ help: consider prefixing with an underscore: `_b`

error: unused variable: `b`
--> $DIR/lint-unused-variables.rs:55:9
|
LL | b: i32,
| ^ help: consider prefixing with an underscore: `_b`

error: unused variable: `b`
--> $DIR/lint-unused-variables.rs:60:9
|
LL | b: i32,
| ^ help: consider prefixing with an underscore: `_b`

error: aborting due to 8 previous errors
error: aborting due to 11 previous errors

16 changes: 16 additions & 0 deletions src/test/ui/rfc-2565-param-attrs/attr-without-param.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#[cfg(FALSE)]
impl S {
fn f(#[attr]) {} //~ ERROR expected parameter name, found `)`
}

#[cfg(FALSE)]
impl T for S {
fn f(#[attr]) {} //~ ERROR expected parameter name, found `)`
}

#[cfg(FALSE)]
trait T {
fn f(#[attr]); //~ ERROR expected argument name, found `)`
}

fn main() {}
20 changes: 20 additions & 0 deletions src/test/ui/rfc-2565-param-attrs/attr-without-param.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: expected parameter name, found `)`
--> $DIR/attr-without-param.rs:3:17
|
LL | fn f(#[attr]) {}
| ^ expected parameter name

error: expected parameter name, found `)`
--> $DIR/attr-without-param.rs:8:17
|
LL | fn f(#[attr]) {}
| ^ expected parameter name

error: expected argument name, found `)`
--> $DIR/attr-without-param.rs:13:17
|
LL | fn f(#[attr]);
| ^ expected argument name

error: aborting due to 3 previous errors

12 changes: 11 additions & 1 deletion src/test/ui/rfc-2565-param-attrs/auxiliary/param-attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ macro_rules! checker {
($attr_name:ident, $expected:literal) => {
#[proc_macro_attribute]
pub fn $attr_name(attr: TokenStream, input: TokenStream) -> TokenStream {
assert!(attr.to_string().is_empty());
assert_eq!(input.to_string(), $expected);
TokenStream::new()
}
Expand All @@ -28,7 +27,18 @@ checker!(attr_inherent_1, "fn inherent1(#[a1] self, #[a2] arg1: u8) { }");
checker!(attr_inherent_2, "fn inherent2(#[a1] &self, #[a2] arg1: u8) { }");
checker!(attr_inherent_3, "fn inherent3<'a>(#[a1] &'a mut self, #[a2] arg1: u8) { }");
checker!(attr_inherent_4, "fn inherent4<'a>(#[a1] self: Box<Self>, #[a2] arg1: u8) { }");
checker!(attr_inherent_issue_64682, "fn inherent5(#[a1] #[a2] arg1: u8, #[a3] arg2: u8) { }");
checker!(attr_trait_1, "fn trait1(#[a1] self, #[a2] arg1: u8);");
checker!(attr_trait_2, "fn trait2(#[a1] &self, #[a2] arg1: u8);");
checker!(attr_trait_3, "fn trait3<'a>(#[a1] &'a mut self, #[a2] arg1: u8);");
checker!(attr_trait_4, "fn trait4<'a>(#[a1] self: Box<Self>, #[a2] arg1: u8, #[a3] Vec<u8>);");
checker!(attr_trait_issue_64682, "fn trait5(#[a1] #[a2] arg1: u8, #[a3] arg2: u8);");
checker!(rename_params, r#"impl Foo {
fn hello(#[angery(true)] a: i32, #[a2] b: i32, #[what = "how"] c: u32) { }
fn hello2(#[a1] #[a2] a: i32, #[what = "how"] b: i32,
#[angery(true)] c: u32) {
}
fn hello_self(#[a1] #[a2] &self, #[a1] #[a2] a: i32,
#[what = "how"] b: i32, #[angery(true)] c: u32) {
}
}"#);
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// aux-build:param-attrs.rs

// check-pass

extern crate param_attrs;

use param_attrs::rename_params;

#[rename_params(send_help)]
impl Foo {
fn hello(#[angery(true)] a: i32, #[a2] b: i32, #[what = "how"] c: u32) {}
fn hello2(#[a1] #[a2] a: i32, #[what = "how"] b: i32, #[angery(true)] c: u32) {}
fn hello_self(
#[a1] #[a2] &self,
#[a1] #[a2] a: i32,
#[what = "how"] b: i32,
#[angery(true)] c: u32
) {}
}

fn main() {}
Loading

0 comments on commit 8fd03b1

Please sign in to comment.