Skip to content

Commit 69c8173

Browse files
windelbouwmanwindelbouwman
authored
Merge pull request RustPython#760 from RustPython/parser-fixes
Add support for trailing comma in function defs. Parser code simplica…
2 parents f1a60e1 + 73cd680 commit 69c8173

3 files changed

Lines changed: 42 additions & 52 deletions

File tree

‎parser/src/lexer.rs‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -466,8 +466,16 @@ where
466466
loop {
467467
match self.next_char() {
468468
Some('\\') => {
469-
if is_raw {
469+
if self.chr0 == Some(quote_char) {
470+
string_content.push(quote_char);
471+
self.next_char();
472+
} else if is_raw {
470473
string_content.push('\\');
474+
if let Some(c) = self.next_char() {
475+
string_content.push(c)
476+
} else {
477+
return Err(LexicalError::StringError);
478+
}
471479
} else {
472480
match self.next_char() {
473481
Some('\\') => {
@@ -711,7 +719,6 @@ where
711719
let tok_start = self.get_pos();
712720
self.next_char();
713721
let tok_end = self.get_pos();
714-
println!("Emoji: {}", c);
715722
return Some(Ok((
716723
tok_start,
717724
Tok::Name {
@@ -1438,7 +1445,7 @@ mod tests {
14381445

14391446
#[test]
14401447
fn test_string() {
1441-
let source = String::from(r#""double" 'single' 'can\'t' "\\\"" '\t\r\n' '\g'"#);
1448+
let source = String::from(r#""double" 'single' 'can\'t' "\\\"" '\t\r\n' '\g' r'raw\''"#);
14421449
let tokens = lex_source(&source);
14431450
assert_eq!(
14441451
tokens,
@@ -1467,6 +1474,10 @@ mod tests {
14671474
value: String::from("\\g"),
14681475
is_fstring: false,
14691476
},
1477+
Tok::String {
1478+
value: String::from("raw\'"),
1479+
is_fstring: false,
1480+
},
14701481
]
14711482
);
14721483
}

‎parser/src/python.lalrpop‎

Lines changed: 23 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,7 @@ CompoundStatement: ast::LocatedStatement = {
336336
IfStatement: ast::LocatedStatement = {
337337
<loc:@L> "if" <t:Test> ":" <s1:Suite> <s2:(@L "elif" Test ":" Suite)*> <s3:("else" ":" Suite)?> => {
338338
// Determine last else:
339-
let mut last = match s3 {
340-
Some(s) => Some(s.2),
341-
None => None,
342-
};
339+
let mut last = s3.map(|s| s.2);
343340

344341
// handle elif:
345342
for i in s2.into_iter().rev() {
@@ -359,10 +356,7 @@ IfStatement: ast::LocatedStatement = {
359356

360357
WhileStatement: ast::LocatedStatement = {
361358
<loc:@L> "while" <e:Test> ":" <s:Suite> <s2:("else" ":" Suite)?> => {
362-
let or_else = match s2 {
363-
Some(s) => Some(s.2),
364-
None => None,
365-
};
359+
let or_else = s2.map(|s| s.2);
366360
ast::LocatedStatement {
367361
location: loc,
368362
node: ast::Statement::While { test: e, body: s, orelse: or_else },
@@ -372,10 +366,7 @@ WhileStatement: ast::LocatedStatement = {
372366

373367
ForStatement: ast::LocatedStatement = {
374368
<loc:@L> "for" <e:ExpressionList> "in" <t:TestList> ":" <s:Suite> <s2:("else" ":" Suite)?> => {
375-
let or_else = match s2 {
376-
Some(s) => Some(s.2),
377-
None => None,
378-
};
369+
let or_else = s2.map(|s| s.2);
379370
ast::LocatedStatement {
380371
location: loc,
381372
node: ast::Statement::For {
@@ -388,14 +379,8 @@ ForStatement: ast::LocatedStatement = {
388379

389380
TryStatement: ast::LocatedStatement = {
390381
<loc:@L> "try" ":" <body:Suite> <handlers:ExceptClause*> <else_suite:("else" ":" Suite)?> <finally:("finally" ":" Suite)?> => {
391-
let or_else = match else_suite {
392-
Some(s) => Some(s.2),
393-
None => None,
394-
};
395-
let finalbody = match finally {
396-
Some(s) => Some(s.2),
397-
None => None,
398-
};
382+
let or_else = else_suite.map(|s| s.2);
383+
let finalbody = finally.map(|s| s.2);
399384
ast::LocatedStatement {
400385
location: loc,
401386
node: ast::Statement::Try {
@@ -436,10 +421,7 @@ WithStatement: ast::LocatedStatement = {
436421

437422
WithItem: ast::WithItem = {
438423
<t:Test> <n:("as" Expression)?> => {
439-
let optional_vars = match n {
440-
Some(val) => Some(val.1),
441-
None => None,
442-
};
424+
let optional_vars = n.map(|val| val.1);
443425
ast::WithItem { context_expr: t, optional_vars }
444426
},
445427
};
@@ -460,27 +442,19 @@ FuncDef: ast::LocatedStatement = {
460442
};
461443

462444
Parameters: ast::Parameters = {
463-
"(" <a: (TypedArgsList<TypedParameter>)?> ")" => {
464-
match a {
465-
Some(a) => a,
466-
None => Default::default(),
467-
}
445+
"(" <a: (ParameterList<TypedParameter>)?> ")" => {
446+
a.unwrap_or_else(Default::default)
468447
},
469448
};
470449

471-
// parameters are (String, None), kwargs are (String, Some(Test)) where Test is
472-
// the default
473450
// Note that this is a macro which is used once for function defs, and
474451
// once for lambda defs.
475-
TypedArgsList<ArgType>: ast::Parameters = {
476-
<param1:TypedParameters<ArgType>> <args2:("," ParameterListStarArgs<ArgType>)?> => {
452+
ParameterList<ArgType>: ast::Parameters = {
453+
<param1:ParameterDefs<ArgType>> <args2:("," ParameterListStarArgs<ArgType>)?> ","? => {
477454
let (names, default_elements) = param1;
478455

479456
// Now gather rest of parameters:
480-
let (vararg, kwonlyargs, kw_defaults, kwarg) = match args2 {
481-
Some((_, x)) => x,
482-
None => (None, vec![], vec![], None),
483-
};
457+
let (vararg, kwonlyargs, kw_defaults, kwarg) = args2.map_or((None, vec![], vec![], None), |x| x.1);
484458

485459
ast::Parameters {
486460
args: names,
@@ -491,7 +465,7 @@ TypedArgsList<ArgType>: ast::Parameters = {
491465
kw_defaults: kw_defaults,
492466
}
493467
},
494-
<param1:TypedParameters<ArgType>> <kw:("," KwargParameter<ArgType>)> => {
468+
<param1:ParameterDefs<ArgType>> <kw:("," KwargParameter<ArgType>)> ","? => {
495469
let (names, default_elements) = param1;
496470

497471
// Now gather rest of parameters:
@@ -509,7 +483,7 @@ TypedArgsList<ArgType>: ast::Parameters = {
509483
kw_defaults: kw_defaults,
510484
}
511485
},
512-
<params:ParameterListStarArgs<ArgType>> => {
486+
<params:ParameterListStarArgs<ArgType>> ","? => {
513487
let (vararg, kwonlyargs, kw_defaults, kwarg) = params;
514488
ast::Parameters {
515489
args: vec![],
@@ -520,7 +494,7 @@ TypedArgsList<ArgType>: ast::Parameters = {
520494
kw_defaults: kw_defaults,
521495
}
522496
},
523-
<kw:KwargParameter<ArgType>> => {
497+
<kw:KwargParameter<ArgType>> ","? => {
524498
ast::Parameters {
525499
args: vec![],
526500
kwonlyargs: vec![],
@@ -534,8 +508,8 @@ TypedArgsList<ArgType>: ast::Parameters = {
534508

535509
// Use inline here to make sure the "," is not creating an ambiguity.
536510
#[inline]
537-
TypedParameters<ArgType>: (Vec<ast::Parameter>, Vec<ast::Expression>) = {
538-
<param1:TypedParameterDef<ArgType>> <param2:("," TypedParameterDef<ArgType>)*> => {
511+
ParameterDefs<ArgType>: (Vec<ast::Parameter>, Vec<ast::Expression>) = {
512+
<param1:ParameterDef<ArgType>> <param2:("," ParameterDef<ArgType>)*> => {
539513
// Combine first parameters:
540514
let mut args = vec![param1];
541515
args.extend(param2.into_iter().map(|x| x.1));
@@ -563,7 +537,7 @@ TypedParameters<ArgType>: (Vec<ast::Parameter>, Vec<ast::Expression>) = {
563537
}
564538
};
565539

566-
TypedParameterDef<ArgType>: (ast::Parameter, Option<ast::Expression>) = {
540+
ParameterDef<ArgType>: (ast::Parameter, Option<ast::Expression>) = {
567541
<i:ArgType> => (i, None),
568542
<i:ArgType> "=" <e:Test> => (i, Some(e)),
569543
};
@@ -579,8 +553,11 @@ TypedParameter: ast::Parameter = {
579553
},
580554
};
581555

556+
// Use inline here to make sure the "," is not creating an ambiguity.
557+
// TODO: figure out another grammar that makes this inline no longer required.
558+
#[inline]
582559
ParameterListStarArgs<ArgType>: (Option<Option<ast::Parameter>>, Vec<ast::Parameter>, Vec<Option<ast::Expression>>, Option<Option<ast::Parameter>>) = {
583-
"*" <va:ArgType?> <kw:("," TypedParameterDef<ArgType>)*> <kwarg:("," KwargParameter<ArgType>)?> => {
560+
"*" <va:ArgType?> <kw:("," ParameterDef<ArgType>)*> <kwarg:("," KwargParameter<ArgType>)?> => {
584561
// Extract keyword arguments:
585562
let mut kwonlyargs = vec![];
586563
let mut kw_defaults = vec![];
@@ -589,10 +566,7 @@ ParameterListStarArgs<ArgType>: (Option<Option<ast::Parameter>>, Vec<ast::Parame
589566
kw_defaults.push(value);
590567
}
591568

592-
let kwarg = match kwarg {
593-
Some((_, name)) => Some(name),
594-
None => None,
595-
};
569+
let kwarg = kwarg.map(|n| n.1);
596570

597571
(Some(va), kwonlyargs, kw_defaults, kwarg)
598572
}
@@ -683,7 +657,7 @@ Test: ast::Expression = {
683657
};
684658

685659
LambdaDef: ast::Expression = {
686-
"lambda" <p:TypedArgsList<UntypedParameter>?> ":" <b:Test> =>
660+
"lambda" <p:ParameterList<UntypedParameter>?> ":" <b:Test> =>
687661
ast::Expression::Lambda {
688662
args: p.unwrap_or(Default::default()),
689663
body:Box::new(b)

‎tests/snippets/function.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,8 @@ def foo():
22
return 42
33

44
assert foo() == 42
5+
6+
def my_func(a,):
7+
return a+2
8+
9+
assert my_func(2) == 4

0 commit comments

Comments
 (0)