Skip to content

Rework value parameter parsing #3040

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 7, 2020
Merged

Rework value parameter parsing #3040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e66785
Rework value parameter parsing
tobz1000 Feb 6, 2020
e1921ea
rustfmt
tobz1000 Feb 6, 2020
0183952
Closure params test
tobz1000 Feb 6, 2020
90ff2be
PR tweaks
tobz1000 Feb 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/ra_parser/src/grammar/expressions/atom.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion crates/ra_syntax/test_data/parser/inline/ok/0004_value_parameters_no_patterns.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
type F = Box<Fn(a: i32, &b: &i32, &mut c: &i32, ())>;
Copy link
Contributor

@matklad matklad Feb 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, this was never allowed in the first place? 😅

Copy link
Member

@bjorn3 bjorn3 Feb 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some code to test all code which should compile with rustc too?

Copy link
Contributor Author

@tobz1000 tobz1000 Feb 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could be a good catch-all, but I think a lot of existing tests won't compile due to undefined symbols, rather than syntax errors.

type F = Box<Fn(i32, &i32, &i32, ())>;
115 changes: 47 additions & 68 deletions crates/ra_syntax/test_data/parser/inline/ok/0004_value_parameters_no_patterns.txt
View file
Open in desktop
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"
2 changes: 1 addition & 1 deletion crates/ra_syntax/test_data/parser/inline/ok/0104_path_fn_trait_args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
type F = Box<Fn(x: i32) -> ()>;
type F = Box<Fn(i32) -> ()>;
Loading