From f23ea5eb09cfac83fcb58dd2ae5c1fb1774a4f1e Mon Sep 17 00:00:00 2001 From: yt2b Date: Sun, 27 Nov 2022 23:30:52 +0900 Subject: [PATCH 1/6] Add bitflags --- Cargo.lock | 1 + compiler/codegen/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index fa4e257123..d7a086f27f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1931,6 +1931,7 @@ name = "rustpython-codegen" version = "0.1.2" dependencies = [ "ahash", + "bitflags", "indexmap", "insta", "itertools", diff --git a/compiler/codegen/Cargo.toml b/compiler/codegen/Cargo.toml index 3d1a455d96..253d976c3e 100644 --- a/compiler/codegen/Cargo.toml +++ b/compiler/codegen/Cargo.toml @@ -12,6 +12,7 @@ rustpython-ast = { path = "../ast", features = ["unparse"] } rustpython-compiler-core = { path = "../core", version = "0.1.1" } ahash = "0.7.6" +bitflags = "1.3.2" indexmap = "1.8.1" itertools = "0.10.3" log = "0.4.16" From 1ffb652adf0dae246f4d54e48745bb4248a67e50 Mon Sep 17 00:00:00 2001 From: yt2b Date: Sun, 27 Nov 2022 23:36:25 +0900 Subject: [PATCH 2/6] Add SymbolFlags --- compiler/codegen/src/symboltable.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/compiler/codegen/src/symboltable.rs b/compiler/codegen/src/symboltable.rs index b100a9e504..24ee5ea20f 100644 --- a/compiler/codegen/src/symboltable.rs +++ b/compiler/codegen/src/symboltable.rs @@ -11,6 +11,7 @@ use crate::{ error::{CodegenError, CodegenErrorType}, IndexMap, }; +use bitflags::bitflags; use rustpython_ast as ast; use rustpython_compiler_core::Location; use std::{borrow::Cow, fmt}; @@ -94,6 +95,33 @@ pub enum SymbolScope { Cell, } +bitflags! { + pub struct SymbolFlags: u16 { + const REFERENCED = 0x001; + const ASSIGNED = 0x002; + const PARAMETER = 0x004; + const ANNOTATED = 0x008; + const IMPORTED = 0x010; + const NONLOCAL = 0x020; + // 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. + const ASSIGNED_IN_COMPREHENSION = 0x040; + // indicates that the symbol is used a bound iterator variable. We distinguish this case + // from normal assignment to detect unallowed re-assignment to iterator variables. + const ITER = 0x080; + /// indicates that the symbol is a free variable in a class method from the scope that the + /// class is defined in, e.g.: + /// ```python + /// def foo(x): + /// class A: + /// def method(self): + /// return x // is_free_class + /// ``` + const FREE_CLASS = 0x100; + const BOUND = Self::ASSIGNED.bits | Self::PARAMETER.bits | Self::IMPORTED.bits | Self::ITER.bits; + } +} + /// A single symbol in a table. Has various properties such as the scope /// of the symbol, and also the various uses of the symbol. #[derive(Debug, Clone)] From de10ce2d8971abe2f926e1fb63ffc48f9ac347cc Mon Sep 17 00:00:00 2001 From: yt2b Date: Sun, 27 Nov 2022 23:43:26 +0900 Subject: [PATCH 3/6] Reduce nesting of match expression --- compiler/codegen/src/symboltable.rs | 59 +++++++++++++---------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/compiler/codegen/src/symboltable.rs b/compiler/codegen/src/symboltable.rs index 24ee5ea20f..f44977d770 100644 --- a/compiler/codegen/src/symboltable.rs +++ b/compiler/codegen/src/symboltable.rs @@ -1178,38 +1178,33 @@ impl SymbolTableBuilder { let symbol = if let Some(symbol) = table.symbols.get_mut(name.as_ref()) { // Role already set.. match role { - SymbolUsage::Global => { - if !symbol.is_global() { - if symbol.is_parameter { - return Err(SymbolTableError { - error: format!("name '{}' is parameter and global", name), - location, - }); - } - if symbol.is_referenced { - return Err(SymbolTableError { - error: format!( - "name '{}' is used prior to global declaration", - name - ), - location, - }); - } - if symbol.is_annotated { - return Err(SymbolTableError { - error: format!("annotated name '{}' can't be global", name), - location, - }); - } - if symbol.is_assigned { - return Err(SymbolTableError { - error: format!( - "name '{}' is assigned to before global declaration", - name - ), - location, - }); - } + SymbolUsage::Global if !symbol.is_global() => { + if symbol.is_parameter { + return Err(SymbolTableError { + error: format!("name '{}' is parameter and global", name), + location, + }); + } + if symbol.is_referenced { + return Err(SymbolTableError { + error: format!("name '{}' is used prior to global declaration", name), + location, + }); + } + if symbol.is_annotated { + return Err(SymbolTableError { + error: format!("annotated name '{}' can't be global", name), + location, + }); + } + if symbol.is_assigned { + return Err(SymbolTableError { + error: format!( + "name '{}' is assigned to before global declaration", + name + ), + location, + }); } } SymbolUsage::Nonlocal => { From dbf27a955de4fa38e80c9559c5849603c68640ee Mon Sep 17 00:00:00 2001 From: yt2b Date: Sun, 27 Nov 2022 23:46:45 +0900 Subject: [PATCH 4/6] Fix comment --- compiler/codegen/src/symboltable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/codegen/src/symboltable.rs b/compiler/codegen/src/symboltable.rs index f44977d770..7bf8f50c8c 100644 --- a/compiler/codegen/src/symboltable.rs +++ b/compiler/codegen/src/symboltable.rs @@ -1260,7 +1260,7 @@ impl SymbolTableBuilder { table.symbols.entry(name.into_owned()).or_insert(symbol) }; - // Set proper flags on symbol: + // Set proper scope and flags on symbol: match role { SymbolUsage::Nonlocal => { symbol.scope = SymbolScope::Free; From a5ef816c412a1d49dbd59ee2eb673d0d80d7353d Mon Sep 17 00:00:00 2001 From: yt2b Date: Mon, 28 Nov 2022 00:46:10 +0900 Subject: [PATCH 5/6] Replace bool variable with SymbolFlags --- compiler/codegen/src/compile.rs | 8 ++- compiler/codegen/src/symboltable.rs | 100 ++++++++++------------------ vm/src/stdlib/symtable.rs | 16 +++-- 3 files changed, 49 insertions(+), 75 deletions(-) diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs index 7bf616923f..602f9b7db2 100644 --- a/compiler/codegen/src/compile.rs +++ b/compiler/codegen/src/compile.rs @@ -8,7 +8,7 @@ use crate::{ error::{CodegenError, CodegenErrorType}, ir, - symboltable::{self, SymbolScope, SymbolTable}, + symboltable::{self, SymbolFlags, SymbolScope, SymbolTable}, IndexSet, }; use itertools::Itertools; @@ -272,7 +272,9 @@ impl Compiler { let freevar_cache = table .symbols .iter() - .filter(|(_, s)| s.scope == SymbolScope::Free || s.is_free_class) + .filter(|(_, s)| { + s.scope == SymbolScope::Free || s.flags.contains(SymbolFlags::FREE_CLASS) + }) .map(|(var, _)| var.clone()) .collect(); @@ -1209,7 +1211,7 @@ impl Compiler { let vars = match symbol.scope { SymbolScope::Free => &parent_code.freevar_cache, SymbolScope::Cell => &parent_code.cellvar_cache, - _ if symbol.is_free_class => &parent_code.freevar_cache, + _ if symbol.flags.contains(SymbolFlags::FREE_CLASS) => &parent_code.freevar_cache, x => unreachable!( "var {} in a {:?} should be free or cell but it's {:?}", var, table.typ, x diff --git a/compiler/codegen/src/symboltable.rs b/compiler/codegen/src/symboltable.rs index 7bf8f50c8c..7de315b8d3 100644 --- a/compiler/codegen/src/symboltable.rs +++ b/compiler/codegen/src/symboltable.rs @@ -129,31 +129,7 @@ pub struct Symbol { pub name: String, // pub table: SymbolTableRef, pub scope: SymbolScope, - // TODO: Use bitflags replace - pub is_referenced: bool, - pub is_assigned: bool, - pub is_parameter: bool, - pub is_annotated: bool, - pub is_imported: bool, - pub is_nonlocal: 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, - - // indicates 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, - - /// indicates that the symbol is a free variable in a class method from the scope that the - /// class is defined in, e.g.: - /// ```python - /// def foo(x): - /// class A: - /// def method(self): - /// return x // is_free_class - /// ``` - pub is_free_class: bool, + pub flags: SymbolFlags, } impl Symbol { @@ -162,15 +138,7 @@ impl Symbol { name: name.to_owned(), // table, scope: SymbolScope::Unknown, - is_referenced: false, - is_assigned: false, - is_parameter: false, - is_annotated: false, - is_imported: false, - is_nonlocal: false, - is_assign_namedexpr_in_comprehension: false, - is_iter: false, - is_free_class: false, + flags: SymbolFlags::empty(), } } @@ -186,7 +154,7 @@ impl Symbol { } pub fn is_bound(&self) -> bool { - self.is_assigned || self.is_parameter || self.is_imported || self.is_iter + !(self.flags & SymbolFlags::BOUND).is_empty() } } @@ -328,7 +296,11 @@ impl SymbolTableAnalyzer { st_typ: SymbolTableType, sub_tables: &mut [SymbolTable], ) -> SymbolTableResult { - if symbol.is_assign_namedexpr_in_comprehension && st_typ == SymbolTableType::Comprehension { + if symbol + .flags + .contains(SymbolFlags::ASSIGNED_IN_COMPREHENSION) + && 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. @@ -416,10 +388,10 @@ impl SymbolTableAnalyzer { for (table, typ) in self.tables.iter_mut().rev().take(decl_depth) { if let SymbolTableType::Class = typ { if let Some(free_class) = table.get_mut(name) { - free_class.is_free_class = true; + free_class.flags.insert(SymbolFlags::FREE_CLASS) } else { let mut symbol = Symbol::new(name); - symbol.is_free_class = true; + symbol.flags.insert(SymbolFlags::FREE_CLASS); symbol.scope = SymbolScope::Free; table.insert(name.to_owned(), symbol); } @@ -443,7 +415,7 @@ impl SymbolTableAnalyzer { ) -> Option { sub_tables.iter().find_map(|st| { st.symbols.get(name).and_then(|sym| { - if sym.scope == SymbolScope::Free || sym.is_free_class { + if sym.scope == SymbolScope::Free || sym.flags.contains(SymbolFlags::FREE_CLASS) { if st_typ == SymbolTableType::Class && name != "__class__" { None } else { @@ -474,7 +446,7 @@ impl SymbolTableAnalyzer { let table_type = last.1; // it is not allowed to use an iterator variable as assignee in a named expression - if symbol.is_iter { + if symbol.flags.contains(SymbolFlags::ITER) { return Err(SymbolTableError { error: format!( "assignment expression cannot rebind comprehension iteration variable {}", @@ -501,7 +473,7 @@ impl SymbolTableAnalyzer { if let Some(parent_symbol) = symbols.get_mut(&symbol.name) { if let SymbolScope::Unknown = parent_symbol.scope { // this information is new, as the assignment is done in inner scope - parent_symbol.is_assigned = true; + parent_symbol.flags.insert(SymbolFlags::ASSIGNED); } symbol.scope = if parent_symbol.is_global() { @@ -520,7 +492,7 @@ impl SymbolTableAnalyzer { match symbols.get_mut(&symbol.name) { Some(parent_symbol) => { // check if assignee is an iterator in top scope - if parent_symbol.is_iter { + if parent_symbol.flags.contains(SymbolFlags::ITER) { return Err(SymbolTableError { error: format!("assignment expression cannot rebind comprehension iteration variable {}", symbol.name), // TODO: accurate location info, somehow @@ -529,7 +501,7 @@ impl SymbolTableAnalyzer { } // we synthesize the assignment to the symbol from inner scope - parent_symbol.is_assigned = true; // more checks are required + parent_symbol.flags.insert(SymbolFlags::ASSIGNED); // more checks are required } None => { // extend the scope of the inner symbol @@ -1176,28 +1148,29 @@ impl SymbolTableBuilder { // Some checks for the symbol that present on this scope level: let symbol = if let Some(symbol) = table.symbols.get_mut(name.as_ref()) { + let flags = &symbol.flags; // Role already set.. match role { SymbolUsage::Global if !symbol.is_global() => { - if symbol.is_parameter { + if flags.contains(SymbolFlags::PARAMETER) { return Err(SymbolTableError { error: format!("name '{}' is parameter and global", name), location, }); } - if symbol.is_referenced { + if flags.contains(SymbolFlags::REFERENCED) { return Err(SymbolTableError { error: format!("name '{}' is used prior to global declaration", name), location, }); } - if symbol.is_annotated { + if flags.contains(SymbolFlags::ANNOTATED) { return Err(SymbolTableError { error: format!("annotated name '{}' can't be global", name), location, }); } - if symbol.is_assigned { + if flags.contains(SymbolFlags::ASSIGNED) { return Err(SymbolTableError { error: format!( "name '{}' is assigned to before global declaration", @@ -1208,25 +1181,25 @@ impl SymbolTableBuilder { } } SymbolUsage::Nonlocal => { - if symbol.is_parameter { + if flags.contains(SymbolFlags::PARAMETER) { return Err(SymbolTableError { error: format!("name '{}' is parameter and nonlocal", name), location, }); } - if symbol.is_referenced { + if flags.contains(SymbolFlags::REFERENCED) { return Err(SymbolTableError { error: format!("name '{}' is used prior to nonlocal declaration", name), location, }); } - if symbol.is_annotated { + if flags.contains(SymbolFlags::ANNOTATED) { return Err(SymbolTableError { error: format!("annotated name '{}' can't be nonlocal", name), location, }); } - if symbol.is_assigned { + if flags.contains(SymbolFlags::ASSIGNED) { return Err(SymbolTableError { error: format!( "name '{}' is assigned to before nonlocal declaration", @@ -1261,47 +1234,44 @@ impl SymbolTableBuilder { }; // Set proper scope and flags on symbol: + let flags = &mut symbol.flags; match role { SymbolUsage::Nonlocal => { symbol.scope = SymbolScope::Free; - symbol.is_nonlocal = true; + flags.insert(SymbolFlags::NONLOCAL); } SymbolUsage::Imported => { - symbol.is_assigned = true; - symbol.is_imported = true; + flags.insert(SymbolFlags::ASSIGNED | SymbolFlags::IMPORTED); } SymbolUsage::Parameter => { - symbol.is_parameter = true; + flags.insert(SymbolFlags::PARAMETER); } SymbolUsage::AnnotationParameter => { - symbol.is_parameter = true; - symbol.is_annotated = true; + flags.insert(SymbolFlags::PARAMETER | SymbolFlags::ANNOTATED); } SymbolUsage::AnnotationAssigned => { - symbol.is_assigned = true; - symbol.is_annotated = true; + flags.insert(SymbolFlags::ASSIGNED | SymbolFlags::ANNOTATED); } SymbolUsage::Assigned => { - symbol.is_assigned = true; + flags.insert(SymbolFlags::ASSIGNED); } SymbolUsage::AssignedNamedExprInCompr => { - symbol.is_assigned = true; - symbol.is_assign_namedexpr_in_comprehension = true; + flags.insert(SymbolFlags::ASSIGNED | SymbolFlags::ASSIGNED_IN_COMPREHENSION); } SymbolUsage::Global => { symbol.scope = SymbolScope::GlobalExplicit; } SymbolUsage::Used => { - symbol.is_referenced = true; + flags.insert(SymbolFlags::REFERENCED); } SymbolUsage::Iter => { - symbol.is_iter = true; + flags.insert(SymbolFlags::ITER); } } // and even more checking // it is not allowed to assign to iterator variables (by named expressions) - if symbol.is_iter && symbol.is_assigned + if flags.contains(SymbolFlags::ITER | SymbolFlags::ASSIGNED) /*&& symbol.is_assign_namedexpr_in_comprehension*/ { return Err(SymbolTableError { diff --git a/vm/src/stdlib/symtable.rs b/vm/src/stdlib/symtable.rs index 80147dee9c..dff69d8cc0 100644 --- a/vm/src/stdlib/symtable.rs +++ b/vm/src/stdlib/symtable.rs @@ -5,7 +5,9 @@ mod symtable { use crate::{ builtins::PyStrRef, compiler, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, }; - use rustpython_codegen::symboltable::{Symbol, SymbolScope, SymbolTable, SymbolTableType}; + use rustpython_codegen::symboltable::{ + Symbol, SymbolFlags, SymbolScope, SymbolTable, SymbolTableType, + }; use std::fmt; #[pyfunction] @@ -179,7 +181,7 @@ mod symtable { #[pymethod] fn is_imported(&self) -> bool { - self.symbol.is_imported + self.symbol.flags.contains(SymbolFlags::IMPORTED) } #[pymethod] @@ -190,22 +192,22 @@ mod symtable { #[pymethod] fn is_nonlocal(&self) -> bool { - self.symbol.is_nonlocal + self.symbol.flags.contains(SymbolFlags::NONLOCAL) } #[pymethod] fn is_referenced(&self) -> bool { - self.symbol.is_referenced + self.symbol.flags.contains(SymbolFlags::REFERENCED) } #[pymethod] fn is_assigned(&self) -> bool { - self.symbol.is_assigned + self.symbol.flags.contains(SymbolFlags::ASSIGNED) } #[pymethod] fn is_parameter(&self) -> bool { - self.symbol.is_parameter + self.symbol.flags.contains(SymbolFlags::PARAMETER) } #[pymethod] @@ -220,7 +222,7 @@ mod symtable { #[pymethod] fn is_annotated(&self) -> bool { - self.symbol.is_annotated + self.symbol.flags.contains(SymbolFlags::ANNOTATED) } #[pymethod] From dd833083961bd64468598a4ca9a45db4a4364b5d Mon Sep 17 00:00:00 2001 From: yt2b Date: Mon, 28 Nov 2022 20:48:03 +0900 Subject: [PATCH 6/6] Use intersects --- compiler/codegen/src/symboltable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/codegen/src/symboltable.rs b/compiler/codegen/src/symboltable.rs index 7de315b8d3..9662707a3a 100644 --- a/compiler/codegen/src/symboltable.rs +++ b/compiler/codegen/src/symboltable.rs @@ -154,7 +154,7 @@ impl Symbol { } pub fn is_bound(&self) -> bool { - !(self.flags & SymbolFlags::BOUND).is_empty() + self.flags.intersects(SymbolFlags::BOUND) } }