Skip to content

Use bitflags in Symbol #4298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 28, 2022
Merged

Use bitflags in Symbol #4298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f23ea5e
Add bitflags
yt2b Nov 27, 2022
1ffb652
Add SymbolFlags
yt2b Nov 27, 2022
de10ce2
Reduce nesting of match expression
yt2b Nov 27, 2022
dbf27a9
Fix comment
yt2b Nov 27, 2022
a5ef816
Replace bool variable with SymbolFlags
yt2b Nov 27, 2022
dd83308
Use intersects
yt2b Nov 28, 2022
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions compiler/codegen/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 5 additions & 3 deletions compiler/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use crate::{
error::{CodegenError, CodegenErrorType},
ir,
symboltable::{self, SymbolScope, SymbolTable},
symboltable::{self, SymbolFlags, SymbolScope, SymbolTable},
IndexSet,
};
use itertools::Itertools;
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down
181 changes: 87 additions & 94 deletions compiler/codegen/src/symboltable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -94,38 +95,41 @@ 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)]
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 {
Expand All @@ -134,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(),
}
}

Expand All @@ -158,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.intersects(SymbolFlags::BOUND)
}
}

Expand Down Expand Up @@ -300,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.
Expand Down Expand Up @@ -388,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);
}
Expand All @@ -415,7 +415,7 @@ impl SymbolTableAnalyzer {
) -> Option<SymbolScope> {
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 {
Expand Down Expand Up @@ -446,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 {}",
Expand All @@ -473,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() {
Expand All @@ -492,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
Expand All @@ -501,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
Expand Down Expand Up @@ -1148,62 +1148,58 @@ 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 {
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 flags.contains(SymbolFlags::PARAMETER) {
return Err(SymbolTableError {
error: format!("name '{}' is parameter and global", name),
location,
});
}
if flags.contains(SymbolFlags::REFERENCED) {
return Err(SymbolTableError {
error: format!("name '{}' is used prior to global declaration", name),
location,
});
}
if flags.contains(SymbolFlags::ANNOTATED) {
return Err(SymbolTableError {
error: format!("annotated name '{}' can't be global", name),
location,
});
}
if flags.contains(SymbolFlags::ASSIGNED) {
return Err(SymbolTableError {
error: format!(
"name '{}' is assigned to before global declaration",
name
),
location,
});
}
}
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",
Expand Down Expand Up @@ -1237,48 +1233,45 @@ impl SymbolTableBuilder {
table.symbols.entry(name.into_owned()).or_insert(symbol)
};

// Set proper flags on symbol:
// 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 {
Expand Down
Loading