Skip to content

Commit fd2e0aa

Browse files
TheAnyKeycoolreader18CodeTriangle
authored
Implement Py38 named expression (PEP 572) (RustPython#1934)
* Initial implementation of named expression and import according CPython tests * added new instruction with inversed evaluation order for dict comprehension, in other cases use regular evaluation order * added further aspects to implementation, cleaned up, imported test from CPython * implemented first parts of scoping enhancement and extended checks * completion of name resolution ongoing, now more test passing, still warinings and cleanup required * further optimization of name resolution in nested scopes * Initialize the vm with imports from _io instead of io * Add the OpenBSD support that I am smart enough to * adapted grammer to full support, most test are passing now, esp. all invalids are passed. Cleaned up in symboltable * more conditional compiling, this time for errors * rustfmt was not pleased * premature push, whoops * Add expected_failure result type to jsontests * Initial implementation of named expression and import according CPython tests * added new instruction with inversed evaluation order for dict comprehension, in other cases use regular evaluation order * added further aspects to implementation, cleaned up, imported test from CPython * implemented first parts of scoping enhancement and extended checks * completion of name resolution ongoing, now more test passing, still warinings and cleanup required * further optimization of name resolution in nested scopes * adapted grammer to full support, most test are passing now, esp. all invalids are passed. Cleaned up in symboltable * Fixed nameing convention violation and removed unnecessary information from symbol resolution. Added some more comments. * Fixed nameing convention violation and removed unnecessary information from symbol resolution. Added some more comments. Co-authored-by: Noah <33094578+coolreader18@users.noreply.github.com> Co-authored-by: Reuben Staley <lighthousemaniac@gmail.com>
1 parent 3a524d3 commit fd2e0aa

File tree

10 files changed

+820
-59
lines changed

10 files changed

+820
-59
lines changed

‎Lib/test/test_named_expression.py‎

Lines changed: 577 additions & 0 deletions
Large diffs are not rendered by default.

‎bytecode/src/bytecode.rs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ pub enum Instruction {
273273
MapAdd {
274274
i: usize,
275275
},
276+
276277
PrintExpr,
277278
LoadBuildClass,
278279
UnpackSequence {
@@ -296,6 +297,13 @@ pub enum Instruction {
296297
},
297298
GetAIter,
298299
GetANext,
300+
301+
/// Reverse order evaluation in MapAdd
302+
/// required to support named expressions of Python 3.8 in dict comprehension
303+
/// today (including Py3.9) only required in dict comprehension.
304+
MapAddRev {
305+
i: usize,
306+
},
299307
}
300308

301309
use self::Instruction::*;
@@ -586,7 +594,7 @@ impl Instruction {
586594
BuildSlice { size } => w!(BuildSlice, size),
587595
ListAppend { i } => w!(ListAppend, i),
588596
SetAdd { i } => w!(SetAdd, i),
589-
MapAdd { i } => w!(MapAdd, i),
597+
MapAddRev { i } => w!(MapAddRev, i),
590598
PrintExpr => w!(PrintExpr),
591599
LoadBuildClass => w!(LoadBuildClass),
592600
UnpackSequence { size } => w!(UnpackSequence, size),
@@ -597,6 +605,7 @@ impl Instruction {
597605
GetAwaitable => w!(GetAwaitable),
598606
GetAIter => w!(GetAIter),
599607
GetANext => w!(GetANext),
608+
MapAdd { i } => w!(MapAdd, i),
600609
}
601610
}
602611
}

‎compiler/src/compile.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1804,6 +1804,12 @@ impl<O: OutputStream> Compiler<O> {
18041804
// End
18051805
self.set_label(end_label);
18061806
}
1807+
1808+
NamedExpression { left, right } => {
1809+
self.compile_expression(right)?;
1810+
self.emit(Instruction::Duplicate);
1811+
self.compile_store(left)?;
1812+
}
18071813
}
18081814
Ok(())
18091815
}
@@ -2066,10 +2072,11 @@ impl<O: OutputStream> Compiler<O> {
20662072
});
20672073
}
20682074
ast::ComprehensionKind::Dict { key, value } => {
2069-
self.compile_expression(value)?;
2075+
// changed evaluation order for Py38 named expression PEP 572
20702076
self.compile_expression(key)?;
2077+
self.compile_expression(value)?;
20712078

2072-
self.emit(Instruction::MapAdd {
2079+
self.emit(Instruction::MapAddRev {
20732080
i: 1 + generators.len(),
20742081
});
20752082
}

‎compiler/src/symboltable.rs‎

Lines changed: 148 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ pub enum SymbolTableType {
6666
Module,
6767
Class,
6868
Function,
69+
Comprehension,
6970
}
7071

7172
impl fmt::Display for SymbolTableType {
@@ -74,6 +75,7 @@ impl fmt::Display for SymbolTableType {
7475
SymbolTableType::Module => write!(f, "module"),
7576
SymbolTableType::Class => write!(f, "class"),
7677
SymbolTableType::Function => write!(f, "function"),
78+
SymbolTableType::Comprehension => write!(f, "comprehension"),
7779
}
7880
}
7981
}
@@ -99,6 +101,10 @@ pub struct Symbol {
99101
pub is_assigned: bool,
100102
pub is_parameter: bool,
101103
pub is_free: bool,
104+
105+
// indicates if the symbol gets a value assigned by a named expression in a comprehension
106+
// this is required to correct the scope in the analysis.
107+
pub is_assign_namedexpr_in_comprehension: bool,
102108
}
103109

104110
impl Symbol {
@@ -111,6 +117,7 @@ impl Symbol {
111117
is_assigned: false,
112118
is_parameter: false,
113119
is_free: false,
120+
is_assign_namedexpr_in_comprehension: false,
114121
}
115122
}
116123

@@ -193,72 +200,138 @@ impl<'a> SymbolTableAnalyzer<'a> {
193200
for sub_table in sub_tables {
194201
self.analyze_symbol_table(sub_table)?;
195202
}
196-
let (symbols, _) = self.tables.pop().unwrap();
203+
let (symbols, st_typ) = self.tables.pop().unwrap();
197204

198205
// Analyze symbols:
199206
for symbol in symbols.values_mut() {
200-
self.analyze_symbol(symbol)?;
207+
self.analyze_symbol(symbol, st_typ)?;
201208
}
202-
203209
Ok(())
204210
}
205211

206-
fn analyze_symbol(&self, symbol: &mut Symbol) -> SymbolTableResult {
207-
match symbol.scope {
208-
SymbolScope::Nonlocal => {
209-
// check if name is defined in parent table!
210-
let parent_symbol_table = self.tables.last();
211-
// symbol.table.borrow().parent.clone();
212-
213-
if let Some((symbols, _)) = parent_symbol_table {
214-
let scope_depth = self.tables.len();
215-
if !symbols.contains_key(&symbol.name) || scope_depth < 2 {
212+
fn analyze_symbol(
213+
&mut self,
214+
symbol: &mut Symbol,
215+
curr_st_typ: SymbolTableType,
216+
) -> SymbolTableResult {
217+
if symbol.is_assign_namedexpr_in_comprehension
218+
&& curr_st_typ == SymbolTableType::Comprehension
219+
{
220+
self.analyze_symbol_comprehension(symbol, 0)?
221+
} else {
222+
match symbol.scope {
223+
SymbolScope::Nonlocal => {
224+
// check if name is defined in parent table!
225+
let parent_symbol_table = self.tables.last();
226+
if let Some((symbols, _)) = parent_symbol_table {
227+
let scope_depth = self.tables.len();
228+
if !symbols.contains_key(&symbol.name) || scope_depth < 2 {
229+
return Err(SymbolTableError {
230+
error: format!("no binding for nonlocal '{}' found", symbol.name),
231+
location: Default::default(),
232+
});
233+
}
234+
} else {
216235
return Err(SymbolTableError {
217-
error: format!("no binding for nonlocal '{}' found", symbol.name),
236+
error: format!(
237+
"nonlocal {} defined at place without an enclosing scope",
238+
symbol.name
239+
),
218240
location: Default::default(),
219241
});
220242
}
221-
} else {
222-
return Err(SymbolTableError {
223-
error: format!(
224-
"nonlocal {} defined at place without an enclosing scope",
225-
symbol.name
226-
),
227-
location: Default::default(),
228-
});
243+
}
244+
SymbolScope::Global => {
245+
// TODO: add more checks for globals?
246+
}
247+
SymbolScope::Local => {
248+
// all is well
249+
}
250+
SymbolScope::Unknown => {
251+
// Try hard to figure out what the scope of this symbol is.
252+
self.analyze_unknown_symbol(symbol);
229253
}
230254
}
231-
SymbolScope::Global => {
232-
// TODO: add more checks for globals?
233-
}
234-
SymbolScope::Local => {
235-
// all is well
255+
}
256+
Ok(())
257+
}
258+
259+
fn analyze_unknown_symbol(&self, symbol: &mut Symbol) {
260+
if symbol.is_assigned || symbol.is_parameter {
261+
symbol.scope = SymbolScope::Local;
262+
} else {
263+
// Interesting stuff about the __class__ variable:
264+
// https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object
265+
let found_in_outer_scope = symbol.name == "__class__"
266+
|| self.tables.iter().skip(1).any(|(symbols, typ)| {
267+
*typ != SymbolTableType::Class && symbols.contains_key(&symbol.name)
268+
});
269+
270+
if found_in_outer_scope {
271+
// Symbol is in some outer scope.
272+
symbol.is_free = true;
273+
} else if self.tables.is_empty() {
274+
// Don't make assumptions when we don't know.
275+
symbol.scope = SymbolScope::Unknown;
276+
} else {
277+
// If there are scopes above we can assume global.
278+
symbol.scope = SymbolScope::Global;
236279
}
237-
SymbolScope::Unknown => {
238-
// Try hard to figure out what the scope of this symbol is.
280+
}
281+
}
239282

240-
if symbol.is_assigned || symbol.is_parameter {
241-
symbol.scope = SymbolScope::Local;
242-
} else {
243-
// Interesting stuff about the __class__ variable:
244-
// https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object
245-
let found_in_outer_scope = symbol.name == "__class__"
246-
|| self.tables.iter().skip(1).any(|(symbols, typ)| {
247-
*typ != SymbolTableType::Class && symbols.contains_key(&symbol.name)
248-
});
283+
// Implements the symbol analysis and scope extension for names
284+
// assigned by a named expression in a comprehension. See:
285+
// https://github.com/python/cpython/blob/7b78e7f9fd77bb3280ee39fb74b86772a7d46a70/Python/symtable.c#L1435
286+
fn analyze_symbol_comprehension(
287+
&mut self,
288+
symbol: &mut Symbol,
289+
parent_offset: usize,
290+
) -> SymbolTableResult {
291+
// TODO: quite C-ish way to implement the iteration
292+
// when this is called, we expect to be in the direct parent scope of the scope that contains 'symbol'
293+
let offs = self.tables.len() - 1 - parent_offset;
294+
let last = self.tables.get_mut(offs).unwrap();
295+
let symbols = &mut last.0;
296+
let table_type = last.1;
297+
298+
match table_type {
299+
SymbolTableType::Module => {
300+
symbol.scope = SymbolScope::Global;
301+
}
302+
SymbolTableType::Class => {}
303+
SymbolTableType::Function => {
304+
if let Some(parent_symbol) = symbols.get_mut(&symbol.name) {
305+
if let SymbolScope::Unknown = parent_symbol.scope {
306+
parent_symbol.is_assigned = true; // this information is new, as the asignment is done in inner scope
307+
self.analyze_unknown_symbol(symbol);
308+
}
249309

250-
if found_in_outer_scope {
251-
// Symbol is in some outer scope.
252-
symbol.is_free = true;
253-
} else if self.tables.is_empty() {
254-
// Don't make assumptions when we don't know.
255-
symbol.scope = SymbolScope::Unknown;
256-
} else {
257-
// If there are scopes above we can assume global.
258-
symbol.scope = SymbolScope::Global;
310+
match symbol.scope {
311+
SymbolScope::Global => {
312+
symbol.scope = SymbolScope::Global;
313+
}
314+
_ => {
315+
symbol.scope = SymbolScope::Nonlocal;
316+
}
259317
}
260318
}
261319
}
320+
SymbolTableType::Comprehension => {
321+
// TODO check for conflicts - requires more context information about variables
322+
match symbols.get_mut(&symbol.name) {
323+
Some(parent_symbol) => {
324+
parent_symbol.is_assigned = true; // more checks are required
325+
}
326+
None => {
327+
let cloned_sym = symbol.clone();
328+
329+
last.0.insert(cloned_sym.name.to_owned(), cloned_sym);
330+
}
331+
}
332+
333+
self.analyze_symbol_comprehension(symbol, parent_offset + 1)?;
334+
}
262335
}
263336
Ok(())
264337
}
@@ -271,6 +344,7 @@ enum SymbolUsage {
271344
Used,
272345
Assigned,
273346
Parameter,
347+
AssignedNamedExprInCompr,
274348
}
275349

276350
#[derive(Default)]
@@ -602,7 +676,7 @@ impl SymbolTableBuilder {
602676

603677
self.enter_scope(
604678
scope_name,
605-
SymbolTableType::Function,
679+
SymbolTableType::Comprehension,
606680
expression.location.row(),
607681
);
608682

@@ -679,6 +753,28 @@ impl SymbolTableBuilder {
679753
self.scan_expression(body, &ExpressionContext::Load)?;
680754
self.scan_expression(orelse, &ExpressionContext::Load)?;
681755
}
756+
757+
NamedExpression { left, right } => {
758+
self.scan_expression(right, &ExpressionContext::Load)?;
759+
760+
// special handling for assigned identifier in named expressions
761+
// that are used in comprehensions. This required to correctly
762+
// propagate the scope of the named assigned named and not to
763+
// propagate inner names.
764+
if let Identifier { name } = &left.node {
765+
let table = self.tables.last().unwrap();
766+
if table.typ == SymbolTableType::Comprehension {
767+
self.register_name(name, SymbolUsage::AssignedNamedExprInCompr)?;
768+
} else {
769+
// omit one recursion. When the handling of an store changes for
770+
// Identifiers this needs adapted - more forward safe would be
771+
// calling scan_expression directly.
772+
self.register_name(name, SymbolUsage::Assigned)?;
773+
}
774+
} else {
775+
self.scan_expression(left, &ExpressionContext::Store)?;
776+
}
777+
}
682778
}
683779
Ok(())
684780
}
@@ -810,6 +906,10 @@ impl SymbolTableBuilder {
810906
SymbolUsage::Assigned => {
811907
symbol.is_assigned = true;
812908
}
909+
SymbolUsage::AssignedNamedExprInCompr => {
910+
symbol.is_assigned = true;
911+
symbol.is_assign_namedexpr_in_comprehension = true;
912+
}
813913
SymbolUsage::Global => {
814914
if let SymbolScope::Unknown = symbol.scope {
815915
symbol.scope = SymbolScope::Global;

‎parser/src/ast.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,12 @@ pub enum ExpressionType {
309309
orelse: Box<Expression>,
310310
},
311311

312+
// A named expression
313+
NamedExpression {
314+
left: Box<Expression>,
315+
right: Box<Expression>,
316+
},
317+
312318
/// The literal 'True'.
313319
True,
314320

@@ -364,6 +370,7 @@ impl Expression {
364370
IfExpression { .. } => "conditional expression",
365371
True | False | None => "keyword",
366372
Ellipsis => "ellipsis",
373+
NamedExpression { .. } => "named expression",
367374
}
368375
}
369376
}

‎parser/src/lexer.rs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1075,7 +1075,16 @@ where
10751075
self.nesting -= 1;
10761076
}
10771077
':' => {
1078-
self.eat_single_char(Tok::Colon);
1078+
let tok_start = self.get_pos();
1079+
self.next_char();
1080+
if let Some('=') = self.chr0 {
1081+
self.next_char();
1082+
let tok_end = self.get_pos();
1083+
self.emit((tok_start, Tok::ColonEqual, tok_end));
1084+
} else {
1085+
let tok_end = self.get_pos();
1086+
self.emit((tok_start, Tok::Colon, tok_end));
1087+
}
10791088
}
10801089
';' => {
10811090
self.eat_single_char(Tok::Semi);

0 commit comments

Comments
 (0)