Skip to content

Commit b9ed63e

Browse files
DimitrisJimDimitrisJim
authored
Merge pull request RustPython#4298 from yt2b/use_bitflags
Use bitflags in `Symbol`
2 parents ec36489 + dd83308 commit b9ed63e

File tree

5 files changed

+103
-104
lines changed

5 files changed

+103
-104
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎compiler/codegen/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ rustpython-ast = { path = "../ast", features = ["unparse"] }
1212
rustpython-compiler-core = { path = "../core", version = "0.1.1" }
1313

1414
ahash = "0.7.6"
15+
bitflags = "1.3.2"
1516
indexmap = "1.8.1"
1617
itertools = "0.10.3"
1718
log = "0.4.16"

‎compiler/codegen/src/compile.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use crate::{
99
error::{CodegenError, CodegenErrorType},
1010
ir,
11-
symboltable::{self, SymbolScope, SymbolTable},
11+
symboltable::{self, SymbolFlags, SymbolScope, SymbolTable},
1212
IndexSet,
1313
};
1414
use itertools::Itertools;
@@ -272,7 +272,9 @@ impl Compiler {
272272
let freevar_cache = table
273273
.symbols
274274
.iter()
275-
.filter(|(_, s)| s.scope == SymbolScope::Free || s.is_free_class)
275+
.filter(|(_, s)| {
276+
s.scope == SymbolScope::Free || s.flags.contains(SymbolFlags::FREE_CLASS)
277+
})
276278
.map(|(var, _)| var.clone())
277279
.collect();
278280

@@ -1209,7 +1211,7 @@ impl Compiler {
12091211
let vars = match symbol.scope {
12101212
SymbolScope::Free => &parent_code.freevar_cache,
12111213
SymbolScope::Cell => &parent_code.cellvar_cache,
1212-
_ if symbol.is_free_class => &parent_code.freevar_cache,
1214+
_ if symbol.flags.contains(SymbolFlags::FREE_CLASS) => &parent_code.freevar_cache,
12131215
x => unreachable!(
12141216
"var {} in a {:?} should be free or cell but it's {:?}",
12151217
var, table.typ, x

‎compiler/codegen/src/symboltable.rs‎

Lines changed: 87 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use crate::{
1111
error::{CodegenError, CodegenErrorType},
1212
IndexMap,
1313
};
14+
use bitflags::bitflags;
1415
use rustpython_ast as ast;
1516
use rustpython_compiler_core::Location;
1617
use std::{borrow::Cow, fmt};
@@ -94,38 +95,41 @@ pub enum SymbolScope {
9495
Cell,
9596
}
9697

98+
bitflags! {
99+
pub struct SymbolFlags: u16 {
100+
const REFERENCED = 0x001;
101+
const ASSIGNED = 0x002;
102+
const PARAMETER = 0x004;
103+
const ANNOTATED = 0x008;
104+
const IMPORTED = 0x010;
105+
const NONLOCAL = 0x020;
106+
// indicates if the symbol gets a value assigned by a named expression in a comprehension
107+
// this is required to correct the scope in the analysis.
108+
const ASSIGNED_IN_COMPREHENSION = 0x040;
109+
// indicates that the symbol is used a bound iterator variable. We distinguish this case
110+
// from normal assignment to detect unallowed re-assignment to iterator variables.
111+
const ITER = 0x080;
112+
/// indicates that the symbol is a free variable in a class method from the scope that the
113+
/// class is defined in, e.g.:
114+
/// ```python
115+
/// def foo(x):
116+
/// class A:
117+
/// def method(self):
118+
/// return x // is_free_class
119+
/// ```
120+
const FREE_CLASS = 0x100;
121+
const BOUND = Self::ASSIGNED.bits | Self::PARAMETER.bits | Self::IMPORTED.bits | Self::ITER.bits;
122+
}
123+
}
124+
97125
/// A single symbol in a table. Has various properties such as the scope
98126
/// of the symbol, and also the various uses of the symbol.
99127
#[derive(Debug, Clone)]
100128
pub struct Symbol {
101129
pub name: String,
102130
// pub table: SymbolTableRef,
103131
pub scope: SymbolScope,
104-
// TODO: Use bitflags replace
105-
pub is_referenced: bool,
106-
pub is_assigned: bool,
107-
pub is_parameter: bool,
108-
pub is_annotated: bool,
109-
pub is_imported: bool,
110-
pub is_nonlocal: bool,
111-
112-
// indicates if the symbol gets a value assigned by a named expression in a comprehension
113-
// this is required to correct the scope in the analysis.
114-
pub is_assign_namedexpr_in_comprehension: bool,
115-
116-
// indicates that the symbol is used a bound iterator variable. We distinguish this case
117-
// from normal assignment to detect unallowed re-assignment to iterator variables.
118-
pub is_iter: bool,
119-
120-
/// indicates that the symbol is a free variable in a class method from the scope that the
121-
/// class is defined in, e.g.:
122-
/// ```python
123-
/// def foo(x):
124-
/// class A:
125-
/// def method(self):
126-
/// return x // is_free_class
127-
/// ```
128-
pub is_free_class: bool,
132+
pub flags: SymbolFlags,
129133
}
130134

131135
impl Symbol {
@@ -134,15 +138,7 @@ impl Symbol {
134138
name: name.to_owned(),
135139
// table,
136140
scope: SymbolScope::Unknown,
137-
is_referenced: false,
138-
is_assigned: false,
139-
is_parameter: false,
140-
is_annotated: false,
141-
is_imported: false,
142-
is_nonlocal: false,
143-
is_assign_namedexpr_in_comprehension: false,
144-
is_iter: false,
145-
is_free_class: false,
141+
flags: SymbolFlags::empty(),
146142
}
147143
}
148144

@@ -158,7 +154,7 @@ impl Symbol {
158154
}
159155

160156
pub fn is_bound(&self) -> bool {
161-
self.is_assigned || self.is_parameter || self.is_imported || self.is_iter
157+
self.flags.intersects(SymbolFlags::BOUND)
162158
}
163159
}
164160

@@ -300,7 +296,11 @@ impl SymbolTableAnalyzer {
300296
st_typ: SymbolTableType,
301297
sub_tables: &mut [SymbolTable],
302298
) -> SymbolTableResult {
303-
if symbol.is_assign_namedexpr_in_comprehension && st_typ == SymbolTableType::Comprehension {
299+
if symbol
300+
.flags
301+
.contains(SymbolFlags::ASSIGNED_IN_COMPREHENSION)
302+
&& st_typ == SymbolTableType::Comprehension
303+
{
304304
// propagate symbol to next higher level that can hold it,
305305
// i.e., function or module. Comprehension is skipped and
306306
// Class is not allowed and detected as error.
@@ -388,10 +388,10 @@ impl SymbolTableAnalyzer {
388388
for (table, typ) in self.tables.iter_mut().rev().take(decl_depth) {
389389
if let SymbolTableType::Class = typ {
390390
if let Some(free_class) = table.get_mut(name) {
391-
free_class.is_free_class = true;
391+
free_class.flags.insert(SymbolFlags::FREE_CLASS)
392392
} else {
393393
let mut symbol = Symbol::new(name);
394-
symbol.is_free_class = true;
394+
symbol.flags.insert(SymbolFlags::FREE_CLASS);
395395
symbol.scope = SymbolScope::Free;
396396
table.insert(name.to_owned(), symbol);
397397
}
@@ -415,7 +415,7 @@ impl SymbolTableAnalyzer {
415415
) -> Option<SymbolScope> {
416416
sub_tables.iter().find_map(|st| {
417417
st.symbols.get(name).and_then(|sym| {
418-
if sym.scope == SymbolScope::Free || sym.is_free_class {
418+
if sym.scope == SymbolScope::Free || sym.flags.contains(SymbolFlags::FREE_CLASS) {
419419
if st_typ == SymbolTableType::Class && name != "__class__" {
420420
None
421421
} else {
@@ -446,7 +446,7 @@ impl SymbolTableAnalyzer {
446446
let table_type = last.1;
447447

448448
// it is not allowed to use an iterator variable as assignee in a named expression
449-
if symbol.is_iter {
449+
if symbol.flags.contains(SymbolFlags::ITER) {
450450
return Err(SymbolTableError {
451451
error: format!(
452452
"assignment expression cannot rebind comprehension iteration variable {}",
@@ -473,7 +473,7 @@ impl SymbolTableAnalyzer {
473473
if let Some(parent_symbol) = symbols.get_mut(&symbol.name) {
474474
if let SymbolScope::Unknown = parent_symbol.scope {
475475
// this information is new, as the assignment is done in inner scope
476-
parent_symbol.is_assigned = true;
476+
parent_symbol.flags.insert(SymbolFlags::ASSIGNED);
477477
}
478478

479479
symbol.scope = if parent_symbol.is_global() {
@@ -492,7 +492,7 @@ impl SymbolTableAnalyzer {
492492
match symbols.get_mut(&symbol.name) {
493493
Some(parent_symbol) => {
494494
// check if assignee is an iterator in top scope
495-
if parent_symbol.is_iter {
495+
if parent_symbol.flags.contains(SymbolFlags::ITER) {
496496
return Err(SymbolTableError {
497497
error: format!("assignment expression cannot rebind comprehension iteration variable {}", symbol.name),
498498
// TODO: accurate location info, somehow
@@ -501,7 +501,7 @@ impl SymbolTableAnalyzer {
501501
}
502502

503503
// we synthesize the assignment to the symbol from inner scope
504-
parent_symbol.is_assigned = true; // more checks are required
504+
parent_symbol.flags.insert(SymbolFlags::ASSIGNED); // more checks are required
505505
}
506506
None => {
507507
// extend the scope of the inner symbol
@@ -1148,62 +1148,58 @@ impl SymbolTableBuilder {
11481148

11491149
// Some checks for the symbol that present on this scope level:
11501150
let symbol = if let Some(symbol) = table.symbols.get_mut(name.as_ref()) {
1151+
let flags = &symbol.flags;
11511152
// Role already set..
11521153
match role {
1153-
SymbolUsage::Global => {
1154-
if !symbol.is_global() {
1155-
if symbol.is_parameter {
1156-
return Err(SymbolTableError {
1157-
error: format!("name '{}' is parameter and global", name),
1158-
location,
1159-
});
1160-
}
1161-
if symbol.is_referenced {
1162-
return Err(SymbolTableError {
1163-
error: format!(
1164-
"name '{}' is used prior to global declaration",
1165-
name
1166-
),
1167-
location,
1168-
});
1169-
}
1170-
if symbol.is_annotated {
1171-
return Err(SymbolTableError {
1172-
error: format!("annotated name '{}' can't be global", name),
1173-
location,
1174-
});
1175-
}
1176-
if symbol.is_assigned {
1177-
return Err(SymbolTableError {
1178-
error: format!(
1179-
"name '{}' is assigned to before global declaration",
1180-
name
1181-
),
1182-
location,
1183-
});
1184-
}
1154+
SymbolUsage::Global if !symbol.is_global() => {
1155+
if flags.contains(SymbolFlags::PARAMETER) {
1156+
return Err(SymbolTableError {
1157+
error: format!("name '{}' is parameter and global", name),
1158+
location,
1159+
});
1160+
}
1161+
if flags.contains(SymbolFlags::REFERENCED) {
1162+
return Err(SymbolTableError {
1163+
error: format!("name '{}' is used prior to global declaration", name),
1164+
location,
1165+
});
1166+
}
1167+
if flags.contains(SymbolFlags::ANNOTATED) {
1168+
return Err(SymbolTableError {
1169+
error: format!("annotated name '{}' can't be global", name),
1170+
location,
1171+
});
1172+
}
1173+
if flags.contains(SymbolFlags::ASSIGNED) {
1174+
return Err(SymbolTableError {
1175+
error: format!(
1176+
"name '{}' is assigned to before global declaration",
1177+
name
1178+
),
1179+
location,
1180+
});
11851181
}
11861182
}
11871183
SymbolUsage::Nonlocal => {
1188-
if symbol.is_parameter {
1184+
if flags.contains(SymbolFlags::PARAMETER) {
11891185
return Err(SymbolTableError {
11901186
error: format!("name '{}' is parameter and nonlocal", name),
11911187
location,
11921188
});
11931189
}
1194-
if symbol.is_referenced {
1190+
if flags.contains(SymbolFlags::REFERENCED) {
11951191
return Err(SymbolTableError {
11961192
error: format!("name '{}' is used prior to nonlocal declaration", name),
11971193
location,
11981194
});
11991195
}
1200-
if symbol.is_annotated {
1196+
if flags.contains(SymbolFlags::ANNOTATED) {
12011197
return Err(SymbolTableError {
12021198
error: format!("annotated name '{}' can't be nonlocal", name),
12031199
location,
12041200
});
12051201
}
1206-
if symbol.is_assigned {
1202+
if flags.contains(SymbolFlags::ASSIGNED) {
12071203
return Err(SymbolTableError {
12081204
error: format!(
12091205
"name '{}' is assigned to before nonlocal declaration",
@@ -1237,48 +1233,45 @@ impl SymbolTableBuilder {
12371233
table.symbols.entry(name.into_owned()).or_insert(symbol)
12381234
};
12391235

1240-
// Set proper flags on symbol:
1236+
// Set proper scope and flags on symbol:
1237+
let flags = &mut symbol.flags;
12411238
match role {
12421239
SymbolUsage::Nonlocal => {
12431240
symbol.scope = SymbolScope::Free;
1244-
symbol.is_nonlocal = true;
1241+
flags.insert(SymbolFlags::NONLOCAL);
12451242
}
12461243
SymbolUsage::Imported => {
1247-
symbol.is_assigned = true;
1248-
symbol.is_imported = true;
1244+
flags.insert(SymbolFlags::ASSIGNED | SymbolFlags::IMPORTED);
12491245
}
12501246
SymbolUsage::Parameter => {
1251-
symbol.is_parameter = true;
1247+
flags.insert(SymbolFlags::PARAMETER);
12521248
}
12531249
SymbolUsage::AnnotationParameter => {
1254-
symbol.is_parameter = true;
1255-
symbol.is_annotated = true;
1250+
flags.insert(SymbolFlags::PARAMETER | SymbolFlags::ANNOTATED);
12561251
}
12571252
SymbolUsage::AnnotationAssigned => {
1258-
symbol.is_assigned = true;
1259-
symbol.is_annotated = true;
1253+
flags.insert(SymbolFlags::ASSIGNED | SymbolFlags::ANNOTATED);
12601254
}
12611255
SymbolUsage::Assigned => {
1262-
symbol.is_assigned = true;
1256+
flags.insert(SymbolFlags::ASSIGNED);
12631257
}
12641258
SymbolUsage::AssignedNamedExprInCompr => {
1265-
symbol.is_assigned = true;
1266-
symbol.is_assign_namedexpr_in_comprehension = true;
1259+
flags.insert(SymbolFlags::ASSIGNED | SymbolFlags::ASSIGNED_IN_COMPREHENSION);
12671260
}
12681261
SymbolUsage::Global => {
12691262
symbol.scope = SymbolScope::GlobalExplicit;
12701263
}
12711264
SymbolUsage::Used => {
1272-
symbol.is_referenced = true;
1265+
flags.insert(SymbolFlags::REFERENCED);
12731266
}
12741267
SymbolUsage::Iter => {
1275-
symbol.is_iter = true;
1268+
flags.insert(SymbolFlags::ITER);
12761269
}
12771270
}
12781271

12791272
// and even more checking
12801273
// it is not allowed to assign to iterator variables (by named expressions)
1281-
if symbol.is_iter && symbol.is_assigned
1274+
if flags.contains(SymbolFlags::ITER | SymbolFlags::ASSIGNED)
12821275
/*&& symbol.is_assign_namedexpr_in_comprehension*/
12831276
{
12841277
return Err(SymbolTableError {

0 commit comments

Comments
 (0)