From 581f6e176cf47102cfd4febe77105485a7a37c8f Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 15 Jan 2023 13:01:59 +0900 Subject: [PATCH 1/7] Fix dict spreading in dict literal --- ast/src/ast_gen.rs | 2 +- parser/python.lalrpop | 72 ++++++++--------- parser/src/parser.rs | 6 ++ ...parser__tests__dict_containing_spread.snap | 81 +++++++++++++++++++ 4 files changed, 124 insertions(+), 37 deletions(-) create mode 100644 parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap diff --git a/ast/src/ast_gen.rs b/ast/src/ast_gen.rs index 241ebe135a..32dd89c2c8 100644 --- a/ast/src/ast_gen.rs +++ b/ast/src/ast_gen.rs @@ -195,7 +195,7 @@ pub enum ExprKind { orelse: Box>, }, Dict { - keys: Vec>, + keys: Vec>>, values: Vec>, }, Set { diff --git a/parser/python.lalrpop b/parser/python.lalrpop index 90f4ce2582..7007905779 100644 --- a/parser/python.lalrpop +++ b/parser/python.lalrpop @@ -1138,42 +1138,42 @@ Atom: ast::Expr = { "{" "}" => { let pairs = e.unwrap_or_default(); - let (keys, values) = match pairs.iter().position(|(k,_)| k.is_none()) { - Some(unpack_idx) => { - let mut pairs = pairs; - let (keys, mut values): (_, Vec<_>) = pairs.drain(..unpack_idx).map(|(k, v)| (*k.unwrap(), v)).unzip(); - - fn build_map(items: &mut Vec<(ast::Expr, ast::Expr)>) -> ast::Expr { - let location = items[0].0.location; - let end_location = items[0].0.end_location; - let (keys, values) = items.drain(..).unzip(); - ast::Expr { - location, - end_location, - custom: (), - node: ast::ExprKind::Dict { keys, values } - } - } - - let mut items = Vec::new(); - for (key, value) in pairs.into_iter() { - if let Some(key) = key { - items.push((*key, value)); - continue; - } - if !items.is_empty() { - values.push(build_map(&mut items)); - } - values.push(value); - } - if !items.is_empty() { - values.push(build_map(&mut items)); - } - (keys, values) - }, - None => pairs.into_iter().map(|(k, v)| (*k.unwrap(), v)).unzip() - }; - + // let (keys, values) = match pairs.iter().position(|(k,_)| k.is_none()) { + // Some(unpack_idx) => { + // let mut pairs = pairs; + // let (keys, mut values): (_, Vec<_>) = pairs.drain(..unpack_idx).map(|(k, v)| (*k.unwrap(), v)).unzip(); + // + // fn build_map(items: &mut Vec<(ast::Expr, ast::Expr)>) -> ast::Expr { + // let location = items[0].0.location; + // let end_location = items[0].0.end_location; + // let (keys, values) = items.drain(..).unzip(); + // ast::Expr { + // location, + // end_location, + // custom: (), + // node: ast::ExprKind::Dict { keys, values } + // } + // } + // + // let mut items = Vec::new(); + // for (key, value) in pairs.into_iter() { + // if let Some(key) = key { + // items.push((*key, value)); + // continue; + // } + // if !items.is_empty() { + // values.push(build_map(&mut items)); + // } + // values.push(value); + // } + // if !items.is_empty() { + // values.push(build_map(&mut items)); + // } + // (keys, values) + // }, + // None => pairs.into_iter().map(|(k, v)| (k, v)).unzip() + // }; + let (keys, values) = pairs.into_iter().map(|(k, v)| (k.map(|x| *x), v)).unzip(); ast::Expr { location, end_location: Some(end_location), diff --git a/parser/src/parser.rs b/parser/src/parser.rs index fb18155971..ced5d9b887 100644 --- a/parser/src/parser.rs +++ b/parser/src/parser.rs @@ -309,4 +309,10 @@ with (0 as a, 1 as b,): pass assert!(parse_program(source, "").is_err()); } } + + #[test] + fn test_dict_containing_spread() { + let parse_ast = parse_expression(r#"{"k": "v", **d}"#, "").unwrap(); + insta::assert_debug_snapshot!(parse_ast); + } } diff --git a/parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap b/parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap new file mode 100644 index 0000000000..29eb084867 --- /dev/null +++ b/parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap @@ -0,0 +1,81 @@ +--- +source: compiler/parser/src/parser.rs +expression: parse_ast +--- +Located { + location: Location { + row: 1, + column: 0, + }, + end_location: Some( + Location { + row: 1, + column: 15, + }, + ), + custom: (), + node: Dict { + keys: [ + Some( + Located { + location: Location { + row: 1, + column: 1, + }, + end_location: Some( + Location { + row: 1, + column: 4, + }, + ), + custom: (), + node: Constant { + value: Str( + "k", + ), + kind: None, + }, + }, + ), + None, + ], + values: [ + Located { + location: Location { + row: 1, + column: 6, + }, + end_location: Some( + Location { + row: 1, + column: 9, + }, + ), + custom: (), + node: Constant { + value: Str( + "v", + ), + kind: None, + }, + }, + Located { + location: Location { + row: 1, + column: 13, + }, + end_location: Some( + Location { + row: 1, + column: 14, + }, + ), + custom: (), + node: Name { + id: "d", + ctx: Load, + }, + }, + ], + }, +} From d3c4551629286d5961f2c24987dce0d7987f0c56 Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 15 Jan 2023 13:11:55 +0900 Subject: [PATCH 2/7] Fix unparse --- ast/src/unparse.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ast/src/unparse.rs b/ast/src/unparse.rs index 551283c0b1..b8b3420958 100644 --- a/ast/src/unparse.rs +++ b/ast/src/unparse.rs @@ -152,7 +152,11 @@ impl<'a> Unparser<'a> { let (packed, unpacked) = values.split_at(keys.len()); for (k, v) in keys.iter().zip(packed) { self.p_delim(&mut first, ", ")?; - write!(self, "{}: {}", *k, *v)?; + if let Some(k) = k { + write!(self, "{}: {}", *k, *v)?; + } else { + write!(self, "**{}", *v)?; + } } for d in unpacked { self.p_delim(&mut first, ", ")?; From 4edd2bf78a92e4e3313f52d5870c9532ece4bd0b Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 15 Jan 2023 16:10:51 +0900 Subject: [PATCH 3/7] Remove commented-out code --- parser/python.lalrpop | 43 +++++-------------------------------------- 1 file changed, 5 insertions(+), 38 deletions(-) diff --git a/parser/python.lalrpop b/parser/python.lalrpop index 7007905779..df5057c5c0 100644 --- a/parser/python.lalrpop +++ b/parser/python.lalrpop @@ -1136,44 +1136,11 @@ Atom: ast::Expr = { }.into()) }, "{" "}" => { - let pairs = e.unwrap_or_default(); - - // let (keys, values) = match pairs.iter().position(|(k,_)| k.is_none()) { - // Some(unpack_idx) => { - // let mut pairs = pairs; - // let (keys, mut values): (_, Vec<_>) = pairs.drain(..unpack_idx).map(|(k, v)| (*k.unwrap(), v)).unzip(); - // - // fn build_map(items: &mut Vec<(ast::Expr, ast::Expr)>) -> ast::Expr { - // let location = items[0].0.location; - // let end_location = items[0].0.end_location; - // let (keys, values) = items.drain(..).unzip(); - // ast::Expr { - // location, - // end_location, - // custom: (), - // node: ast::ExprKind::Dict { keys, values } - // } - // } - // - // let mut items = Vec::new(); - // for (key, value) in pairs.into_iter() { - // if let Some(key) = key { - // items.push((*key, value)); - // continue; - // } - // if !items.is_empty() { - // values.push(build_map(&mut items)); - // } - // values.push(value); - // } - // if !items.is_empty() { - // values.push(build_map(&mut items)); - // } - // (keys, values) - // }, - // None => pairs.into_iter().map(|(k, v)| (k, v)).unzip() - // }; - let (keys, values) = pairs.into_iter().map(|(k, v)| (k.map(|x| *x), v)).unzip(); + let (keys, values) = e + .unwrap_or_default() + .into_iter() + .map(|(k, v)| (k.map(|x| *x), v)) + .unzip(); ast::Expr { location, end_location: Some(end_location), From 393869c47c6fc373e41559d0c3e5beb282526263 Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 15 Jan 2023 16:43:13 +0900 Subject: [PATCH 4/7] Add Option to Dict.keys field --- ast/asdl_rs.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ast/asdl_rs.py b/ast/asdl_rs.py index ed5800f5de..a18ec651e5 100755 --- a/ast/asdl_rs.py +++ b/ast/asdl_rs.py @@ -227,12 +227,12 @@ class StructVisitor(TypeInfoEmitVisitor): if cons.fields: self.emit(f"{cons.name} {{", depth) for f in cons.fields: - self.visit(f, parent, "", depth + 1) + self.visit(f, parent, "", depth + 1, cons.name) self.emit("},", depth) else: self.emit(f"{cons.name},", depth) - def visitField(self, field, parent, vis, depth): + def visitField(self, field, parent, vis, depth, constructor=None): typ = get_rust_type(field.type) fieldtype = self.typeinfo.get(field.type) if fieldtype and fieldtype.has_userdata: @@ -240,7 +240,11 @@ class StructVisitor(TypeInfoEmitVisitor): # don't box if we're doing Vec, but do box if we're doing Vec>> if fieldtype and fieldtype.boxed and (not (parent.product or field.seq) or field.opt): typ = f"Box<{typ}>" - if field.opt: + if field.opt or ( + # Add `Option` to allow `Dict.keys` to contain `None` for dictionary unpacking + # in a dict literal. + constructor == "Dict" and field.name == "keys" + ): typ = f"Option<{typ}>" if field.seq: typ = f"Vec<{typ}>" From d5fc7c4c873233f2caa6793396ec551fb4c0040f Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 15 Jan 2023 16:53:13 +0900 Subject: [PATCH 5/7] Improve test --- parser/src/parser.rs | 2 +- ...parser__tests__dict_containing_spread.snap | 48 +++++++++++++++++-- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/parser/src/parser.rs b/parser/src/parser.rs index ced5d9b887..41945f283e 100644 --- a/parser/src/parser.rs +++ b/parser/src/parser.rs @@ -312,7 +312,7 @@ with (0 as a, 1 as b,): pass #[test] fn test_dict_containing_spread() { - let parse_ast = parse_expression(r#"{"k": "v", **d}"#, "").unwrap(); + let parse_ast = parse_expression(r#"{"a": "b", **c, "d": "e"}"#, "").unwrap(); insta::assert_debug_snapshot!(parse_ast); } } diff --git a/parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap b/parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap index 29eb084867..212f737ee3 100644 --- a/parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap +++ b/parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap @@ -10,7 +10,7 @@ Located { end_location: Some( Location { row: 1, - column: 15, + column: 25, }, ), custom: (), @@ -31,13 +31,34 @@ Located { custom: (), node: Constant { value: Str( - "k", + "a", ), kind: None, }, }, ), None, + Some( + Located { + location: Location { + row: 1, + column: 16, + }, + end_location: Some( + Location { + row: 1, + column: 19, + }, + ), + custom: (), + node: Constant { + value: Str( + "d", + ), + kind: None, + }, + }, + ), ], values: [ Located { @@ -54,7 +75,7 @@ Located { custom: (), node: Constant { value: Str( - "v", + "b", ), kind: None, }, @@ -72,10 +93,29 @@ Located { ), custom: (), node: Name { - id: "d", + id: "c", ctx: Load, }, }, + Located { + location: Location { + row: 1, + column: 21, + }, + end_location: Some( + Location { + row: 1, + column: 24, + }, + ), + custom: (), + node: Constant { + value: Str( + "e", + ), + kind: None, + }, + }, ], }, } From 2d019930e9dba99473517ddf27012943c9092ab1 Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 15 Jan 2023 23:36:07 +0900 Subject: [PATCH 6/7] Rename test --- parser/src/parser.rs | 2 +- ...ap => rustpython_parser__parser__tests__dict_unpacking.snap} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename parser/src/snapshots/{rustpython_parser__parser__tests__dict_containing_spread.snap => rustpython_parser__parser__tests__dict_unpacking.snap} (100%) diff --git a/parser/src/parser.rs b/parser/src/parser.rs index 41945f283e..9761e9e167 100644 --- a/parser/src/parser.rs +++ b/parser/src/parser.rs @@ -311,7 +311,7 @@ with (0 as a, 1 as b,): pass } #[test] - fn test_dict_containing_spread() { + fn test_dict_unpacking() { let parse_ast = parse_expression(r#"{"a": "b", **c, "d": "e"}"#, "").unwrap(); insta::assert_debug_snapshot!(parse_ast); } diff --git a/parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap b/parser/src/snapshots/rustpython_parser__parser__tests__dict_unpacking.snap similarity index 100% rename from parser/src/snapshots/rustpython_parser__parser__tests__dict_containing_spread.snap rename to parser/src/snapshots/rustpython_parser__parser__tests__dict_unpacking.snap From f2ffe12f8ca5759fdb6206fb4de7074a405ab5e2 Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 22 Jan 2023 00:06:52 +0900 Subject: [PATCH 7/7] Fix comment --- ast/asdl_rs.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ast/asdl_rs.py b/ast/asdl_rs.py index a18ec651e5..b217e629a7 100755 --- a/ast/asdl_rs.py +++ b/ast/asdl_rs.py @@ -241,8 +241,9 @@ class StructVisitor(TypeInfoEmitVisitor): if fieldtype and fieldtype.boxed and (not (parent.product or field.seq) or field.opt): typ = f"Box<{typ}>" if field.opt or ( - # Add `Option` to allow `Dict.keys` to contain `None` for dictionary unpacking - # in a dict literal. + # When a dictionary literal contains dictionary unpacking (e.g., `{**d}`), + # the expression to be unpacked goes in `values` with a `None` at the corresponding + # position in `keys`. To handle this, the type of `keys` needs to be `Option>`. constructor == "Dict" and field.name == "keys" ): typ = f"Option<{typ}>"