From 62fd967cd4c4f83fddfa26f7f228542434725519 Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Thu, 29 Dec 2022 01:12:30 +0800 Subject: [PATCH 1/7] added lex error: DuplicateArguments --- compiler/parser/python.lalrpop | 55 +++++++++++++++++++-------------- compiler/parser/src/error.rs | 4 +++ compiler/parser/src/function.rs | 33 ++++++++++++++++++++ 3 files changed, 68 insertions(+), 24 deletions(-) diff --git a/compiler/parser/python.lalrpop b/compiler/parser/python.lalrpop index a73d83520..70ad18f7b 100644 --- a/compiler/parser/python.lalrpop +++ b/compiler/parser/python.lalrpop @@ -6,7 +6,7 @@ use crate::{ ast, error::{LexicalError, LexicalErrorType}, - function::{ArgumentList, parse_args, parse_params}, + function::{ArgumentList, parse_args, parse_params, validate_arguments}, lexer, context::set_context, string::parse_strings, @@ -552,16 +552,20 @@ FuncDef: ast::Stmt = { }; Parameters: ast::Arguments = { - "(" )?> ")" => { - a.unwrap_or_else(|| ast::Arguments { - posonlyargs: vec![], - args: vec![], - vararg: None, - kwonlyargs: vec![], - kw_defaults: vec![], - kwarg: None, - defaults: vec![] - }) + "(" )?> ")" =>? { + let args = validate_arguments( + a.unwrap_or_else(|| ast::Arguments { + posonlyargs: vec![], + args: vec![], + vararg: None, + kwonlyargs: vec![], + kw_defaults: vec![], + kwarg: None, + defaults: vec![] + }) + )?; + + Ok(args) } }; @@ -774,19 +778,22 @@ NamedExpression: ast::Expr = { }; LambdaDef: ast::Expr = { - "lambda" ?> ":" > => { - let p = p.unwrap_or_else(|| { - ast::Arguments { - posonlyargs: vec![], - args: vec![], - vararg: None, - kwonlyargs: vec![], - kw_defaults: vec![], - kwarg: None, - defaults: vec![] + "lambda" ?> ":" > =>? { + let p = validate_arguments( + p.unwrap_or_else(|| { + ast::Arguments { + posonlyargs: vec![], + args: vec![], + vararg: None, + kwonlyargs: vec![], + kw_defaults: vec![], + kwarg: None, + defaults: vec![] + } } - }); - ast::Expr { + ))?; + + Ok(ast::Expr { location, end_location: Some(end_location), custom: (), @@ -794,7 +801,7 @@ LambdaDef: ast::Expr = { args: Box::new(p), body: Box::new(body) } - } + }) } } diff --git a/compiler/parser/src/error.rs b/compiler/parser/src/error.rs index 47096a544..b9d7cbfe0 100644 --- a/compiler/parser/src/error.rs +++ b/compiler/parser/src/error.rs @@ -21,6 +21,7 @@ pub enum LexicalErrorType { TabError, TabsAfterSpaces, DefaultArgumentError, + DuplicateArgumentError, PositionalArgumentError, UnpackedArgumentError, DuplicateKeywordArgumentError, @@ -50,6 +51,9 @@ impl fmt::Display for LexicalErrorType { LexicalErrorType::DefaultArgumentError => { write!(f, "non-default argument follows default argument") } + LexicalErrorType::DuplicateArgumentError => { + write!(f, "duplicate argument in function definition") + } LexicalErrorType::DuplicateKeywordArgumentError => { write!(f, "keyword argument repeated") } diff --git a/compiler/parser/src/function.rs b/compiler/parser/src/function.rs index 466211304..adbb269e1 100644 --- a/compiler/parser/src/function.rs +++ b/compiler/parser/src/function.rs @@ -10,6 +10,39 @@ pub struct ArgumentList { type ParameterDefs = (Vec, Vec, Vec); type ParameterDef = (ast::Arg, Option); +pub fn validate_arguments( + arguments: ast::Arguments +) -> Result { + let mut all_args: Vec<&ast::Located> = vec![]; + + all_args.extend(arguments.posonlyargs.iter()); + all_args.extend(arguments.args.iter()); + + if let Some(a) = &arguments.vararg { + all_args.push(a); + } + + all_args.extend(arguments.kwonlyargs.iter()); + + if let Some(a) = &arguments.kwarg { + all_args.push(a); + } + + let mut all_arg_names = + FxHashSet::with_hasher(Default::default()); + for arg in all_args { + let arg_name = arg.node.arg.clone(); + if !all_arg_names.insert(arg_name) { + return Err(LexicalError { + error: LexicalErrorType::DuplicateArgumentError, + location: arg.location, + }); + } + } + + return Ok(arguments); +} + pub fn parse_params( params: (Vec, Vec), ) -> Result { From 0e66e143fdb6b019d2cc8334cf7c5fb4a1f042ea Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Thu, 29 Dec 2022 02:16:44 +0800 Subject: [PATCH 2/7] added check: named arguments must follow bare star --- compiler/parser/python.lalrpop | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/compiler/parser/python.lalrpop b/compiler/parser/python.lalrpop index 70ad18f7b..7683b606c 100644 --- a/compiler/parser/python.lalrpop +++ b/compiler/parser/python.lalrpop @@ -667,7 +667,7 @@ TypedParameter: ast::Arg = { // TODO: figure out another grammar that makes this inline no longer required. #[inline] ParameterListStarArgs: (Option>, Vec, Vec, Option>) = { - "*" )*> )?> => { + "*" )*> )?> =>? { // Extract keyword arguments: let mut kwonlyargs = Vec::new(); let mut kw_defaults = Vec::new(); @@ -685,7 +685,14 @@ ParameterListStarArgs: (Option>, Vec, Vec Date: Thu, 29 Dec 2022 21:39:38 +0800 Subject: [PATCH 3/7] add arg_name in duplicate argument error msg --- compiler/parser/src/error.rs | 6 +++--- compiler/parser/src/function.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/parser/src/error.rs b/compiler/parser/src/error.rs index b9d7cbfe0..4f7b101f0 100644 --- a/compiler/parser/src/error.rs +++ b/compiler/parser/src/error.rs @@ -21,7 +21,7 @@ pub enum LexicalErrorType { TabError, TabsAfterSpaces, DefaultArgumentError, - DuplicateArgumentError, + DuplicateArgumentError(String), PositionalArgumentError, UnpackedArgumentError, DuplicateKeywordArgumentError, @@ -51,8 +51,8 @@ impl fmt::Display for LexicalErrorType { LexicalErrorType::DefaultArgumentError => { write!(f, "non-default argument follows default argument") } - LexicalErrorType::DuplicateArgumentError => { - write!(f, "duplicate argument in function definition") + LexicalErrorType::DuplicateArgumentError(arg_name) => { + write!(f, "duplicate argument '{arg_name}' in function definition") } LexicalErrorType::DuplicateKeywordArgumentError => { write!(f, "keyword argument repeated") diff --git a/compiler/parser/src/function.rs b/compiler/parser/src/function.rs index adbb269e1..91f967dfc 100644 --- a/compiler/parser/src/function.rs +++ b/compiler/parser/src/function.rs @@ -31,16 +31,16 @@ pub fn validate_arguments( let mut all_arg_names = FxHashSet::with_hasher(Default::default()); for arg in all_args { - let arg_name = arg.node.arg.clone(); + let arg_name = &arg.node.arg; if !all_arg_names.insert(arg_name) { return Err(LexicalError { - error: LexicalErrorType::DuplicateArgumentError, + error: LexicalErrorType::DuplicateArgumentError(arg_name.to_string()), location: arg.location, }); } } - return Ok(arguments); + Ok(arguments) } pub fn parse_params( From c7c9602ea44e577728ac40526c6585b946b8f1ee Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Thu, 29 Dec 2022 21:39:57 +0800 Subject: [PATCH 4/7] use is_none --- compiler/parser/python.lalrpop | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/parser/python.lalrpop b/compiler/parser/python.lalrpop index 7683b606c..5652be1b6 100644 --- a/compiler/parser/python.lalrpop +++ b/compiler/parser/python.lalrpop @@ -682,16 +682,16 @@ ParameterListStarArgs: (Option>, Vec, Vec Date: Thu, 29 Dec 2022 22:00:35 +0800 Subject: [PATCH 5/7] remove expectedFailure decorator in test_argument_handling --- Lib/test/test_compile.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index 3242a4ec6..fb3cbb8b7 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -42,8 +42,6 @@ class TestSpecifics(unittest.TestCase): self.assertEqual(__debug__, prev) setattr(builtins, '__debug__', prev) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_argument_handling(self): # detect duplicate positional and keyword arguments self.assertRaises(SyntaxError, eval, 'lambda a,a:0') From 2d5e0449933d98c0fd44fb0e52a2ce6fe5254b9a Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Thu, 29 Dec 2022 22:42:45 +0800 Subject: [PATCH 6/7] format code --- compiler/parser/src/function.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/parser/src/function.rs b/compiler/parser/src/function.rs index 91f967dfc..1caa4598e 100644 --- a/compiler/parser/src/function.rs +++ b/compiler/parser/src/function.rs @@ -10,9 +10,7 @@ pub struct ArgumentList { type ParameterDefs = (Vec, Vec, Vec); type ParameterDef = (ast::Arg, Option); -pub fn validate_arguments( - arguments: ast::Arguments -) -> Result { +pub fn validate_arguments(arguments: ast::Arguments) -> Result { let mut all_args: Vec<&ast::Located> = vec![]; all_args.extend(arguments.posonlyargs.iter()); @@ -28,8 +26,7 @@ pub fn validate_arguments( all_args.push(a); } - let mut all_arg_names = - FxHashSet::with_hasher(Default::default()); + let mut all_arg_names = FxHashSet::with_hasher(Default::default()); for arg in all_args { let arg_name = &arg.node.arg; if !all_arg_names.insert(arg_name) { From 7e3c59d21d2149b83db32d630d5e42f9ea6d7eda Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Thu, 29 Dec 2022 22:48:34 +0800 Subject: [PATCH 7/7] remove expectedFailure decorators --- Lib/test/test_positional_only_arg.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Lib/test/test_positional_only_arg.py b/Lib/test/test_positional_only_arg.py index 76a5df2a3..d9d8b0306 100644 --- a/Lib/test/test_positional_only_arg.py +++ b/Lib/test/test_positional_only_arg.py @@ -22,8 +22,6 @@ class PositionalOnlyTestCase(unittest.TestCase): with self.assertRaisesRegex(SyntaxError, regex): compile(codestr + "\n", "", "single") - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_invalid_syntax_errors(self): check_syntax_error(self, "def f(a, b = 5, /, c): pass", "non-default argument follows default argument") check_syntax_error(self, "def f(a = 5, b, /, c): pass", "non-default argument follows default argument") @@ -45,8 +43,6 @@ class PositionalOnlyTestCase(unittest.TestCase): check_syntax_error(self, "def f(a, /, c, /, d, *, e): pass") check_syntax_error(self, "def f(a, *, c, /, d, e): pass") - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_invalid_syntax_errors_async(self): check_syntax_error(self, "async def f(a, b = 5, /, c): pass", "non-default argument follows default argument") check_syntax_error(self, "async def f(a = 5, b, /, c): pass", "non-default argument follows default argument") @@ -240,8 +236,6 @@ class PositionalOnlyTestCase(unittest.TestCase): x = lambda a, b, /, : a + b self.assertEqual(x(1, 2), 3) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_invalid_syntax_lambda(self): check_syntax_error(self, "lambda a, b = 5, /, c: None", "non-default argument follows default argument") check_syntax_error(self, "lambda a = 5, b, /, c: None", "non-default argument follows default argument")