From 71f260355679047d5c43c022789336020c12a5ff Mon Sep 17 00:00:00 2001 From: TheAnyKey Date: Sat, 13 Jun 2020 21:16:51 +0000 Subject: [PATCH 1/3] completion of error handling for named expressions and scope handling improvements --- src/symboltable.rs | 181 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 171 insertions(+), 10 deletions(-) diff --git a/src/symboltable.rs b/src/symboltable.rs index cb53c01..2d71ea0 100644 --- a/src/symboltable.rs +++ b/src/symboltable.rs @@ -105,6 +105,10 @@ pub struct Symbol { // 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, + + // inidicates that the symbol is used a bound iterator variable. We distinguish this case + // from normal assignment to detect unallowed re-assignment to iterator variables. + pub is_iter: bool, } impl Symbol { @@ -118,6 +122,7 @@ impl Symbol { is_parameter: false, is_free: false, is_assign_namedexpr_in_comprehension: false, + is_iter: false, } } @@ -217,6 +222,10 @@ impl<'a> SymbolTableAnalyzer<'a> { if symbol.is_assign_namedexpr_in_comprehension && curr_st_typ == SymbolTableType::Comprehension { + // propagate symbol to next higher level that can hold it, + // i.e., function or module. Comprehension is skipped and + // Class is not allowed and detected as error. + //symbol.scope = SymbolScope::Nonlocal; self.analyze_symbol_comprehension(symbol, 0)? } else { match symbol.scope { @@ -262,6 +271,74 @@ impl<'a> SymbolTableAnalyzer<'a> { } } Ok(()) + + /*let mut handled=false; + if curr_st_typ == SymbolTableType::Comprehension + { + if symbol.is_assign_namedexpr_in_comprehension { + // propagate symbol to next higher level that can hold it, + // i.e., function or module. Comprehension is skipped and + // Class is not allowed and detected as error. + //symbol.scope = SymbolScope::Nonlocal; + handled = true; + self.analyze_symbol_comprehension(symbol, 0)? + } + if symbol.is_iter { + if let Some(parent_symbol) = self.tables.last().unwrap().0.get(&symbol.name) { + if parent_symbol.is_assigned { + return Err(SymbolTableError { + error: format!("assignment expression cannot rebind comprehension iteration variable {}", symbol.name), + location: Default::default(), + }); + } + } + } + } + + if !handled { + match symbol.scope { + SymbolScope::Nonlocal => { + let scope_depth = self.tables.len(); + if scope_depth > 0 { + // check if the name is already defined in any outer scope + // therefore + if scope_depth < 2 + || !self + .tables + .iter() + .skip(1) // omit the global scope as it is not non-local + .rev() // revert the order for better performance + .any(|t| t.0.contains_key(&symbol.name)) + // true when any of symbol tables contains the name -> then negate + { + return Err(SymbolTableError { + error: format!("no binding for nonlocal '{}' found", 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); + } + } + } + Ok(())*/ } fn analyze_unknown_symbol(&self, symbol: &mut Symbol) { @@ -303,16 +380,35 @@ impl<'a> SymbolTableAnalyzer<'a> { let symbols = &mut last.0; let table_type = last.1; + // it is not allowed to use an iterator variable as assignee in a named expression + if + /*symbol.is_assign_namedexpr_in_comprehension && (maybe this could be asserted) */ + symbol.is_iter { + return Err(SymbolTableError { + error: format!( + "assignment expression cannot rebind comprehension iteration variable {}", + symbol.name + ), + location: Default::default(), + }); + } + match table_type { SymbolTableType::Module => { symbol.scope = SymbolScope::Global; } - SymbolTableType::Class => {} + SymbolTableType::Class => { + // named expressions are forbidden in comprehensions on class scope + return Err(SymbolTableError { + error: "assignment expression within a comprehension cannot be used in a class body".to_string(), + location: Default::default(), + } ); + } 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); + //self.analyze_unknown_symbol(symbol); // not needed, symbol is analyzed anyhow when its scope is analyzed } match symbol.scope { @@ -323,17 +419,34 @@ impl<'a> SymbolTableAnalyzer<'a> { symbol.scope = SymbolScope::Nonlocal; } } + } else { + let mut cloned_sym = symbol.clone(); + cloned_sym.scope = SymbolScope::Local; + last.0.insert(cloned_sym.name.to_owned(), cloned_sym); } } SymbolTableType::Comprehension => { // TODO check for conflicts - requires more context information about variables match symbols.get_mut(&symbol.name) { Some(parent_symbol) => { + // check if assignee is an iterator in top scope + if parent_symbol.is_iter { + return Err(SymbolTableError { + error: format!("assignment expression cannot rebind comprehension iteration variable {}", symbol.name), + location: Default::default(), + }); + } + + // we synthesize the assignment to the symbol from inner scope parent_symbol.is_assigned = true; // more checks are required } None => { - let cloned_sym = symbol.clone(); - + // extend the scope of the inner symbol + // as we are in a nested comprehension, we expect that the symbol is needed + // ouside, too, and set it therefore to non-local scope. I.e., we expect to + // find a definition on a higher level + let mut cloned_sym = symbol.clone(); + cloned_sym.scope = SymbolScope::Nonlocal; last.0.insert(cloned_sym.name.to_owned(), cloned_sym); } } @@ -353,6 +466,7 @@ enum SymbolUsage { Assigned, Parameter, AssignedNamedExprInCompr, + Iter, } #[derive(Default)] @@ -369,6 +483,8 @@ enum ExpressionContext { Load, Store, Delete, + Iter, + IterDefinitionExp, } impl SymbolTableBuilder { @@ -705,11 +821,14 @@ impl SymbolTableBuilder { let mut is_first_generator = true; for generator in generators { - self.scan_expression(&generator.target, &ExpressionContext::Store)?; + self.scan_expression(&generator.target, &ExpressionContext::Iter)?; if is_first_generator { is_first_generator = false; } else { - self.scan_expression(&generator.iter, &ExpressionContext::Load)?; + self.scan_expression( + &generator.iter, + &ExpressionContext::IterDefinitionExp, + )?; } for if_expr in &generator.ifs { @@ -721,14 +840,22 @@ impl SymbolTableBuilder { // The first iterable is passed as an argument into the created function: assert!(!generators.is_empty()); - self.scan_expression(&generators[0].iter, &ExpressionContext::Load)?; + self.scan_expression(&generators[0].iter, &ExpressionContext::IterDefinitionExp)?; } Call { function, args, keywords, } => { - self.scan_expression(function, &ExpressionContext::Load)?; + match *context { + ExpressionContext::IterDefinitionExp => { + self.scan_expression(function, &ExpressionContext::IterDefinitionExp)?; + } + _ => { + self.scan_expression(function, &ExpressionContext::Load)?; + } + } + self.scan_expressions(args, &ExpressionContext::Load)?; for keyword in keywords { self.scan_expression(&keyword.value, &ExpressionContext::Load)?; @@ -743,17 +870,27 @@ impl SymbolTableBuilder { ExpressionContext::Delete => { self.register_name(name, SymbolUsage::Used)?; } - ExpressionContext::Load => { + ExpressionContext::Load | ExpressionContext::IterDefinitionExp => { self.register_name(name, SymbolUsage::Used)?; } ExpressionContext::Store => { self.register_name(name, SymbolUsage::Assigned)?; } + ExpressionContext::Iter => { + self.register_name(name, SymbolUsage::Iter)?; + } } } Lambda { args, body } => { self.enter_function("lambda", args, expression.location.row())?; - self.scan_expression(body, &ExpressionContext::Load)?; + match *context { + ExpressionContext::IterDefinitionExp => { + self.scan_expression(body, &ExpressionContext::IterDefinitionExp)?; + } + _ => { + self.scan_expression(body, &ExpressionContext::Load)?; + } + } self.leave_scope(); } IfExpression { test, body, orelse } => { @@ -763,6 +900,15 @@ impl SymbolTableBuilder { } NamedExpression { left, right } => { + // named expressions are not allowed in the definiton of + // comprehension iterator definitions + if let ExpressionContext::IterDefinitionExp = *context { + return Err(SymbolTableError { + error: "assignment expression cannot be used in a comprehension iterable expression".to_string(), + location: Default::default(), + }); + } + self.scan_expression(right, &ExpressionContext::Load)?; // special handling for assigned identifier in named expressions @@ -933,8 +1079,23 @@ impl SymbolTableBuilder { SymbolUsage::Used => { symbol.is_referenced = true; } + SymbolUsage::Iter => { + symbol.is_iter = true; + } } + // and even more checking + // it is not allowed to assign to iterator variables (by named expressions) + if symbol.is_iter && symbol.is_assigned + /*&& symbol.is_assign_namedexpr_in_comprehension*/ + { + return Err(SymbolTableError { + error: + "assignment expression cannot be used in a comprehension iterable expression" + .to_string(), + location, + }); + } Ok(()) } } From 162e5d02abe6fa99546a3a80639bdf55a89bc54c Mon Sep 17 00:00:00 2001 From: TheAnyKey Date: Sun, 14 Jun 2020 12:06:39 +0000 Subject: [PATCH 2/3] commented and cleaned up --- src/symboltable.rs | 74 ++-------------------------------------------- 1 file changed, 2 insertions(+), 72 deletions(-) diff --git a/src/symboltable.rs b/src/symboltable.rs index 2d71ea0..e7bd6f0 100644 --- a/src/symboltable.rs +++ b/src/symboltable.rs @@ -271,74 +271,6 @@ impl<'a> SymbolTableAnalyzer<'a> { } } Ok(()) - - /*let mut handled=false; - if curr_st_typ == SymbolTableType::Comprehension - { - if symbol.is_assign_namedexpr_in_comprehension { - // propagate symbol to next higher level that can hold it, - // i.e., function or module. Comprehension is skipped and - // Class is not allowed and detected as error. - //symbol.scope = SymbolScope::Nonlocal; - handled = true; - self.analyze_symbol_comprehension(symbol, 0)? - } - if symbol.is_iter { - if let Some(parent_symbol) = self.tables.last().unwrap().0.get(&symbol.name) { - if parent_symbol.is_assigned { - return Err(SymbolTableError { - error: format!("assignment expression cannot rebind comprehension iteration variable {}", symbol.name), - location: Default::default(), - }); - } - } - } - } - - if !handled { - match symbol.scope { - SymbolScope::Nonlocal => { - let scope_depth = self.tables.len(); - if scope_depth > 0 { - // check if the name is already defined in any outer scope - // therefore - if scope_depth < 2 - || !self - .tables - .iter() - .skip(1) // omit the global scope as it is not non-local - .rev() // revert the order for better performance - .any(|t| t.0.contains_key(&symbol.name)) - // true when any of symbol tables contains the name -> then negate - { - return Err(SymbolTableError { - error: format!("no binding for nonlocal '{}' found", 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); - } - } - } - Ok(())*/ } fn analyze_unknown_symbol(&self, symbol: &mut Symbol) { @@ -381,9 +313,7 @@ impl<'a> SymbolTableAnalyzer<'a> { let table_type = last.1; // it is not allowed to use an iterator variable as assignee in a named expression - if - /*symbol.is_assign_namedexpr_in_comprehension && (maybe this could be asserted) */ - symbol.is_iter { + if symbol.is_iter { return Err(SymbolTableError { error: format!( "assignment expression cannot rebind comprehension iteration variable {}", @@ -903,7 +833,7 @@ impl SymbolTableBuilder { // named expressions are not allowed in the definiton of // comprehension iterator definitions if let ExpressionContext::IterDefinitionExp = *context { - return Err(SymbolTableError { + return Err(SymbolTableError { error: "assignment expression cannot be used in a comprehension iterable expression".to_string(), location: Default::default(), }); From 60a787e3e058ddfeb43e6b3cc202c482a7c80eaf Mon Sep 17 00:00:00 2001 From: TheAnyKey Date: Sun, 21 Jun 2020 22:18:43 +0000 Subject: [PATCH 3/3] fixed scoping issue, commented test with references to original version --- src/symboltable.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/symboltable.rs b/src/symboltable.rs index e7bd6f0..58c27eb 100644 --- a/src/symboltable.rs +++ b/src/symboltable.rs @@ -1011,6 +1011,7 @@ impl SymbolTableBuilder { } SymbolUsage::Iter => { symbol.is_iter = true; + symbol.scope = SymbolScope::Local; } }