Skip to content

Add support for trailing comma in function defs. Parser code simplica… #760

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 1 commit into from
Mar 31, 2019
Merged

Add support for trailing comma in function defs. Parser code simplica… #760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 14 additions & 3 deletions parser/src/lexer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,16 @@ where
loop {
match self.next_char() {
Some('\\') => {
if is_raw {
if self.chr0 == Some(quote_char) {
string_content.push(quote_char);
self.next_char();
} else if is_raw {
string_content.push('\\');
if let Some(c) = self.next_char() {
string_content.push(c)
} else {
return Err(LexicalError::StringError);
}
} else {
match self.next_char() {
Some('\\') => {
Expand Down Expand Up @@ -711,7 +719,6 @@ where
let tok_start = self.get_pos();
self.next_char();
let tok_end = self.get_pos();
println!("Emoji: {}", c);
return Some(Ok((
tok_start,
Tok::Name {
Expand Down Expand Up @@ -1438,7 +1445,7 @@ mod tests {

#[test]
fn test_string() {
let source = String::from(r#""double" 'single' 'can\'t' "\\\"" '\t\r\n' '\g'"#);
let source = String::from(r#""double" 'single' 'can\'t' "\\\"" '\t\r\n' '\g' r'raw\''"#);
let tokens = lex_source(&source);
assert_eq!(
tokens,
Expand Down Expand Up @@ -1467,6 +1474,10 @@ mod tests {
value: String::from("\\g"),
is_fstring: false,
},
Tok::String {
value: String::from("raw\'"),
is_fstring: false,
},
]
);
}
Expand Down
72 changes: 23 additions & 49 deletions parser/src/python.lalrpop
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,7 @@ CompoundStatement: ast::LocatedStatement = {
IfStatement: ast::LocatedStatement = {
<loc:@L> "if" <t:Test> ":" <s1:Suite> <s2:(@L "elif" Test ":" Suite)*> <s3:("else" ":" Suite)?> => {
// Determine last else:
let mut last = match s3 {
Some(s) => Some(s.2),
None => None,
};
let mut last = s3.map(|s| s.2);

// handle elif:
for i in s2.into_iter().rev() {
Expand All @@ -360,10 +357,7 @@ IfStatement: ast::LocatedStatement = {

WhileStatement: ast::LocatedStatement = {
<loc:@L> "while" <e:Test> ":" <s:Suite> <s2:("else" ":" Suite)?> => {
let or_else = match s2 {
Some(s) => Some(s.2),
None => None,
};
let or_else = s2.map(|s| s.2);
ast::LocatedStatement {
location: loc,
node: ast::Statement::While { test: e, body: s, orelse: or_else },
Expand All @@ -373,10 +367,7 @@ WhileStatement: ast::LocatedStatement = {

ForStatement: ast::LocatedStatement = {
<loc:@L> "for" <e:ExpressionList> "in" <t:TestList> ":" <s:Suite> <s2:("else" ":" Suite)?> => {
let or_else = match s2 {
Some(s) => Some(s.2),
None => None,
};
let or_else = s2.map(|s| s.2);
ast::LocatedStatement {
location: loc,
node: ast::Statement::For {
Expand All @@ -389,14 +380,8 @@ ForStatement: ast::LocatedStatement = {

TryStatement: ast::LocatedStatement = {
<loc:@L> "try" ":" <body:Suite> <handlers:ExceptClause*> <else_suite:("else" ":" Suite)?> <finally:("finally" ":" Suite)?> => {
let or_else = match else_suite {
Some(s) => Some(s.2),
None => None,
};
let finalbody = match finally {
Some(s) => Some(s.2),
None => None,
};
let or_else = else_suite.map(|s| s.2);
let finalbody = finally.map(|s| s.2);
ast::LocatedStatement {
location: loc,
node: ast::Statement::Try {
Expand Down Expand Up @@ -437,10 +422,7 @@ WithStatement: ast::LocatedStatement = {

WithItem: ast::WithItem = {
<t:Test> <n:("as" Expression)?> => {
let optional_vars = match n {
Some(val) => Some(val.1),
None => None,
};
let optional_vars = n.map(|val| val.1);
ast::WithItem { context_expr: t, optional_vars }
},
};
Expand All @@ -461,27 +443,19 @@ FuncDef: ast::LocatedStatement = {
};

Parameters: ast::Parameters = {
"(" <a: (TypedArgsList<TypedParameter>)?> ")" => {
match a {
Some(a) => a,
None => Default::default(),
}
"(" <a: (ParameterList<TypedParameter>)?> ")" => {
a.unwrap_or_else(Default::default)
},
};

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

// Now gather rest of parameters:
let (vararg, kwonlyargs, kw_defaults, kwarg) = match args2 {
Some((_, x)) => x,
None => (None, vec![], vec![], None),
};
let (vararg, kwonlyargs, kw_defaults, kwarg) = args2.map_or((None, vec![], vec![], None), |x| x.1);

ast::Parameters {
args: names,
Expand All @@ -492,7 +466,7 @@ TypedArgsList<ArgType>: ast::Parameters = {
kw_defaults: kw_defaults,
}
},
<param1:TypedParameters<ArgType>> <kw:("," KwargParameter<ArgType>)> => {
<param1:ParameterDefs<ArgType>> <kw:("," KwargParameter<ArgType>)> ","? => {
let (names, default_elements) = param1;

// Now gather rest of parameters:
Expand All @@ -510,7 +484,7 @@ TypedArgsList<ArgType>: ast::Parameters = {
kw_defaults: kw_defaults,
}
},
<params:ParameterListStarArgs<ArgType>> => {
<params:ParameterListStarArgs<ArgType>> ","? => {
let (vararg, kwonlyargs, kw_defaults, kwarg) = params;
ast::Parameters {
args: vec![],
Expand All @@ -521,7 +495,7 @@ TypedArgsList<ArgType>: ast::Parameters = {
kw_defaults: kw_defaults,
}
},
<kw:KwargParameter<ArgType>> => {
<kw:KwargParameter<ArgType>> ","? => {
ast::Parameters {
args: vec![],
kwonlyargs: vec![],
Expand All @@ -535,8 +509,8 @@ TypedArgsList<ArgType>: ast::Parameters = {

// Use inline here to make sure the "," is not creating an ambiguity.
#[inline]
TypedParameters<ArgType>: (Vec<ast::Parameter>, Vec<ast::Expression>) = {
<param1:TypedParameterDef<ArgType>> <param2:("," TypedParameterDef<ArgType>)*> => {
ParameterDefs<ArgType>: (Vec<ast::Parameter>, Vec<ast::Expression>) = {
<param1:ParameterDef<ArgType>> <param2:("," ParameterDef<ArgType>)*> => {
// Combine first parameters:
let mut args = vec![param1];
args.extend(param2.into_iter().map(|x| x.1));
Expand Down Expand Up @@ -564,7 +538,7 @@ TypedParameters<ArgType>: (Vec<ast::Parameter>, Vec<ast::Expression>) = {
}
};

TypedParameterDef<ArgType>: (ast::Parameter, Option<ast::Expression>) = {
ParameterDef<ArgType>: (ast::Parameter, Option<ast::Expression>) = {
<i:ArgType> => (i, None),
<i:ArgType> "=" <e:Test> => (i, Some(e)),
};
Expand All @@ -580,8 +554,11 @@ TypedParameter: ast::Parameter = {
},
};

// Use inline here to make sure the "," is not creating an ambiguity.
// TODO: figure out another grammar that makes this inline no longer required.
#[inline]
ParameterListStarArgs<ArgType>: (Option<Option<ast::Parameter>>, Vec<ast::Parameter>, Vec<Option<ast::Expression>>, Option<Option<ast::Parameter>>) = {
"*" <va:ArgType?> <kw:("," TypedParameterDef<ArgType>)*> <kwarg:("," KwargParameter<ArgType>)?> => {
"*" <va:ArgType?> <kw:("," ParameterDef<ArgType>)*> <kwarg:("," KwargParameter<ArgType>)?> => {
// Extract keyword arguments:
let mut kwonlyargs = vec![];
let mut kw_defaults = vec![];
Expand All @@ -590,10 +567,7 @@ ParameterListStarArgs<ArgType>: (Option<Option<ast::Parameter>>, Vec<ast::Parame
kw_defaults.push(value);
}

let kwarg = match kwarg {
Some((_, name)) => Some(name),
None => None,
};
let kwarg = kwarg.map(|n| n.1);

(Some(va), kwonlyargs, kw_defaults, kwarg)
}
Expand Down Expand Up @@ -684,7 +658,7 @@ Test: ast::Expression = {
};

LambdaDef: ast::Expression = {
"lambda" <p:TypedArgsList<UntypedParameter>?> ":" <b:Test> =>
"lambda" <p:ParameterList<UntypedParameter>?> ":" <b:Test> =>
ast::Expression::Lambda {
args: p.unwrap_or(Default::default()),
body:Box::new(b)
Expand Down
5 changes: 5 additions & 0 deletions tests/snippets/function.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@ def foo():
return 42

assert foo() == 42

def my_func(a,):
return a+2

assert my_func(2) == 4