Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3342a00
Initial implementation of named expression and import according CPy…
TheAnyKey May 16, 2020
f16274d
added new instruction with inversed evaluation order for dict compreh…
TheAnyKey May 17, 2020
2bce6b6
added further aspects to implementation, cleaned up, imported test fr…
TheAnyKey May 17, 2020
8bdd69c
implemented first parts of scoping enhancement and extended checks
TheAnyKey May 23, 2020
3295147
completion of name resolution ongoing, now more test passing, still w…
TheAnyKey May 24, 2020
17008ef
further optimization of name resolution in nested scopes
TheAnyKey May 24, 2020
3b29fb7
Initialize the vm with imports from _io instead of io
coolreader18 May 25, 2020
f2eb588
Add the OpenBSD support that I am smart enough to
CodeTriangle May 27, 2020
1968c5d
adapted grammer to full support, most test are passing now, esp. all …
TheAnyKey May 27, 2020
b35b840
more conditional compiling, this time for errors
CodeTriangle May 27, 2020
d3dc524
rustfmt was not pleased
CodeTriangle May 28, 2020
eafb741
premature push, whoops
CodeTriangle May 28, 2020
0d7d4f0
Add expected_failure result type to jsontests
coolreader18 May 28, 2020
802913b
Merge pull request #1946 from CodeTriangle/master
coolreader18 May 28, 2020
8452dbe
Merge pull request #1942 from RustPython/coolreader18/init-with-_io
coolreader18 May 28, 2020
ca9eac5
Initial implementation of named expression and import according CPy…
TheAnyKey May 16, 2020
1a27660
added new instruction with inversed evaluation order for dict compreh…
TheAnyKey May 17, 2020
9728625
added further aspects to implementation, cleaned up, imported test fr…
TheAnyKey May 17, 2020
678deb5
implemented first parts of scoping enhancement and extended checks
TheAnyKey May 23, 2020
ffea11b
completion of name resolution ongoing, now more test passing, still w…
TheAnyKey May 24, 2020
0ebf58f
further optimization of name resolution in nested scopes
TheAnyKey May 24, 2020
3296760
adapted grammer to full support, most test are passing now, esp. all …
TheAnyKey May 27, 2020
e1d382d
Merge branch 'TheAnyKey/p38_named_expr' of https://github.com/TheAnyK…
TheAnyKey May 28, 2020
0f2ed00
Fixed nameing convention violation and removed unnecessary informatio…
TheAnyKey May 30, 2020
0e868c5
Fixed nameing convention violation and removed unnecessary informatio…
TheAnyKey May 30, 2020
7e77c20
Merge branch 'TheAnyKey/p38_named_expr' of https://github.com/TheAnyK…
TheAnyKey May 30, 2020
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
577 changes: 577 additions & 0 deletions Lib/test/test_named_expression.py

Large diffs are not rendered by default.

11 changes: 10 additions & 1 deletion bytecode/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ pub enum Instruction {
MapAdd {
i: usize,
},

PrintExpr,
LoadBuildClass,
UnpackSequence {
Expand All @@ -296,6 +297,13 @@ pub enum Instruction {
},
GetAIter,
GetANext,

/// Reverse order evaluation in MapAdd
/// required to support named expressions of Python 3.8 in dict comprehension
/// today (including Py3.9) only required in dict comprehension.
MapAddRev {
i: usize,
},
}

use self::Instruction::*;
Expand Down Expand Up @@ -586,7 +594,7 @@ impl Instruction {
BuildSlice { size } => w!(BuildSlice, size),
ListAppend { i } => w!(ListAppend, i),
SetAdd { i } => w!(SetAdd, i),
MapAdd { i } => w!(MapAdd, i),
MapAddRev { i } => w!(MapAddRev, i),
PrintExpr => w!(PrintExpr),
LoadBuildClass => w!(LoadBuildClass),
UnpackSequence { size } => w!(UnpackSequence, size),
Expand All @@ -597,6 +605,7 @@ impl Instruction {
GetAwaitable => w!(GetAwaitable),
GetAIter => w!(GetAIter),
GetANext => w!(GetANext),
MapAdd { i } => w!(MapAdd, i),
}
}
}
Expand Down
11 changes: 9 additions & 2 deletions compiler/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1801,6 +1801,12 @@ impl<O: OutputStream> Compiler<O> {
// End
self.set_label(end_label);
}

NamedExpression { left, right } => {
self.compile_expression(right)?;
self.emit(Instruction::Duplicate);
self.compile_store(left)?;
}
}
Ok(())
}
Expand Down Expand Up @@ -2051,10 +2057,11 @@ impl<O: OutputStream> Compiler<O> {
});
}
ast::ComprehensionKind::Dict { key, value } => {
self.compile_expression(value)?;
// changed evaluation order for Py38 named expression PEP 572
self.compile_expression(key)?;
self.compile_expression(value)?;

self.emit(Instruction::MapAdd {
self.emit(Instruction::MapAddRev {
i: 1 + generators.len(),
});
}
Expand Down
196 changes: 148 additions & 48 deletions compiler/src/symboltable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ pub enum SymbolTableType {
Module,
Class,
Function,
Comprehension,
Comment thread
TheAnyKey marked this conversation as resolved.
}

impl fmt::Display for SymbolTableType {
Expand All @@ -74,6 +75,7 @@ impl fmt::Display for SymbolTableType {
SymbolTableType::Module => write!(f, "module"),
SymbolTableType::Class => write!(f, "class"),
SymbolTableType::Function => write!(f, "function"),
SymbolTableType::Comprehension => write!(f, "comprehension"),
}
}
}
Expand All @@ -99,6 +101,10 @@ pub struct Symbol {
pub is_assigned: bool,
pub is_parameter: bool,
pub is_free: bool,

// indicates if the symbol gets a value assigned by a named expression in a comprehension
// this is required to correct the scope in the analysis.
pub is_assign_namedexpr_in_comprehension: bool,
Comment thread
TheAnyKey marked this conversation as resolved.
}

impl Symbol {
Expand All @@ -111,6 +117,7 @@ impl Symbol {
is_assigned: false,
is_parameter: false,
is_free: false,
is_assign_namedexpr_in_comprehension: false,
}
}

Expand Down Expand Up @@ -193,72 +200,138 @@ impl<'a> SymbolTableAnalyzer<'a> {
for sub_table in sub_tables {
self.analyze_symbol_table(sub_table)?;
}
let (symbols, _) = self.tables.pop().unwrap();
let (symbols, st_typ) = self.tables.pop().unwrap();

// Analyze symbols:
for symbol in symbols.values_mut() {
self.analyze_symbol(symbol)?;
self.analyze_symbol(symbol, st_typ)?;
}

Ok(())
}

fn analyze_symbol(&self, symbol: &mut Symbol) -> SymbolTableResult {
match symbol.scope {
SymbolScope::Nonlocal => {
// check if name is defined in parent table!
let parent_symbol_table = self.tables.last();
// symbol.table.borrow().parent.clone();

if let Some((symbols, _)) = parent_symbol_table {
let scope_depth = self.tables.len();
if !symbols.contains_key(&symbol.name) || scope_depth < 2 {
fn analyze_symbol(
&mut self,
symbol: &mut Symbol,
curr_st_typ: SymbolTableType,
) -> SymbolTableResult {
if symbol.is_assign_namedexpr_in_comprehension
&& curr_st_typ == SymbolTableType::Comprehension
{
self.analyze_symbol_comprehension(symbol, 0)?
Comment thread
TheAnyKey marked this conversation as resolved.
} else {
match symbol.scope {
SymbolScope::Nonlocal => {
// check if name is defined in parent table!
let parent_symbol_table = self.tables.last();
if let Some((symbols, _)) = parent_symbol_table {
let scope_depth = self.tables.len();
if !symbols.contains_key(&symbol.name) || scope_depth < 2 {
return Err(SymbolTableError {
error: format!("no binding for nonlocal '{}' found", symbol.name),
location: Default::default(),
});
}
} else {
return Err(SymbolTableError {
error: format!("no binding for nonlocal '{}' found", symbol.name),
error: format!(
"nonlocal {} defined at place without an enclosing scope",
symbol.name
),
location: Default::default(),
});
}
} else {
return Err(SymbolTableError {
error: format!(
"nonlocal {} defined at place without an enclosing scope",
symbol.name
),
location: Default::default(),
});
}
SymbolScope::Global => {
// TODO: add more checks for globals?
}
SymbolScope::Local => {
// all is well
}
SymbolScope::Unknown => {
// Try hard to figure out what the scope of this symbol is.
self.analyze_unknown_symbol(symbol);
}
}
SymbolScope::Global => {
// TODO: add more checks for globals?
}
SymbolScope::Local => {
// all is well
}
Ok(())
}

fn analyze_unknown_symbol(&self, symbol: &mut Symbol) {
if symbol.is_assigned || symbol.is_parameter {
symbol.scope = SymbolScope::Local;
} else {
// Interesting stuff about the __class__ variable:
// https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object
let found_in_outer_scope = symbol.name == "__class__"
|| self.tables.iter().skip(1).any(|(symbols, typ)| {
*typ != SymbolTableType::Class && symbols.contains_key(&symbol.name)
});

if found_in_outer_scope {
// Symbol is in some outer scope.
symbol.is_free = true;
} else if self.tables.is_empty() {
// Don't make assumptions when we don't know.
symbol.scope = SymbolScope::Unknown;
} else {
// If there are scopes above we can assume global.
symbol.scope = SymbolScope::Global;
}
SymbolScope::Unknown => {
// Try hard to figure out what the scope of this symbol is.
}
}

if symbol.is_assigned || symbol.is_parameter {
symbol.scope = SymbolScope::Local;
} else {
// Interesting stuff about the __class__ variable:
// https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object
let found_in_outer_scope = symbol.name == "__class__"
|| self.tables.iter().skip(1).any(|(symbols, typ)| {
*typ != SymbolTableType::Class && symbols.contains_key(&symbol.name)
});
// Implements the symbol analysis and scope extension for names
// assigned by a named expression in a comprehension. See:
// https://github.com/python/cpython/blob/7b78e7f9fd77bb3280ee39fb74b86772a7d46a70/Python/symtable.c#L1435
fn analyze_symbol_comprehension(
Comment thread
TheAnyKey marked this conversation as resolved.
&mut self,
symbol: &mut Symbol,
parent_offset: usize,
) -> SymbolTableResult {
// TODO: quite C-ish way to implement the iteration
// when this is called, we expect to be in the direct parent scope of the scope that contains 'symbol'
let offs = self.tables.len() - 1 - parent_offset;
let last = self.tables.get_mut(offs).unwrap();
let symbols = &mut last.0;
let table_type = last.1;

match table_type {
SymbolTableType::Module => {
symbol.scope = SymbolScope::Global;
}
SymbolTableType::Class => {}
SymbolTableType::Function => {
if let Some(parent_symbol) = symbols.get_mut(&symbol.name) {
if let SymbolScope::Unknown = parent_symbol.scope {
parent_symbol.is_assigned = true; // this information is new, as the asignment is done in inner scope
self.analyze_unknown_symbol(symbol);
}

if found_in_outer_scope {
// Symbol is in some outer scope.
symbol.is_free = true;
} else if self.tables.is_empty() {
// Don't make assumptions when we don't know.
symbol.scope = SymbolScope::Unknown;
} else {
// If there are scopes above we can assume global.
symbol.scope = SymbolScope::Global;
match symbol.scope {
SymbolScope::Global => {
symbol.scope = SymbolScope::Global;
}
_ => {
symbol.scope = SymbolScope::Nonlocal;
}
}
}
}
SymbolTableType::Comprehension => {
// TODO check for conflicts - requires more context information about variables
match symbols.get_mut(&symbol.name) {
Some(parent_symbol) => {
parent_symbol.is_assigned = true; // more checks are required
}
None => {
let cloned_sym = symbol.clone();

last.0.insert(cloned_sym.name.to_owned(), cloned_sym);
}
}

self.analyze_symbol_comprehension(symbol, parent_offset + 1)?;
}
}
Ok(())
}
Expand All @@ -271,6 +344,7 @@ enum SymbolUsage {
Used,
Assigned,
Parameter,
AssignedNamedExprInCompr,
}

#[derive(Default)]
Expand Down Expand Up @@ -602,7 +676,7 @@ impl SymbolTableBuilder {

self.enter_scope(
scope_name,
SymbolTableType::Function,
SymbolTableType::Comprehension,
expression.location.row(),
);

Expand Down Expand Up @@ -679,6 +753,28 @@ impl SymbolTableBuilder {
self.scan_expression(body, &ExpressionContext::Load)?;
self.scan_expression(orelse, &ExpressionContext::Load)?;
}

NamedExpression { left, right } => {
self.scan_expression(right, &ExpressionContext::Load)?;

// special handling for assigned identifier in named expressions
// that are used in comprehensions. This required to correctly
// propagate the scope of the named assigned named and not to
// propagate inner names.
if let Identifier { name } = &left.node {
let table = self.tables.last().unwrap();
if table.typ == SymbolTableType::Comprehension {
self.register_name(name, SymbolUsage::AssignedNamedExprInCompr)?;
} else {
// omit one recursion. When the handling of an store changes for
// Identifiers this needs adapted - more forward safe would be
// calling scan_expression directly.
self.register_name(name, SymbolUsage::Assigned)?;
}
} else {
self.scan_expression(left, &ExpressionContext::Store)?;
}
}
}
Ok(())
}
Expand Down Expand Up @@ -810,6 +906,10 @@ impl SymbolTableBuilder {
SymbolUsage::Assigned => {
symbol.is_assigned = true;
}
SymbolUsage::AssignedNamedExprInCompr => {
symbol.is_assigned = true;
symbol.is_assign_namedexpr_in_comprehension = true;
}
SymbolUsage::Global => {
if let SymbolScope::Unknown = symbol.scope {
symbol.scope = SymbolScope::Global;
Expand Down
7 changes: 7 additions & 0 deletions parser/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,12 @@ pub enum ExpressionType {
orelse: Box<Expression>,
},

// A named expression
NamedExpression {
left: Box<Expression>,
right: Box<Expression>,
},

/// The literal 'True'.
True,

Expand Down Expand Up @@ -364,6 +370,7 @@ impl Expression {
IfExpression { .. } => "conditional expression",
True | False | None => "keyword",
Ellipsis => "ellipsis",
NamedExpression { .. } => "named expression",
}
}
}
Expand Down
11 changes: 10 additions & 1 deletion parser/src/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,16 @@ where
self.nesting -= 1;
}
':' => {
self.eat_single_char(Tok::Colon);
let tok_start = self.get_pos();
self.next_char();
if let Some('=') = self.chr0 {
self.next_char();
let tok_end = self.get_pos();
self.emit((tok_start, Tok::ColonEqual, tok_end));
} else {
let tok_end = self.get_pos();
self.emit((tok_start, Tok::Colon, tok_end));
}
}
';' => {
self.eat_single_char(Tok::Semi);
Expand Down
Loading