Skip to content

Commit 0009dd6

Browse files
authored
Clean up compiler parity leftovers (RustPython#8174)
1 parent c449564 commit 0009dd6

3 files changed

Lines changed: 93 additions & 209 deletions

File tree

crates/codegen/src/error.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ pub enum CodegenErrorType {
8888
ConflictingNameBindPattern,
8989
/// break/continue/return inside except* block
9090
BreakContinueReturnInExceptStar,
91-
NotImplementedYet, // RustPython marker for unimplemented features
9291
}
9392

9493
impl core::error::Error for CodegenErrorType {}
@@ -173,9 +172,6 @@ impl fmt::Display for CodegenErrorType {
173172
"'break', 'continue' and 'return' cannot appear in an except* block"
174173
)
175174
}
176-
Self::NotImplementedYet => {
177-
write!(f, "RustPython does not implement this feature yet")
178-
}
179175
}
180176
}
181177
}

crates/codegen/src/symboltable.rs

Lines changed: 91 additions & 204 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ pub struct Symbol {
325325
pub name: String,
326326
pub scope: SymbolScope,
327327
pub flags: SymbolFlags,
328+
pub location: Option<SourceLocation>,
328329
}
329330

330331
impl Symbol {
@@ -334,6 +335,7 @@ impl Symbol {
334335
// table,
335336
scope: SymbolScope::Unknown,
336337
flags: SymbolFlags::empty(),
338+
location: None,
337339
}
338340
}
339341

@@ -777,117 +779,98 @@ impl SymbolTableAnalyzer {
777779
sub_tables: &[SymbolTable],
778780
class_entry: Option<&SymbolMap>,
779781
) -> SymbolTableResult {
780-
if symbol
781-
.flags
782-
.contains(SymbolFlags::ASSIGNED_IN_COMPREHENSION)
783-
&& st_typ == CompilerScope::Comprehension
784-
{
785-
// propagate symbol to next higher level that can hold it,
786-
// i.e., function or module. Comprehension is skipped and
787-
// Class is not allowed and detected as error.
788-
self.analyze_symbol_comprehension(symbol, 0)?
789-
} else {
790-
match symbol.scope {
791-
SymbolScope::Free => {
792-
if !self.tables.as_ref().is_empty() {
793-
let scope_depth = self.tables.as_ref().len();
794-
// check if the name is already defined in any outer scope
795-
if scope_depth < 2
796-
|| self.found_in_outer_scope(
797-
&symbol.name,
798-
st_typ,
799-
skip_enclosing_function_scope,
800-
) != Some(SymbolScope::Free)
801-
{
802-
return Err(SymbolTableError {
803-
error: format!("no binding for nonlocal '{}' found", symbol.name),
804-
// TODO: accurate location info, somehow
805-
location: None,
806-
});
807-
}
808-
// Check if the nonlocal binding refers to a type parameter
809-
if symbol.flags.contains(SymbolFlags::NONLOCAL) {
810-
for (symbols, _typ, _skip) in self.tables.iter().rev() {
811-
if let Some(sym) = symbols.get(&symbol.name) {
812-
if sym.flags.contains(SymbolFlags::TYPE_PARAM) {
813-
return Err(SymbolTableError {
814-
error: format!(
815-
"nonlocal binding not allowed for type parameter '{}'",
816-
symbol.name
817-
),
818-
location: None,
819-
});
820-
}
821-
if sym.is_bound() {
822-
break;
823-
}
782+
match symbol.scope {
783+
SymbolScope::Free => {
784+
if !self.tables.as_ref().is_empty() {
785+
let scope_depth = self.tables.as_ref().len();
786+
// check if the name is already defined in any outer scope
787+
if scope_depth < 2
788+
|| self.found_in_outer_scope(
789+
&symbol.name,
790+
st_typ,
791+
skip_enclosing_function_scope,
792+
) != Some(SymbolScope::Free)
793+
{
794+
return Err(SymbolTableError {
795+
error: format!("no binding for nonlocal '{}' found", symbol.name),
796+
location: symbol.location,
797+
});
798+
}
799+
// Check if the nonlocal binding refers to a type parameter
800+
if symbol.flags.contains(SymbolFlags::NONLOCAL) {
801+
for (symbols, _typ, _skip) in self.tables.iter().rev() {
802+
if let Some(sym) = symbols.get(&symbol.name) {
803+
if sym.flags.contains(SymbolFlags::TYPE_PARAM) {
804+
return Err(SymbolTableError {
805+
error: format!(
806+
"nonlocal binding not allowed for type parameter '{}'",
807+
symbol.name
808+
),
809+
location: symbol.location,
810+
});
811+
}
812+
if sym.is_bound() {
813+
break;
824814
}
825815
}
826816
}
827-
} else {
828-
return Err(SymbolTableError {
829-
error: format!(
830-
"nonlocal {} defined at place without an enclosing scope",
831-
symbol.name
832-
),
833-
// TODO: accurate location info, somehow
834-
location: None,
835-
});
836817
}
818+
} else {
819+
return Err(SymbolTableError {
820+
error: format!(
821+
"nonlocal {} defined at place without an enclosing scope",
822+
symbol.name
823+
),
824+
location: symbol.location,
825+
});
837826
}
838-
SymbolScope::GlobalExplicit | SymbolScope::GlobalImplicit => {
839-
// TODO: add more checks for globals?
840-
}
841-
SymbolScope::Local | SymbolScope::Cell => {
842-
// all is well
843-
}
844-
SymbolScope::Unknown => {
845-
// Try hard to figure out what the scope of this symbol is.
846-
let scope = if symbol.is_bound() {
847-
if symbol.flags.contains(SymbolFlags::COMP_CELL)
848-
&& matches!(st_typ, CompilerScope::Module | CompilerScope::Class)
849-
{
850-
// CPython keeps comprehension-only cells in
851-
// module/class scopes as normal local/name
852-
// bindings and uses DEF_COMP_CELL to allocate the
853-
// synthetic cell slot. The spliced comp child
854-
// should not force the outer name itself to CELL.
855-
SymbolScope::Local
856-
} else {
857-
self.found_in_inner_scope(sub_tables, &symbol.name, st_typ)
858-
.unwrap_or(SymbolScope::Local)
859-
}
860-
} else if let Some(scope) = class_entry
861-
.and_then(|class_symbols| class_symbols.get(&symbol.name))
862-
.and_then(|class_sym| {
863-
if class_sym.flags.contains(SymbolFlags::GLOBAL) {
864-
Some(SymbolScope::GlobalExplicit)
865-
} else if class_sym.is_bound() && class_sym.scope != SymbolScope::Free {
866-
// If name is bound in enclosing class, use GlobalImplicit
867-
// so it can be accessed via __classdict__
868-
Some(SymbolScope::GlobalImplicit)
869-
} else {
870-
None
871-
}
872-
})
827+
}
828+
SymbolScope::GlobalExplicit | SymbolScope::GlobalImplicit => {}
829+
SymbolScope::Local | SymbolScope::Cell => {}
830+
SymbolScope::Unknown => {
831+
// Try hard to figure out what the scope of this symbol is.
832+
let scope = if symbol.is_bound() {
833+
if symbol.flags.contains(SymbolFlags::COMP_CELL)
834+
&& matches!(st_typ, CompilerScope::Module | CompilerScope::Class)
873835
{
874-
scope
875-
} else if let Some(scope) = self.found_in_outer_scope(
876-
&symbol.name,
877-
st_typ,
878-
skip_enclosing_function_scope,
879-
) {
880-
// If found in enclosing scope (function/TypeParams), use that
881-
scope
882-
} else if self.tables.is_empty() {
883-
// Don't make assumptions when we don't know.
884-
SymbolScope::Unknown
836+
// CPython keeps comprehension-only cells in
837+
// module/class scopes as normal local/name
838+
// bindings and uses DEF_COMP_CELL to allocate the
839+
// synthetic cell slot. The spliced comp child
840+
// should not force the outer name itself to CELL.
841+
SymbolScope::Local
885842
} else {
886-
// If there are scopes above we assume global.
887-
SymbolScope::GlobalImplicit
888-
};
889-
symbol.scope = scope;
890-
}
843+
self.found_in_inner_scope(sub_tables, &symbol.name, st_typ)
844+
.unwrap_or(SymbolScope::Local)
845+
}
846+
} else if let Some(scope) = class_entry
847+
.and_then(|class_symbols| class_symbols.get(&symbol.name))
848+
.and_then(|class_sym| {
849+
if class_sym.flags.contains(SymbolFlags::GLOBAL) {
850+
Some(SymbolScope::GlobalExplicit)
851+
} else if class_sym.is_bound() && class_sym.scope != SymbolScope::Free {
852+
// If name is bound in enclosing class, use GlobalImplicit
853+
// so it can be accessed via __classdict__
854+
Some(SymbolScope::GlobalImplicit)
855+
} else {
856+
None
857+
}
858+
})
859+
{
860+
scope
861+
} else if let Some(scope) =
862+
self.found_in_outer_scope(&symbol.name, st_typ, skip_enclosing_function_scope)
863+
{
864+
// If found in enclosing scope (function/TypeParams), use that
865+
scope
866+
} else if self.tables.is_empty() {
867+
// Don't make assumptions when we don't know.
868+
SymbolScope::Unknown
869+
} else {
870+
// If there are scopes above we assume global.
871+
SymbolScope::GlobalImplicit
872+
};
873+
symbol.scope = scope;
891874
}
892875
}
893876
Ok(())
@@ -1023,106 +1006,6 @@ impl SymbolTableAnalyzer {
10231006
}
10241007
})
10251008
}
1026-
1027-
// Implements the symbol analysis and scope extension for names
1028-
// assigned by a named expression in a comprehension. See:
1029-
// https://github.com/python/cpython/blob/7b78e7f9fd77bb3280ee39fb74b86772a7d46a70/Python/symtable.c#L1435
1030-
fn analyze_symbol_comprehension(
1031-
&mut self,
1032-
symbol: &mut Symbol,
1033-
parent_offset: usize,
1034-
) -> SymbolTableResult {
1035-
// when this is called, we expect to be in the direct parent scope of the scope that contains 'symbol'
1036-
let last = self.tables.iter_mut().rev().nth(parent_offset).unwrap();
1037-
let symbols = &mut last.0;
1038-
let table_type = last.1;
1039-
1040-
// it is not allowed to use an iterator variable as assignee in a named expression
1041-
if symbol.flags.contains(SymbolFlags::ITER) {
1042-
return Err(SymbolTableError {
1043-
error: format!(
1044-
"assignment expression cannot rebind comprehension iteration variable {}",
1045-
symbol.name
1046-
),
1047-
// TODO: accurate location info, somehow
1048-
location: None,
1049-
});
1050-
}
1051-
1052-
match table_type {
1053-
CompilerScope::Module => {
1054-
symbol.scope = SymbolScope::GlobalImplicit;
1055-
}
1056-
CompilerScope::Class => {
1057-
// named expressions are forbidden in comprehensions on class scope
1058-
return Err(SymbolTableError {
1059-
error: "assignment expression within a comprehension cannot be used in a class body".to_string(),
1060-
// TODO: accurate location info, somehow
1061-
location: None,
1062-
});
1063-
}
1064-
CompilerScope::Function | CompilerScope::AsyncFunction | CompilerScope::Lambda => {
1065-
if let Some(parent_symbol) = symbols.get_mut(&symbol.name) {
1066-
if let SymbolScope::Unknown = parent_symbol.scope {
1067-
// this information is new, as the assignment is done in inner scope
1068-
parent_symbol.flags.insert(SymbolFlags::ASSIGNED);
1069-
}
1070-
1071-
symbol.scope = if parent_symbol.is_global() {
1072-
parent_symbol.scope
1073-
} else {
1074-
SymbolScope::Free
1075-
};
1076-
} else {
1077-
let mut cloned_sym = symbol.clone();
1078-
cloned_sym.scope = SymbolScope::Cell;
1079-
last.0.insert(cloned_sym.name.to_owned(), cloned_sym);
1080-
}
1081-
}
1082-
CompilerScope::Comprehension => {
1083-
// TODO check for conflicts - requires more context information about variables
1084-
match symbols.get_mut(&symbol.name) {
1085-
Some(parent_symbol) => {
1086-
// check if assignee is an iterator in top scope
1087-
if parent_symbol.flags.contains(SymbolFlags::ITER) {
1088-
return Err(SymbolTableError {
1089-
error: format!(
1090-
"assignment expression cannot rebind comprehension iteration variable {}",
1091-
symbol.name
1092-
),
1093-
location: None,
1094-
});
1095-
}
1096-
1097-
// we synthesize the assignment to the symbol from inner scope
1098-
parent_symbol.flags.insert(SymbolFlags::ASSIGNED); // more checks are required
1099-
}
1100-
None => {
1101-
// extend the scope of the inner symbol
1102-
// as we are in a nested comprehension, we expect that the symbol is needed
1103-
// outside, too, and set it therefore to non-local scope. I.e., we expect to
1104-
// find a definition on a higher level
1105-
let mut cloned_sym = symbol.clone();
1106-
cloned_sym.scope = SymbolScope::Free;
1107-
last.0.insert(cloned_sym.name.to_owned(), cloned_sym);
1108-
}
1109-
}
1110-
1111-
self.analyze_symbol_comprehension(symbol, parent_offset + 1)?;
1112-
}
1113-
CompilerScope::TypeParams => {
1114-
// Named expression in comprehension cannot be used in type params
1115-
return Err(SymbolTableError {
1116-
error: "assignment expression within a comprehension cannot be used within the definition of a generic".to_string(),
1117-
location: None,
1118-
});
1119-
}
1120-
CompilerScope::Annotation | CompilerScope::TypeAlias | CompilerScope::TypeVariable => {
1121-
self.analyze_symbol_comprehension(symbol, parent_offset + 1)?;
1122-
}
1123-
}
1124-
Ok(())
1125-
}
11261009
}
11271010

11281011
#[derive(Clone, Copy, Debug)]
@@ -3475,6 +3358,10 @@ impl SymbolTableBuilder {
34753358
table.symbols.entry(name.into_owned()).or_insert(symbol)
34763359
};
34773360

3361+
if matches!(role, SymbolUsage::Global | SymbolUsage::Nonlocal) {
3362+
symbol.location = location;
3363+
}
3364+
34783365
// Set proper scope and flags on symbol:
34793366
let flags = &mut symbol.flags;
34803367
match role {

crates/vm/src/stdlib/_ast.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2438,6 +2438,7 @@ pub(crate) fn compile(
24382438
}
24392439
};
24402440
opts.future_features |= codegen::preprocess::future_features(&ast);
2441+
let source = text.clone();
24412442
let source_file = SourceFileBuilder::new(filename, text).finish();
24422443
#[cfg(feature = "parser")]
24432444
let code = {
@@ -2491,7 +2492,7 @@ pub(crate) fn compile(
24912492
};
24922493
#[cfg(not(feature = "parser"))]
24932494
let code = codegen::compile::compile_top(ast, source_file, mode, opts);
2494-
let code = code.map_err(|err| vm.new_syntax_error(&err.into(), None))?; // FIXME source
2495+
let code = code.map_err(|err| vm.new_syntax_error(&err.into(), Some(source.as_str())))?;
24952496
Ok(crate::builtins::PyCode::new_ref_from_bytecode(vm, code).into())
24962497
}
24972498

0 commit comments

Comments
 (0)