Skip to content

Commit

Permalink
Merge #3040
Browse files Browse the repository at this point in the history
3040: Rework value parameter parsing r=matklad a=tobz1000

Fixes #2847.

- `Fn__(...)` parameters with idents/patterns no longer parse
- Trait function parameters with arbitrary patterns parse
- Trait function parameters without idents/patterns no longer parse
- `fn(...)` parameters no longer parse with patterns other than a single ident

__Question__: The pre-existing test `param_list_opt_patterns` has been kept as-is, although the name no longer makes sense (it's testing `Fn__(...)` params, which aren't allowed patterns any more). What would be best to do about this?

Co-authored-by: Toby Dimmick <tobydimmick@pm.me>
  • Loading branch information
bors[bot] and tobz1000 committed Feb 7, 2020
2 parents 4d0d113 + 90ff2be commit 6d6a995
Show file tree
Hide file tree
Showing 19 changed files with 686 additions and 259 deletions.
2 changes: 1 addition & 1 deletion crates/ra_parser/src/grammar/expressions/atom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ fn lambda_expr(p: &mut Parser) -> CompletedMarker {
let m = p.start();
p.eat(T![async]);
p.eat(T![move]);
params::param_list_opt_types(p);
params::param_list_closure(p);
if opt_fn_ret_type(p) {
if !p.at(T!['{']) {
p.error("expected `{`");
Expand Down
9 changes: 3 additions & 6 deletions crates/ra_parser/src/grammar/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ pub(super) fn maybe_item(p: &mut Parser, m: Marker, flavor: ItemFlavor) -> Resul
// async unsafe fn foo() {}
// unsafe const fn bar() {}
T![fn] => {
fn_def(p, flavor);
fn_def(p);
m.complete(p, FN_DEF);
}

Expand Down Expand Up @@ -301,7 +301,7 @@ pub(crate) fn extern_item_list(p: &mut Parser) {
m.complete(p, EXTERN_ITEM_LIST);
}

fn fn_def(p: &mut Parser, flavor: ItemFlavor) {
fn fn_def(p: &mut Parser) {
assert!(p.at(T![fn]));
p.bump(T![fn]);

Expand All @@ -311,10 +311,7 @@ fn fn_def(p: &mut Parser, flavor: ItemFlavor) {
type_params::opt_type_param_list(p);

if p.at(T!['(']) {
match flavor {
ItemFlavor::Mod => params::param_list(p),
ItemFlavor::Trait => params::param_list_opt_patterns(p),
}
params::param_list_fn_def(p);
} else {
p.error("expected function arguments");
}
Expand Down
120 changes: 71 additions & 49 deletions crates/ra_parser/src/grammar/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,54 +7,60 @@ use super::*;
// fn b(x: i32) {}
// fn c(x: i32, ) {}
// fn d(x: i32, y: ()) {}
pub(super) fn param_list(p: &mut Parser) {
list_(p, Flavor::Normal)
pub(super) fn param_list_fn_def(p: &mut Parser) {
list_(p, Flavor::FnDef)
}

// test param_list_opt_patterns
// fn foo<F: FnMut(&mut Foo<'a>)>(){}
pub(super) fn param_list_opt_patterns(p: &mut Parser) {
list_(p, Flavor::OptionalPattern)
pub(super) fn param_list_fn_trait(p: &mut Parser) {
list_(p, Flavor::FnTrait)
}

pub(super) fn param_list_opt_types(p: &mut Parser) {
list_(p, Flavor::OptionalType)
pub(super) fn param_list_fn_ptr(p: &mut Parser) {
list_(p, Flavor::FnPointer)
}

#[derive(Clone, Copy, Eq, PartialEq)]
enum Flavor {
OptionalType,
OptionalPattern,
Normal,
pub(super) fn param_list_closure(p: &mut Parser) {
list_(p, Flavor::Closure)
}

impl Flavor {
fn type_required(self) -> bool {
match self {
Flavor::OptionalType => false,
_ => true,
}
}
#[derive(Debug, Clone, Copy)]
enum Flavor {
FnDef, // Includes trait fn params; omitted param idents are not supported
FnTrait, // Params for `Fn(...)`/`FnMut(...)`/`FnOnce(...)` annotations
FnPointer,
Closure,
}

fn list_(p: &mut Parser, flavor: Flavor) {
let (bra, ket) = if flavor.type_required() { (T!['('], T![')']) } else { (T![|], T![|]) };
assert!(p.at(bra));
use Flavor::*;

let (bra, ket) = match flavor {
Closure => (T![|], T![|]),
FnDef | FnTrait | FnPointer => (T!['('], T![')']),
};

let m = p.start();
p.bump(bra);
if flavor.type_required() {

if let FnDef = flavor {
// test self_param_outer_attr
// fn f(#[must_use] self) {}
attributes::outer_attributes(p);
opt_self_param(p);
}

while !p.at(EOF) && !p.at(ket) {
// test param_outer_arg
// fn f(#[attr1] pat: Type) {}
attributes::outer_attributes(p);

if flavor.type_required() && p.at(T![...]) {
break;
// test param_list_vararg
// extern "C" { fn printf(format: *const i8, ...) -> i32; }
match flavor {
FnDef | FnPointer if p.eat(T![...]) => break,
_ => (),
}

if !p.at_ts(VALUE_PARAMETER_FIRST) {
Expand All @@ -66,11 +72,7 @@ fn list_(p: &mut Parser, flavor: Flavor) {
p.expect(T![,]);
}
}
// test param_list_vararg
// extern "C" { fn printf(format: *const i8, ...) -> i32; }
if flavor.type_required() {
p.eat(T![...]);
}

p.expect(ket);
m.complete(p, PARAM_LIST);
}
Expand All @@ -80,36 +82,56 @@ const VALUE_PARAMETER_FIRST: TokenSet = patterns::PATTERN_FIRST.union(types::TYP
fn value_parameter(p: &mut Parser, flavor: Flavor) {
let m = p.start();
match flavor {
Flavor::OptionalType | Flavor::Normal => {
// test trait_fn_placeholder_parameter
// trait Foo {
// fn bar(_: u64, mut x: i32);
// }

// test trait_fn_patterns
// trait T {
// fn f1((a, b): (usize, usize)) {}
// fn f2(S { a, b }: S) {}
// fn f3(NewType(a): NewType) {}
// fn f4(&&a: &&usize) {}
// }

// test fn_patterns
// impl U {
// fn f1((a, b): (usize, usize)) {}
// fn f2(S { a, b }: S) {}
// fn f3(NewType(a): NewType) {}
// fn f4(&&a: &&usize) {}
// }
Flavor::FnDef => {
patterns::pattern(p);
if p.at(T![:]) && !p.at(T![::]) || flavor.type_required() {
types::ascription(p)
}
types::ascription(p);
}
// test value_parameters_no_patterns
// type F = Box<Fn(a: i32, &b: &i32, &mut c: &i32, ())>;
Flavor::OptionalPattern => {
let la0 = p.current();
let la1 = p.nth(1);
let la2 = p.nth(2);
let la3 = p.nth(3);

// test trait_fn_placeholder_parameter
// trait Foo {
// fn bar(_: u64, mut x: i32);
// }
if (la0 == IDENT || la0 == T![_]) && la1 == T![:] && !p.nth_at(1, T![::])
|| la0 == T![mut] && la1 == IDENT && la2 == T![:]
|| la0 == T![&]
&& (la1 == IDENT && la2 == T![:] && !p.nth_at(2, T![::])
|| la1 == T![mut] && la2 == IDENT && la3 == T![:] && !p.nth_at(3, T![::]))
{
// type F = Box<Fn(i32, &i32, &i32, ())>;
Flavor::FnTrait => {
types::type_(p);
}
// test fn_pointer_param_ident_path
// type Foo = fn(Bar::Baz);
// type Qux = fn(baz: Bar::Baz);
Flavor::FnPointer => {
if p.at(IDENT) && p.nth(1) == T![:] && !p.nth_at(1, T![::]) {
patterns::pattern(p);
types::ascription(p);
} else {
types::type_(p);
}
}
// test closure_params
// fn main() {
// let foo = |bar, baz: Baz, qux: Qux::Quux| ();
// }
Flavor::Closure => {
patterns::pattern(p);
if p.at(T![:]) && !p.at(T![::]) {
types::ascription(p);
}
}
}
m.complete(p, PARAM);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/ra_parser/src/grammar/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ fn opt_path_type_args(p: &mut Parser, mode: Mode) {
Mode::Use => return,
Mode::Type => {
// test path_fn_trait_args
// type F = Box<Fn(x: i32) -> ()>;
// type F = Box<Fn(i32) -> ()>;
if p.at(T!['(']) {
params::param_list_opt_patterns(p);
params::param_list_fn_trait(p);
opt_fn_ret_type(p);
} else {
type_args::opt_type_arg_list(p, false)
Expand Down
2 changes: 1 addition & 1 deletion crates/ra_parser/src/grammar/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ fn fn_pointer_type(p: &mut Parser) {
return;
}
if p.at(T!['(']) {
params::param_list_opt_patterns(p);
params::param_list_fn_ptr(p);
} else {
p.error("expected parameters")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
type F = Box<Fn(a: i32, &b: &i32, &mut c: &i32, ())>;
type F = Box<Fn(i32, &i32, &i32, ())>;
Original file line number Diff line number Diff line change
@@ -1,81 +1,60 @@
SOURCE_FILE@[0; 54)
TYPE_ALIAS_DEF@[0; 53)
SOURCE_FILE@[0; 39)
TYPE_ALIAS_DEF@[0; 38)
TYPE_KW@[0; 4) "type"
WHITESPACE@[4; 5) " "
NAME@[5; 6)
IDENT@[5; 6) "F"
WHITESPACE@[6; 7) " "
EQ@[7; 8) "="
WHITESPACE@[8; 9) " "
PATH_TYPE@[9; 52)
PATH@[9; 52)
PATH_SEGMENT@[9; 52)
PATH_TYPE@[9; 37)
PATH@[9; 37)
PATH_SEGMENT@[9; 37)
NAME_REF@[9; 12)
IDENT@[9; 12) "Box"
TYPE_ARG_LIST@[12; 52)
TYPE_ARG_LIST@[12; 37)
L_ANGLE@[12; 13) "<"
TYPE_ARG@[13; 51)
PATH_TYPE@[13; 51)
PATH@[13; 51)
PATH_SEGMENT@[13; 51)
TYPE_ARG@[13; 36)
PATH_TYPE@[13; 36)
PATH@[13; 36)
PATH_SEGMENT@[13; 36)
NAME_REF@[13; 15)
IDENT@[13; 15) "Fn"
PARAM_LIST@[15; 51)
PARAM_LIST@[15; 36)
L_PAREN@[15; 16) "("
PARAM@[16; 22)
BIND_PAT@[16; 17)
NAME@[16; 17)
IDENT@[16; 17) "a"
COLON@[17; 18) ":"
WHITESPACE@[18; 19) " "
PATH_TYPE@[19; 22)
PATH@[19; 22)
PATH_SEGMENT@[19; 22)
NAME_REF@[19; 22)
IDENT@[19; 22) "i32"
COMMA@[22; 23) ","
WHITESPACE@[23; 24) " "
PARAM@[24; 32)
REF_PAT@[24; 26)
AMP@[24; 25) "&"
BIND_PAT@[25; 26)
NAME@[25; 26)
IDENT@[25; 26) "b"
COLON@[26; 27) ":"
WHITESPACE@[27; 28) " "
REFERENCE_TYPE@[28; 32)
AMP@[28; 29) "&"
PATH_TYPE@[29; 32)
PATH@[29; 32)
PATH_SEGMENT@[29; 32)
NAME_REF@[29; 32)
IDENT@[29; 32) "i32"
COMMA@[32; 33) ","
WHITESPACE@[33; 34) " "
PARAM@[34; 46)
REF_PAT@[34; 40)
AMP@[34; 35) "&"
MUT_KW@[35; 38) "mut"
WHITESPACE@[38; 39) " "
BIND_PAT@[39; 40)
NAME@[39; 40)
IDENT@[39; 40) "c"
COLON@[40; 41) ":"
WHITESPACE@[41; 42) " "
REFERENCE_TYPE@[42; 46)
AMP@[42; 43) "&"
PATH_TYPE@[43; 46)
PATH@[43; 46)
PATH_SEGMENT@[43; 46)
NAME_REF@[43; 46)
IDENT@[43; 46) "i32"
COMMA@[46; 47) ","
WHITESPACE@[47; 48) " "
PARAM@[48; 50)
TUPLE_TYPE@[48; 50)
L_PAREN@[48; 49) "("
R_PAREN@[49; 50) ")"
R_PAREN@[50; 51) ")"
R_ANGLE@[51; 52) ">"
SEMI@[52; 53) ";"
WHITESPACE@[53; 54) "\n"
PARAM@[16; 19)
PATH_TYPE@[16; 19)
PATH@[16; 19)
PATH_SEGMENT@[16; 19)
NAME_REF@[16; 19)
IDENT@[16; 19) "i32"
COMMA@[19; 20) ","
WHITESPACE@[20; 21) " "
PARAM@[21; 25)
REFERENCE_TYPE@[21; 25)
AMP@[21; 22) "&"
PATH_TYPE@[22; 25)
PATH@[22; 25)
PATH_SEGMENT@[22; 25)
NAME_REF@[22; 25)
IDENT@[22; 25) "i32"
COMMA@[25; 26) ","
WHITESPACE@[26; 27) " "
PARAM@[27; 31)
REFERENCE_TYPE@[27; 31)
AMP@[27; 28) "&"
PATH_TYPE@[28; 31)
PATH@[28; 31)
PATH_SEGMENT@[28; 31)
NAME_REF@[28; 31)
IDENT@[28; 31) "i32"
COMMA@[31; 32) ","
WHITESPACE@[32; 33) " "
PARAM@[33; 35)
TUPLE_TYPE@[33; 35)
L_PAREN@[33; 34) "("
R_PAREN@[34; 35) ")"
R_PAREN@[35; 36) ")"
R_ANGLE@[36; 37) ">"
SEMI@[37; 38) ";"
WHITESPACE@[38; 39) "\n"
Original file line number Diff line number Diff line change
@@ -1 +1 @@
type F = Box<Fn(x: i32) -> ()>;
type F = Box<Fn(i32) -> ()>;
Loading

0 comments on commit 6d6a995

Please sign in to comment.