From a8f1356f7fc97739e8d4c2182eb76fd9eeda77f9 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:28:11 +0300 Subject: [PATCH] Use `ruff_python_ast::name::Name` for symtable and compiler --- crates/codegen/src/compile.rs | 242 ++++++++++--------- crates/codegen/src/symboltable.rs | 370 ++++++++++++++++-------------- crates/vm/src/stdlib/_symtable.rs | 4 +- 3 files changed, 326 insertions(+), 290 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index ba2d41f12b3..4889bd1ff98 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -22,7 +22,7 @@ use core::{mem, slice}; use malachite_bigint::BigInt; use num_complex::Complex; use num_traits::{Num, ToPrimitive, Zero}; -use ruff_python_ast as ast; +use ruff_python_ast::{self as ast, name::Name}; use ruff_text_size::{Ranged, TextRange, TextSize}; use rustpython_compiler_core::{ Mode, OneIndexed, PositionEncoding, SourceFile, SourceLocation, @@ -675,7 +675,7 @@ fn compiler_unwrap_option(zelf: &Compiler<'_>, o: Option) -> T { #[derive(Clone)] pub struct PatternContext { /// A list of names captured by the pattern. - pub stores: Vec, + pub stores: Vec, /// If false, then any name captures against our subject will raise. pub allow_irrefutable: bool, /// A list of jump target labels used on pattern failure. @@ -1268,7 +1268,7 @@ impl<'warnings> Compiler<'warnings> { fn starunpack_helper_impl( &mut self, elts: &[ast::Expr], - injected_arg: Option<&str>, + injected_arg: Option<&Name>, pushed: u32, collection_type: CollectionType, ) -> CompileResult<()> { @@ -1713,7 +1713,7 @@ impl<'warnings> Compiler<'warnings> { // 5. "super" must be GlobalImplicit in the current scope. let table = self.current_symbol_table(); - if let Some(symbol) = table.lookup("super") + if let Some(symbol) = table.lookup(&"super".into()) && symbol.scope != SymbolScope::GlobalImplicit { return None; @@ -1721,7 +1721,7 @@ impl<'warnings> Compiler<'warnings> { // Then check the top-level scope and reject any statically // visible symbol for "super", not just local bindings. if let Some(top_table) = self.symbol_table_stack.first() - && top_table.lookup("super").is_some() + && top_table.lookup(&"super".into()).is_some() { return None; } @@ -1753,7 +1753,7 @@ impl<'warnings> Compiler<'warnings> { // Check if __class__ is available as a cell/free variable // The scope must be Free (from enclosing class) or have DEF_FREE_CLASS flag { - let symbol = table.lookup("__class__")?; + let symbol = table.lookup(&"__class__".into())?; if symbol.scope != SymbolScope::Free && !symbol.flags.contains(SymbolFlags::DEF_FREE_CLASS) { @@ -1777,7 +1777,7 @@ impl<'warnings> Compiler<'warnings> { ) -> CompileResult<()> { // 1. Load global super self.set_source_range(super_name_range); - self.compile_name("super", NameUsage::Load)?; + self.compile_name(&"super".into(), NameUsage::Load)?; match super_type { SuperCallType::TwoArg { @@ -1792,7 +1792,9 @@ impl<'warnings> Compiler<'warnings> { // 0-arg: load __class__ cell and first parameter // Load __class__ from cell/free variable self.set_source_range(super_call_range); - let scope = self.get_ref_type("__class__").map_err(|e| self.error(e))?; + let scope = self + .get_ref_type(&"__class__".into()) + .map_err(|e| self.error(e))?; let idx = match scope { SymbolScope::Cell => self.get_cell_var_index("__class__"), SymbolScope::Free => self.get_free_var_index("__class__"), @@ -1817,7 +1819,7 @@ impl<'warnings> Compiler<'warnings> { )) })?; self.set_source_range(super_call_range); - self.compile_name(&first_param, NameUsage::Load)?; + self.compile_name(&first_param.into(), NameUsage::Load)?; } } Ok(()) @@ -1861,7 +1863,7 @@ impl<'warnings> Compiler<'warnings> { }; // Use varnames from symbol table (already collected in definition order) - let varname_cache: IndexSet = ste.varnames.iter().cloned().collect(); + let varname_cache: IndexSet = ste.varnames.iter().cloned().collect(); let nparams = ste.varnames.len(); // Build cellvars using dictbytype (CELL scope or COMP_CELL flag, sorted) @@ -1876,7 +1878,7 @@ impl<'warnings> Compiler<'warnings> { .collect(); cell_names.sort(); for name in cell_names { - cellvar_cache.insert(name); + cellvar_cache.insert(name.into()); } // Handle implicit __class__ cell if needed @@ -1900,7 +1902,7 @@ impl<'warnings> Compiler<'warnings> { // Build freevars using dictbytype (FREE scope, offset by cellvars size) let mut freevar_cache = IndexSet::default(); - let annotation_free_names: IndexSet = ste + let annotation_free_names: IndexSet = ste .annotation_block .as_ref() .map(|annotation| { @@ -1936,7 +1938,7 @@ impl<'warnings> Compiler<'warnings> { .collect(); free_names.sort(); for name in free_names { - freevar_cache.insert(name); + freevar_cache.insert(name.into()); } // Initialize u_metadata fields @@ -2022,7 +2024,7 @@ impl<'warnings> Compiler<'warnings> { qualname: None, // Will be set below consts: Default::default(), names: IndexSet::default(), - varnames: varname_cache, + varnames: varname_cache.into_iter().map(Into::into).collect(), cellvars: cellvar_cache, freevars: freevar_cache, fast_hidden: IndexMap::default(), @@ -2499,10 +2501,10 @@ impl<'warnings> Compiler<'warnings> { self.emit_load_const(ConstantData::None); self.mark_unwind_no_location(*loc); self.set_unwind_source_range(*loc); - self.store_name(name)?; + self.store_name(&name.into())?; self.mark_unwind_no_location(*loc); self.set_unwind_source_range(*loc); - self.compile_name(name, NameUsage::Delete)?; + self.compile_name(&name.into(), NameUsage::Delete)?; self.mark_unwind_no_location(*loc); } } @@ -2594,11 +2596,12 @@ impl<'warnings> Compiler<'warnings> { name: &str, cache: impl FnOnce(&mut ir::CodeInfo) -> &mut IndexSet, ) -> u32 { - let name = self.mangle(name); + let target = name.into(); + let name = self.mangle(&target); let cache = cache(self.current_code_info()); cache - .get_index_of(name.as_ref()) - .unwrap_or_else(|| cache.insert_full(name.into_owned()).0) + .get_index_of::(&name.as_str().into()) + .unwrap_or_else(|| cache.insert_full(name.to_string()).0) .to_u32() } @@ -2672,14 +2675,15 @@ impl<'warnings> Compiler<'warnings> { // We might be in a situation where symbol table isn't pushed yet // In this case, check the parent symbol table if let Some(parent_table) = self.symbol_table_stack.last() - && let Some(symbol) = parent_table.lookup(¤t_obj_name) + && let Some(symbol) = parent_table.lookup(¤t_obj_name.clone().into()) && symbol.scope == SymbolScope::GlobalExplicit { force_global = true; } } else if let Some(_current_table) = self.symbol_table_stack.last() { // Mangle the name if necessary (for private names in classes) - let mangled_name = self.mangle(¤t_obj_name); + let target = ¤t_obj_name.clone().into(); + let mangled_name = self.mangle(target); // Look up in parent symbol table to check scope if self.symbol_table_stack.len() >= 2 { @@ -2774,7 +2778,7 @@ impl<'warnings> Compiler<'warnings> { if Self::scope_needs_conditional_annotations_cell(self.current_symbol_table()) { self.set_source_range(module_start_loc); emit!(self, Instruction::BuildSet { count: 0 }); - self.store_name("__conditional_annotations__")?; + self.store_name(&"__conditional_annotations__".into())?; } if self.future_annotations && annotations_used { @@ -2843,7 +2847,7 @@ impl<'warnings> Compiler<'warnings> { if self.current_symbol_table().has_conditional_annotations { self.set_source_range(module_start_loc); emit!(self, Instruction::BuildSet { count: 0 }); - self.store_name("__conditional_annotations__")?; + self.store_name(&"__conditional_annotations__".into())?; } if self.future_annotations && annotations_used { @@ -2895,7 +2899,7 @@ impl<'warnings> Compiler<'warnings> { }) => { validate_duplicate_params(parameters).map_err(|e| self.error(e))?; self.compile_function_def( - name.as_str(), + name.id(), parameters, body, decorator_list, @@ -2914,7 +2918,7 @@ impl<'warnings> Compiler<'warnings> { .. }) => { self.compile_class_def( - name.as_str(), + name.id(), body, decorator_list, type_params.as_deref(), @@ -3038,15 +3042,15 @@ impl<'warnings> Compiler<'warnings> { } } - fn load_name(&mut self, name: &str) -> CompileResult<()> { + fn load_name(&mut self, name: &Name) -> CompileResult<()> { self.compile_name(name, NameUsage::Load) } - fn store_name(&mut self, name: &str) -> CompileResult<()> { + fn store_name(&mut self, name: &Name) -> CompileResult<()> { self.compile_name(name, NameUsage::Store) } - fn emit_no_location_exception_name_cleanup(&mut self, name: &str) -> CompileResult<()> { + fn emit_no_location_exception_name_cleanup(&mut self, name: &Name) -> CompileResult<()> { // CPython codegen_try_except() emits `name = None; del name` // with NO_LOCATION for `except ... as name` cleanup. self.emit_load_const(ConstantData::None); @@ -3058,17 +3062,17 @@ impl<'warnings> Compiler<'warnings> { Ok(()) } - fn mangle<'a>(&self, name: &'a str) -> Cow<'a, str> { + fn mangle<'a>(&self, name: &'a Name) -> Cow<'a, Name> { // Use private from current code unit for name mangling let private = self .code_stack .last() - .and_then(|info| info.private.as_deref()); + .and_then(|info| info.private.as_ref()); let mangled_names = self.current_symbol_table().mangled_names.as_ref(); - symboltable::maybe_mangle_name(private, mangled_names, name) + symboltable::maybe_mangle_name(private.map(Name::from).as_ref(), mangled_names, name) } - fn module_name_declared_global_in_nested_scope(table: &SymbolTable, name: &str) -> bool { + fn module_name_declared_global_in_nested_scope(table: &SymbolTable, name: &Name) -> bool { table.sub_tables.iter().any(|subtable| { (!subtable.comp_inlined && subtable @@ -3079,7 +3083,7 @@ impl<'warnings> Compiler<'warnings> { } // = compiler_nameop - fn compile_name(&mut self, name: &str, usage: NameUsage) -> CompileResult<()> { + fn compile_name(&mut self, name: &Name, usage: NameUsage) -> CompileResult<()> { enum NameOp { Fast, Global, @@ -3091,7 +3095,7 @@ impl<'warnings> Compiler<'warnings> { let name = self.mangle(name); // Special handling for __debug__ - if NameUsage::Load == usage && name == "__debug__" { + if NameUsage::Load == usage && name.as_str() == "__debug__" { self.emit_load_const(ConstantData::Boolean { value: self.opts.optimize == 0, }); @@ -3113,14 +3117,14 @@ impl<'warnings> Compiler<'warnings> { let can_see_class = current_table.can_see_class_scope; // First try to find in current table - let symbol = current_table.lookup(name.as_ref()); + let symbol = current_table.lookup(&name); // If not found and we're in ast::TypeParams or Annotation scope, try parent scope let symbol = if symbol.is_none() && (is_typeparams || is_annotation) { self.symbol_table_stack .get(self.symbol_table_stack.len() - 2) // Try to get parent index .expect("Symbol has no parent! This is a compiler bug.") - .lookup(name.as_ref()) + .lookup(&name) } else { symbol }; @@ -3129,7 +3133,7 @@ impl<'warnings> Compiler<'warnings> { .iter() .rev() .find(|table| table.typ == CompilerScope::Class) - .and_then(|table| table.lookup(name.as_ref())) + .and_then(|table| table.lookup(&name)) .is_some_and(|symbol| symbol.flags.contains(SymbolFlags::DEF_GLOBAL)); ( @@ -3149,10 +3153,12 @@ impl<'warnings> Compiler<'warnings> { if current_table.typ == CompilerScope::Class && !self.current_code_info().in_inlined_comp && ((usage == NameUsage::Load - && (name == "__class__" - || name == "__classdict__" - || name == "__conditional_annotations__")) - || (name == "__conditional_annotations__" && usage == NameUsage::Store)) + && matches!( + name.as_str(), + "__class__" | "__classdict__" | "__conditional_annotations__" + )) + || (name.as_str() == "__conditional_annotations__" + && usage == NameUsage::Store)) { Some(SymbolScope::Cell) } else { @@ -3176,7 +3182,7 @@ impl<'warnings> Compiler<'warnings> { ) { SymbolScope::GlobalImplicit } else if matches!( - name.as_ref(), + name.as_str(), "__name__" | "__module__" | "__qualname__" @@ -3213,7 +3219,7 @@ impl<'warnings> Compiler<'warnings> { .current_code_info() .metadata .fast_hidden - .get(name.as_ref()) + .get(name.as_str()) .is_some_and(|&hidden| hidden) { NameOp::Fast @@ -3380,12 +3386,12 @@ impl<'warnings> Compiler<'warnings> { emit!(self, Instruction::PopTop); } } - self.store_name(alias.as_str())?; + self.store_name(alias.id())?; if !parts.is_empty() { emit!(self, Instruction::PopTop); } } else { - self.store_name(name.name.split('.').next().unwrap())? + self.store_name(&name.name.split('.').next().unwrap().into())? } } } @@ -3437,9 +3443,9 @@ impl<'warnings> Compiler<'warnings> { // Store module under proper name: if let Some(alias) = &name.asname { - self.store_name(alias.as_str())? + self.store_name(alias.id())? } else { - self.store_name(name.name.as_str())? + self.store_name(name.name.id())? } } @@ -3557,7 +3563,7 @@ impl<'warnings> Compiler<'warnings> { validate_duplicate_params(parameters).map_err(|e| self.error(e))?; self.compile_function_def( - name.as_str(), + name.id(), parameters, body, decorator_list, @@ -3575,7 +3581,7 @@ impl<'warnings> Compiler<'warnings> { arguments, .. }) => self.compile_class_def( - name.as_str(), + name.id(), body, decorator_list, type_params.as_deref(), @@ -3772,7 +3778,7 @@ impl<'warnings> Compiler<'warnings> { "type alias expect name".to_owned(), ))); }; - let name_string = name.id.to_string(); + let name_string = name.id(); if let Some(type_params) = type_params { self.set_source_range(*range); @@ -3792,10 +3798,10 @@ impl<'warnings> Compiler<'warnings> { self.set_source_range(*range); self.emit_load_const(ConstantData::Str { - value: name_string.clone().into(), + value: name_string.as_str().into(), }); self.compile_type_params(type_params)?; - self.compile_typealias_value_closure(&name_string, value, *range)?; + self.compile_typealias_value_closure(name_string, value, *range)?; self.set_source_range(*range); emit!(self, Instruction::BuildTuple { count: 3 }); emit!( @@ -3816,10 +3822,10 @@ impl<'warnings> Compiler<'warnings> { } else { self.set_source_range(*range); self.emit_load_const(ConstantData::Str { - value: name_string.clone().into(), + value: name_string.as_str().into(), }); self.emit_load_const(ConstantData::None); - self.compile_typealias_value_closure(&name_string, value, *range)?; + self.compile_typealias_value_closure(name_string, value, *range)?; self.set_source_range(*range); emit!(self, Instruction::BuildTuple { count: 3 }); emit!( @@ -3831,7 +3837,7 @@ impl<'warnings> Compiler<'warnings> { } self.set_source_range(*range); - self.store_name(&name_string)?; + self.store_name(name_string)?; } ast::Stmt::IpyEscapeCommand(stmt) => { return Err(self.error_ranged( @@ -3849,7 +3855,7 @@ impl<'warnings> Compiler<'warnings> { let result = (|| -> CompileResult<()> { match &expression { ast::Expr::Name(ast::ExprName { id, .. }) => { - self.compile_name(id.as_str(), NameUsage::Delete)? + self.compile_name(id, NameUsage::Delete)? } ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => { self.compile_expression(value)?; @@ -4104,7 +4110,7 @@ impl<'warnings> Compiler<'warnings> { self.set_source_range(*range); emit!(self, Instruction::Copy { i: 1 }); - self.store_name(name.as_ref())?; + self.store_name(name.id())?; } ast::TypeParam::ParamSpec(ast::TypeParamParamSpec { name, @@ -4148,7 +4154,7 @@ impl<'warnings> Compiler<'warnings> { self.set_source_range(*range); emit!(self, Instruction::Copy { i: 1 }); - self.store_name(name.as_ref())?; + self.store_name(name.id())?; } ast::TypeParam::TypeVarTuple(ast::TypeParamTypeVarTuple { name, @@ -4193,7 +4199,7 @@ impl<'warnings> Compiler<'warnings> { self.set_source_range(*range); emit!(self, Instruction::Copy { i: 1 }); - self.store_name(name.as_ref())?; + self.store_name(name.id())?; } }; } @@ -4389,7 +4395,7 @@ impl<'warnings> Compiler<'warnings> { let cleanup_end = self.new_block(); let cleanup_body = self.new_block(); - self.store_name(alias.as_str())?; + self.store_name(alias.id())?; emit!(self, PseudoInstruction::SetupCleanup { delta: cleanup_end }); self.use_cpython_label_block(cleanup_body); @@ -4411,7 +4417,7 @@ impl<'warnings> Compiler<'warnings> { emit!(self, Instruction::PopExcept); self.set_no_location(); - self.emit_no_location_exception_name_cleanup(alias.as_str())?; + self.emit_no_location_exception_name_cleanup(alias.id())?; emit!( self, @@ -4420,7 +4426,7 @@ impl<'warnings> Compiler<'warnings> { self.set_no_location(); self.use_cpython_label_block(cleanup_end); - self.emit_no_location_exception_name_cleanup(alias.as_str())?; + self.emit_no_location_exception_name_cleanup(alias.id())?; emit!(self, Instruction::Reraise { depth: 1 }); self.set_no_location(); } else { @@ -4680,7 +4686,7 @@ impl<'warnings> Compiler<'warnings> { // Store match to name or pop if let Some(alias) = name { - self.store_name(alias.as_str())?; + self.store_name(alias.id())?; } else { emit!(self, Instruction::PopTop); // pop match } @@ -4716,7 +4722,7 @@ impl<'warnings> Compiler<'warnings> { // Cleanup name binding if let Some(alias) = name { - self.emit_no_location_exception_name_cleanup(alias.as_str())?; + self.emit_no_location_exception_name_cleanup(alias.id())?; } emit!( @@ -4734,7 +4740,7 @@ impl<'warnings> Compiler<'warnings> { // Cleanup name binding if let Some(alias) = name { - self.emit_no_location_exception_name_cleanup(alias.as_str())?; + self.emit_no_location_exception_name_cleanup(alias.id())?; } // LIST_APPEND(3) - append raised_exc to list @@ -4925,7 +4931,7 @@ impl<'warnings> Compiler<'warnings> { for (arg, default) in &kw_with_defaults { self.set_source_range(loc); self.emit_load_const(ConstantData::Str { - value: self.mangle(arg.name.as_str()).into_owned().into(), + value: self.mangle(arg.name().id()).as_str().into(), }); self.compile_expression(default)?; } @@ -5092,7 +5098,7 @@ impl<'warnings> Compiler<'warnings> { if let Some(annotation) = ¶m.annotation { self.set_source_range(func_range); self.emit_load_const(ConstantData::Str { - value: self.mangle(param.name.as_str()).into_owned().into(), + value: self.mangle(param.name.id()).as_str().into(), }); self.compile_annotation(annotation)?; } @@ -5279,7 +5285,7 @@ impl<'warnings> Compiler<'warnings> { } = stmt; let simple_name = if *simple { match target.as_ref() { - ast::Expr::Name(ast::ExprName { id, .. }) => Some(id.as_str()), + ast::Expr::Name(ast::ExprName { id, .. }) => Some(id), _ => None, } } else { @@ -5329,7 +5335,7 @@ impl<'warnings> Compiler<'warnings> { self.set_source_range(*range); emit!(self, Instruction::Copy { i: 2 }); self.emit_load_const(ConstantData::Str { - value: self.mangle(name).into_owned().into(), + value: self.mangle(name).as_str().into(), }); self.set_source_range(loc); emit!(self, Instruction::StoreSubscr); @@ -5374,9 +5380,10 @@ impl<'warnings> Compiler<'warnings> { "__annotate_func__" } else { "__annotate__" - }; + } + .into(); self.set_source_range(loc); - self.store_name(name)?; + self.store_name(&name)?; Ok(true) } @@ -5385,7 +5392,7 @@ impl<'warnings> Compiler<'warnings> { #[expect(clippy::too_many_arguments, reason = "ignore warning for now")] fn compile_function_def( &mut self, - name: &str, + name: &Name, parameters: &ast::Parameters, body: &[ast::Stmt], decorator_list: &[ast::Decorator], @@ -5578,7 +5585,7 @@ impl<'warnings> Compiler<'warnings> { /// Determines if a variable should be CELL or FREE type // = get_ref_type - fn get_ref_type(&self, name: &str) -> Result { + fn get_ref_type(&self, name: &Name) -> Result { let table = self.symbol_table_stack.last().unwrap(); // Special handling for __class__, __classdict__, and __conditional_annotations__ in class scope @@ -5626,7 +5633,9 @@ impl<'warnings> Compiler<'warnings> { // well as by the normal name lookup logic. // Get reference type using our get_ref_type function - let ref_type = self.get_ref_type(var).map_err(|e| self.error(e))?; + let ref_type = self + .get_ref_type(&var.as_str().into()) + .map_err(|e| self.error(e))?; // Get parent code info let parent_code = self.code_stack.last().unwrap(); @@ -5789,26 +5798,26 @@ impl<'warnings> Compiler<'warnings> { self.set_source_range(class_body_prefix_range); // Load __name__ and store as __module__ - self.load_name("__name__")?; - self.store_name("__module__")?; + self.load_name(&"__name__".into())?; + self.store_name(&"__module__".into())?; // Store __qualname__ self.emit_load_const(ConstantData::Str { value: qualname.into(), }); - self.store_name("__qualname__")?; + self.store_name(&"__qualname__".into())?; // Store __firstlineno__ before __doc__ self.emit_load_const(ConstantData::Integer { value: BigInt::from(firstlineno), }); - self.store_name("__firstlineno__")?; + self.store_name(&"__firstlineno__".into())?; // Set __type_params__ from the enclosing type-params closure when // compiling a generic class body. if type_params.is_some() { - self.load_name(".type_params")?; - self.store_name("__type_params__")?; + self.load_name(&".type_params".into())?; + self.store_name(&"__type_params__".into())?; } // PEP 649: Initialize __classdict__ after synthetic generic-class @@ -5823,7 +5832,7 @@ impl<'warnings> Compiler<'warnings> { let annotations_used = self.current_symbol_table().annotations_used; if Self::scope_needs_conditional_annotations_cell(self.current_symbol_table()) { emit!(self, Instruction::BuildSet { count: 0 }); - self.store_name("__conditional_annotations__")?; + self.store_name(&"__conditional_annotations__".into())?; } if self.future_annotations && annotations_used { @@ -5835,7 +5844,7 @@ impl<'warnings> Compiler<'warnings> { let saved_range = self.current_source_range; self.set_source_range(range); self.emit_load_const(ConstantData::Str { value: doc.into() }); - self.store_name("__doc__")?; + self.store_name(&"__doc__".into())?; self.set_no_location(); self.set_source_range(saved_range); } @@ -5875,7 +5884,7 @@ impl<'warnings> Compiler<'warnings> { .collect(), }); self.set_no_location(); - self.store_name("__static_attributes__")?; + self.store_name(&"__static_attributes__".into())?; self.set_no_location(); } @@ -5884,7 +5893,7 @@ impl<'warnings> Compiler<'warnings> { let classdict_idx = u32::from(self.get_cell_var_index("__classdict__")); emit!(self, PseudoInstruction::LoadClosure { i: classdict_idx }); self.set_no_location(); - self.store_name("__classdictcell__")?; + self.store_name(&"__classdictcell__".into())?; self.set_no_location(); } @@ -5898,7 +5907,7 @@ impl<'warnings> Compiler<'warnings> { self.set_no_location(); emit!(self, Instruction::Copy { i: 1 }); self.set_no_location(); - self.store_name("__classcell__")?; + self.store_name(&"__classcell__".into())?; self.set_no_location(); } else { self.emit_load_const(ConstantData::None); @@ -5916,7 +5925,7 @@ impl<'warnings> Compiler<'warnings> { fn compile_class_def( &mut self, - name: &str, + name: &Name, body: &[ast::Stmt], decorator_list: &[ast::Decorator], type_params: Option<&ast::TypeParams>, @@ -5962,7 +5971,7 @@ impl<'warnings> Compiler<'warnings> { )?; // Set private name for name mangling - self.code_stack.last_mut().unwrap().private = Some(name.to_owned()); + self.code_stack.last_mut().unwrap().private = Some(name.as_str().to_owned()); // TypeParams scope is function-like self.ctx = CompileContext { @@ -5975,7 +5984,7 @@ impl<'warnings> Compiler<'warnings> { // generic class bodies close over. self.compile_type_params(type_params.unwrap())?; self.set_source_range(class_source_range); - self.store_name(".type_params")?; + self.store_name(&".type_params".into())?; } // Step 2: Compile class body (always done, whether generic or not) @@ -6001,12 +6010,14 @@ impl<'warnings> Compiler<'warnings> { // Create the class body function with the .type_params closure // captured through the class code object's freevars. self.make_closure(class_code, bytecode::MakeFunctionFlags::new())?; - self.emit_load_const(ConstantData::Str { value: name.into() }); + self.emit_load_const(ConstantData::Str { + value: name.as_str().into(), + }); // Create .generic_base after the class function and name are on the // stack so the remaining call shape matches CPython's ordering. self.set_source_range(class_source_range); - self.load_name(".type_params")?; + self.load_name(&".type_params".into())?; emit!( self, Instruction::CallIntrinsic1 { @@ -6014,7 +6025,7 @@ impl<'warnings> Compiler<'warnings> { } ); self.set_source_range(class_source_range); - self.store_name(".generic_base")?; + self.store_name(&".generic_base".into())?; let (bases, keywords) = arguments.map_or((&[][..], &[][..]), |args| { (&args.args[..], &args.keywords[..]) @@ -6025,7 +6036,7 @@ impl<'warnings> Compiler<'warnings> { keywords, class_source_range, None, - Some(".generic_base"), + Some(&".generic_base".into()), )?; // Return the created class @@ -6050,7 +6061,9 @@ impl<'warnings> Compiler<'warnings> { // Create class function with closure self.make_closure(class_code, bytecode::MakeFunctionFlags::new())?; - self.emit_load_const(ConstantData::Str { value: name.into() }); + self.emit_load_const(ConstantData::Str { + value: name.as_str().into(), + }); if let Some(arguments) = arguments { self.codegen_call_helper(2, arguments, class_source_range, None)?; @@ -6617,7 +6630,7 @@ impl<'warnings> Compiler<'warnings> { Some(name) => { // Ensure we don't store the same name twice. // TODO: maybe pc.stores should be a set? - if pc.stores.contains(&name.to_string()) { + if pc.stores.contains(name.id()) { return Err(self.error_ranged( CodegenErrorType::DuplicateStore(name.as_str().to_string()), loc, @@ -6629,7 +6642,7 @@ impl<'warnings> Compiler<'warnings> { self.pattern_helper_rotate(loc, rotations); // Append the name to the captured stores. - pc.stores.push(name.to_string()); + pc.stores.push(name.id().clone()); Ok(()) } } @@ -7391,7 +7404,7 @@ impl<'warnings> Compiler<'warnings> { let old_pc = pc.clone(); // Simulate Py_INCREF on pc.stores by cloning it. pc.stores = pc.stores.clone(); - let mut control: Option> = None; // Will hold the capture list of the first alternative. + let mut control: Option> = None; // Will hold the capture list of the first alternative. // Process each alternative. for (i, alt) in p.patterns.iter().enumerate() { @@ -8093,7 +8106,7 @@ impl<'warnings> Compiler<'warnings> { // Load the variable name self.set_source_range(loc); self.emit_load_const(ConstantData::Str { - value: self.mangle(id.as_str()).into_owned().into(), + value: self.mangle(id).as_str().into(), }); // Store: __annotations__[name] = annotation self.set_source_range(loc); @@ -8150,7 +8163,7 @@ impl<'warnings> Compiler<'warnings> { self.set_source_range(target.range()); let result = (|| -> CompileResult<()> { match &target { - ast::Expr::Name(ast::ExprName { id, .. }) => self.store_name(id.as_str())?, + ast::Expr::Name(ast::ExprName { id, .. }) => self.store_name(id)?, ast::Expr::Subscript(ast::ExprSubscript { value, slice, ctx, .. }) => { @@ -8240,7 +8253,7 @@ impl<'warnings> Compiler<'warnings> { let target_range = target.range(); enum AugAssignKind<'a> { Name { - id: &'a str, + id: &'a Name, }, Subscript { use_slice_opt: bool, @@ -8253,7 +8266,6 @@ impl<'warnings> Compiler<'warnings> { let kind = match &target { ast::Expr::Name(ast::ExprName { id, .. }) => { - let id = id.as_str(); self.set_source_range(target_range); self.compile_name(id, NameUsage::Load)?; AugAssignKind::Name { id } @@ -8878,7 +8890,7 @@ impl<'warnings> Compiler<'warnings> { self.emit_load_const(ConstantData::None); let _ = self.compile_yield_from_sequence(false); } - ast::Expr::Name(ast::ExprName { id, .. }) => self.load_name(id.as_str())?, + ast::Expr::Name(ast::ExprName { id, .. }) => self.load_name(id)?, ast::Expr::Lambda(ast::ExprLambda { parameters, body, @@ -8923,7 +8935,7 @@ impl<'warnings> Compiler<'warnings> { for (arg, default) in &kw_with_defaults { self.set_source_range(*range); self.emit_load_const(ConstantData::Str { - value: self.mangle(arg.name.as_str()).into_owned().into(), + value: self.mangle(arg.name().id()).as_str().into(), }); self.compile_expression(default)?; } @@ -9140,10 +9152,12 @@ impl<'warnings> Compiler<'warnings> { if self.current_code_info().in_inlined_comp && let ast::Expr::Name(ast::ExprName { id, .. }) = target.as_ref() { - let name = self.mangle(id.as_str()); + let name = self.mangle(id); let info = self.code_stack.last_mut().unwrap(); info.metadata.fast_hidden.insert(name.to_string(), false); - info.metadata.fast_hidden_final.swap_remove(name.as_ref()); + info.metadata + .fast_hidden_final + .swap_remove(name.into_owned().as_str()); } self.compile_expression(value)?; self.set_source_range(*range); @@ -9656,7 +9670,7 @@ impl<'warnings> Compiler<'warnings> { keywords: &[ast::Keyword], call_range: TextRange, kw_names_range: Option, - injected_arg: Option<&str>, + injected_arg: Option<&Name>, ) -> CompileResult<()> { self.validate_keywords(keywords)?; @@ -10851,7 +10865,7 @@ impl<'warnings> Compiler<'warnings> { }; self.current_code_info().in_inlined_comp = true; - let mut temp_symbols: IndexMap = IndexMap::default(); + let mut temp_symbols: IndexMap = IndexMap::default(); let mut changed_fast_hidden = Vec::new(); let result = (|| { @@ -10871,8 +10885,8 @@ impl<'warnings> Compiler<'warnings> { current_table.sub_tables.insert(insert_pos + i, st.clone()); } } - let mut pushed_locals: Vec = Vec::new(); - let mut fast_hidden_locals: Vec = Vec::new(); + let mut pushed_locals: Vec = Vec::new(); + let mut fast_hidden_locals: Vec = Vec::new(); for (name, sym) in &comp_table.symbols { if sym.flags.contains(SymbolFlags::DEF_PARAM) { continue; // skip .0 @@ -10926,11 +10940,11 @@ impl<'warnings> Compiler<'warnings> { self.current_code_info() .metadata .fast_hidden - .insert(name.clone(), true); + .insert(name.clone().into(), true); self.current_code_info() .metadata .fast_hidden_final - .insert(name.clone()); + .insert(name.clone().into()); changed_fast_hidden.push(name.clone()); } } @@ -11203,7 +11217,7 @@ impl<'warnings> Compiler<'warnings> { self.current_code_info() .metadata .fast_hidden - .insert(name, false); + .insert(name.into(), false); } self.current_code_info().in_inlined_comp = was_in_inlined_comp; @@ -14166,7 +14180,7 @@ mod tests { .unwrap(); assert!( - table.lookup("frozenset").is_none(), + table.lookup(&"frozenset".into()).is_none(), "CPython symtable Constant_kind does not visit the lowered frozenset() expression" ); } @@ -33141,7 +33155,7 @@ deoptmap = { for name in ["base", "family", "specialized"] { let symbol = symbol_table - .lookup(name) + .lookup(&name.into()) .unwrap_or_else(|| panic!("missing module symbol {name}")); assert_eq!( symbol.scope, @@ -33157,7 +33171,7 @@ deoptmap = { assert!(comp.comp_inlined, "expected comprehension to be inlined"); for name in ["base", "family", "specialized"] { let symbol = comp - .lookup(name) + .lookup(&name.into()) .unwrap_or_else(|| panic!("missing comprehension symbol {name}")); assert_eq!( symbol.scope, diff --git a/crates/codegen/src/symboltable.rs b/crates/codegen/src/symboltable.rs index bafd065ef93..63b92ceef55 100644 --- a/crates/codegen/src/symboltable.rs +++ b/crates/codegen/src/symboltable.rs @@ -13,7 +13,7 @@ use crate::{ }; use alloc::{borrow::Cow, fmt}; use bitflags::bitflags; -use ruff_python_ast as ast; +use ruff_python_ast::{self as ast, name::Name}; use ruff_text_size::{Ranged, TextRange}; use rustpython_compiler_core::{PositionEncoding, SourceFile, SourceLocation}; @@ -24,7 +24,7 @@ const RECURSION_ERROR: &str = "maximum recursion depth exceeded during compilati #[derive(Clone)] pub struct SymbolTable { /// The name of this symbol table. Often the name of the class or function. - pub name: String, + pub name: Name, /// The type of symbol table pub typ: CompilerScope, @@ -39,7 +39,7 @@ pub struct SymbolTable { pub is_method: bool, /// A set of symbols present on this scope level. - pub symbols: IndexMap, + pub symbols: IndexMap, /// A list of sub-scopes in the order as found in the /// AST nodes. @@ -63,7 +63,7 @@ pub struct SymbolTable { pub next_sub_table: usize, /// Variable names in definition order (parameters first, then locals) - pub varnames: Vec, + pub varnames: Vec, /// Whether this class scope needs an implicit __class__ cell pub needs_class_closure: bool, @@ -116,11 +116,11 @@ pub struct SymbolTable { /// Names of type parameters that should still be mangled in type param scopes. /// When Some, only names in this set are mangled; other names are left unmangled. /// Set on type param blocks for generic classes; inherited by non-class child scopes. - pub mangled_names: Option>, + pub mangled_names: Option>, } impl SymbolTable { - fn new(name: String, typ: CompilerScope, line_number: u32, is_nested: bool) -> Self { + fn new(name: Name, typ: CompilerScope, line_number: u32, is_nested: bool) -> Self { Self { name, typ, @@ -154,16 +154,16 @@ impl SymbolTable { } fn add_format_parameter(&mut self) { - let name = ".format"; + let name = Name::new_static(".format"); let symbol = self .symbols - .entry(name.to_owned()) - .or_insert_with(|| Symbol::new(name)); + .entry(name.clone()) + .or_insert_with(|| Symbol::new(name.clone())); symbol .flags .insert(SymbolFlags::DEF_PARAM | SymbolFlags::USE); - if !self.varnames.iter().any(|varname| varname == name) { - self.varnames.push(name.to_owned()); + if !self.varnames.contains(&name) { + self.varnames.push(name); } } @@ -213,7 +213,7 @@ impl SymbolTable { } #[must_use] - pub fn lookup(&self, name: &str) -> Option<&Symbol> { + pub fn lookup(&self, name: &Name) -> Option<&Symbol> { self.symbols.get(name) } } @@ -332,16 +332,16 @@ bitflags! { /// of the symbol, and also the various uses of the symbol. #[derive(Debug, Clone)] pub struct Symbol { - pub name: String, + pub name: Name, pub scope: SymbolScope, pub flags: SymbolFlags, pub location: Option, } impl Symbol { - fn new(name: &str) -> Self { + fn new(name: Name) -> Self { Self { - name: name.to_owned(), + name, // table, scope: SymbolScope::Unknown, flags: SymbolFlags::empty(), @@ -422,7 +422,7 @@ fn analyze_symbol_table(symbol_table: &mut SymbolTable) -> SymbolTableResult { `newfree` set (which contains free variables collected from all child scopes) and sets the corresponding flags on the class's symbol table entry. */ -fn drop_class_free(symbol_table: &mut SymbolTable, newfree: &mut IndexSet) { +fn drop_class_free(symbol_table: &mut SymbolTable, newfree: &mut IndexSet) { // Check if __class__ is in the free variables collected from children // If found, it means a child scope (method) references __class__ if newfree.shift_remove("__class__") { @@ -446,10 +446,10 @@ fn drop_class_free(symbol_table: &mut SymbolTable, newfree: &mut IndexSet, - inlined_cells: &mut IndexSet, + comp_free: &mut IndexSet, + inlined_cells: &mut IndexSet, parent_type: CompilerScope, -) -> IndexSet { +) -> IndexSet { let mut removed_class_implicits = IndexSet::default(); for (name, sub_symbol) in &comp.symbols { // Skip the .0 parameter @@ -513,7 +513,7 @@ fn inline_comprehension( removed_class_implicits } -type SymbolMap = IndexMap; +type SymbolMap = IndexMap; mod stack { use alloc::vec::Vec; @@ -596,7 +596,7 @@ impl SymbolTableAnalyzer { &mut self, symbol_table: &mut SymbolTable, class_entry: Option<&SymbolMap>, - ) -> SymbolTableResult> { + ) -> SymbolTableResult> { let symbols = core::mem::take(&mut symbol_table.symbols); let sub_tables = &mut *symbol_table.sub_tables; @@ -624,8 +624,8 @@ impl SymbolTableAnalyzer { // Collect (child_free, is_inlined) pairs from child scopes. // We need to process inlined comprehensions after the closure // when we have access to symbol_table.symbols. - let mut child_frees: Vec<(IndexSet, bool)> = Vec::new(); - let mut annotation_free: Option> = None; + let mut child_frees: Vec<(IndexSet, bool)> = Vec::new(); + let mut annotation_free: Option> = None; let mut info = ( symbols, @@ -664,7 +664,7 @@ impl SymbolTableAnalyzer { // PEP 709: Process inlined comprehensions. // Merge symbols from inlined comps into parent scope without bail-out. - let mut inlined_cells: IndexSet = IndexSet::default(); + let mut inlined_cells: IndexSet = IndexSet::default(); let mut newfree = IndexSet::default(); for (idx, (mut child_free, is_inlined)) in child_frees.into_iter().enumerate() { if is_inlined { @@ -890,7 +890,7 @@ impl SymbolTableAnalyzer { fn found_in_outer_scope( &mut self, - name: &str, + name: &Name, st_typ: CompilerScope, skip_enclosing_function_scope: bool, ) -> Option { @@ -961,7 +961,7 @@ impl SymbolTableAnalyzer { if let Some(free_class) = table.get_mut(name) { free_class.flags.insert(SymbolFlags::DEF_FREE_CLASS) } else { - let mut symbol = Symbol::new(name); + let mut symbol = Symbol::new(name.clone()); symbol.flags.insert(SymbolFlags::DEF_FREE_CLASS); symbol.scope = SymbolScope::Free; table.insert(name.to_owned(), symbol); @@ -977,7 +977,7 @@ impl SymbolTableAnalyzer { // Skip: don't add __classdict__/__conditional_annotations__ // as free vars in regular functions — only annotation/type scopes need them } else if !table.contains_key(name) { - let mut symbol = Symbol::new(name); + let mut symbol = Symbol::new(name.clone()); symbol.scope = SymbolScope::Free; table.insert(name.to_owned(), symbol); } @@ -990,7 +990,7 @@ impl SymbolTableAnalyzer { fn found_in_inner_scope( &self, sub_tables: &[SymbolTable], - name: &str, + name: &Name, st_typ: CompilerScope, ) -> Option { sub_tables.iter().find_map(|st| { @@ -1036,16 +1036,16 @@ enum SymbolUsage { } struct SymbolTableBuilder { - class_name: Option, + class_name: Option, // Scope stack. tables: Vec, future_annotations: bool, allow_top_level_await: bool, source_file: SourceFile, // Current scope's varnames being collected (temporary storage) - current_varnames: Vec, + current_varnames: Vec, // Stack to preserve parent varnames when entering nested scopes - varnames_stack: Vec>, + varnames_stack: Vec>, // Track if we're inside an iterable definition expression (for nested comprehensions) in_iter_def_exp: bool, // Track if we're scanning an inner loop iteration target (not the first generator) @@ -1090,7 +1090,7 @@ impl SymbolTableBuilder { recursion_depth: 0, recursion_limit: DEFAULT_RECURSION_LIMIT, }; - this.enter_scope("top", CompilerScope::Module, 0); + this.enter_scope(&"top".into(), CompilerScope::Module, 0); this } @@ -1149,7 +1149,7 @@ impl SymbolTableBuilder { Ok(symbol_table) } - fn enter_scope(&mut self, name: &str, typ: CompilerScope, line_number: u32) { + fn enter_scope(&mut self, name: &Name, typ: CompilerScope, line_number: u32) { let parent = self.tables.last(); let is_nested = parent.is_some_and(|table| table.is_nested || Self::is_function_like_scope(table.typ)); @@ -1181,7 +1181,7 @@ impl SymbolTableBuilder { fn enter_type_param_block( &mut self, - name: &str, + name: &Name, range: TextRange, for_class: bool, has_defaults: bool, @@ -1211,22 +1211,22 @@ impl SymbolTableBuilder { // Add __classdict__ as a USE symbol in type param scope if in class if in_class { - self.register_name("__classdict__", SymbolUsage::Used, range)?; + self.register_name(&"__classdict__".into(), SymbolUsage::Used, range)?; } if for_class { // It gets set when we create the type params tuple and used when // we build up the bases. - self.register_name(".type_params", SymbolUsage::Assigned, range)?; - self.register_name(".type_params", SymbolUsage::Used, range)?; - self.register_name(".generic_base", SymbolUsage::Assigned, range)?; - self.register_name(".generic_base", SymbolUsage::Used, range)?; + self.register_name(&".type_params".into(), SymbolUsage::Assigned, range)?; + self.register_name(&".type_params".into(), SymbolUsage::Used, range)?; + self.register_name(&".generic_base".into(), SymbolUsage::Assigned, range)?; + self.register_name(&".generic_base".into(), SymbolUsage::Used, range)?; } if has_defaults { - self.register_name(".defaults", SymbolUsage::Parameter, range)?; + self.register_name(&".defaults".into(), SymbolUsage::Parameter, range)?; } if has_kwdefaults { - self.register_name(".kwdefaults", SymbolUsage::Parameter, range)?; + self.register_name(&".kwdefaults".into(), SymbolUsage::Parameter, range)?; } Ok(()) @@ -1267,7 +1267,7 @@ impl SymbolTableBuilder { // Create annotation block if not exists if current.annotation_block.is_none() { let mut annotation_table = SymbolTable::new( - "__annotate__".to_owned(), + Name::new_static("__annotate__"), CompilerScope::Annotation, line_number, is_nested, @@ -1311,10 +1311,10 @@ impl SymbolTableBuilder { fn add_classdict_freevar(&mut self) { let table = self.tables.last_mut().unwrap(); - let name = "__classdict__"; + let name = Name::new_static("__classdict__"); let symbol = table .symbols - .entry(name.to_owned()) + .entry(name.clone()) .or_insert_with(|| Symbol::new(name)); symbol.scope = SymbolScope::Free; symbol @@ -1324,10 +1324,10 @@ impl SymbolTableBuilder { fn add_conditional_annotations_freevar(&mut self) { let table = self.tables.last_mut().unwrap(); - let name = "__conditional_annotations__"; + let name = Name::new_static("__conditional_annotations__"); let symbol = table .symbols - .entry(name.to_owned()) + .entry(name.clone()) .or_insert_with(|| Symbol::new(name)); symbol.scope = SymbolScope::Free; symbol @@ -1426,11 +1426,19 @@ impl SymbolTableBuilder { let current = self.tables.last().unwrap(); let can_see_class_scope = current.typ == CompilerScope::Class || current.can_see_class_scope; - self.enter_scope("__annotate__", CompilerScope::Annotation, line_number); + self.enter_scope( + &"__annotate__".into(), + CompilerScope::Annotation, + line_number, + ); self.tables.last_mut().unwrap().can_see_class_scope = can_see_class_scope; self.tables.last_mut().unwrap().add_format_parameter(); if can_see_class_scope { - self.register_name("__classdict__", SymbolUsage::Used, TextRange::default())?; + self.register_name( + &"__classdict__".into(), + SymbolUsage::Used, + TextRange::default(), + )?; } let was_in_unevaluated_annotation = self.tables.last().unwrap().in_unevaluated_annotation; @@ -1519,7 +1527,7 @@ impl SymbolTableBuilder { if should_register_conditional_annotations { self.register_name( - "__conditional_annotations__", + &"__conditional_annotations__".into(), SymbolUsage::Used, annotation.range(), )?; @@ -1554,16 +1562,12 @@ impl SymbolTableBuilder { match &statement { Stmt::Global(StmtGlobal { names, .. }) => { for name in names { - self.register_name(name.as_str(), SymbolUsage::Global, statement.range())?; + self.register_name(name.id(), SymbolUsage::Global, statement.range())?; } } Stmt::Nonlocal(StmtNonlocal { names, .. }) => { for name in names { - self.register_name( - name.as_str(), - SymbolUsage::Nonlocal, - statement.range(), - )?; + self.register_name(name.id(), SymbolUsage::Nonlocal, statement.range())?; } } Stmt::FunctionDef(StmtFunctionDef { @@ -1577,7 +1581,7 @@ impl SymbolTableBuilder { is_async, .. }) => { - self.register_name(name.as_str(), SymbolUsage::Assigned, *range)?; + self.register_name(name.id(), SymbolUsage::Assigned, *range)?; self.scan_parameter_defaults(parameters)?; self.scan_decorators(decorator_list, ExpressionContext::Load)?; @@ -1586,7 +1590,7 @@ impl SymbolTableBuilder { // annotation scopes are nested inside and can see type parameters. if let Some(type_params) = type_params { self.enter_type_param_block( - name.as_str(), + name.id(), *range, false, Self::has_positional_defaults(parameters), @@ -1595,7 +1599,7 @@ impl SymbolTableBuilder { self.scan_type_params(type_params)?; } self.enter_scope_with_parameters( - name.as_str(), + name.id(), parameters, self.line_index_start(*range), returns.as_deref(), @@ -1626,19 +1630,19 @@ impl SymbolTableBuilder { .. }) => { let prev_class = self.class_name.clone(); - self.register_name(name.as_str(), SymbolUsage::Assigned, *range)?; + self.register_name(name.id(), SymbolUsage::Assigned, *range)?; self.scan_decorators(decorator_list, ExpressionContext::Load)?; if let Some(type_params) = type_params { self.enter_type_param_block( - name.as_str(), + name.id(), *range, true, // for_class: enable selective mangling false, false, )?; // Set class_name for mangling in type param scope - self.class_name = Some(name.to_string()); + self.class_name = Some(name.id().clone()); self.scan_type_params(type_params)?; } @@ -1650,11 +1654,7 @@ impl SymbolTableBuilder { self.scan_expressions(&arguments.args, ExpressionContext::Load)?; for keyword in &arguments.keywords { if let Some(arg) = &keyword.arg { - self.check_name( - arg.as_str(), - ExpressionContext::Store, - keyword.range, - )?; + self.check_name(arg.id(), ExpressionContext::Store, keyword.range)?; } } for keyword in &arguments.keywords { @@ -1663,17 +1663,21 @@ impl SymbolTableBuilder { } self.enter_scope( - name.as_str(), + name.id(), CompilerScope::Class, self.line_index_start(*range), ); // Reset in_conditional_block for new class scope let saved_in_conditional = self.in_conditional_block; self.in_conditional_block = false; - self.class_name = Some(name.to_string()); + self.class_name = Some(name.id().clone()); if type_params.is_some() { - self.register_name(".type_params", SymbolUsage::Used, *range)?; - self.register_name("__type_params__", SymbolUsage::Assigned, *range)?; + self.register_name(&".type_params".into(), SymbolUsage::Used, *range)?; + self.register_name( + &"__type_params__".into(), + SymbolUsage::Assigned, + *range, + )?; } self.scan_statements(body)?; self.leave_scope(); @@ -1754,11 +1758,7 @@ impl SymbolTableBuilder { for name in names { if let Some(alias) = &name.asname { // `import my_module as my_alias` - self.register_name( - alias.as_str(), - SymbolUsage::Imported, - name.name.range, - )?; + self.register_name(alias.id(), SymbolUsage::Imported, name.name.range)?; } else if name.name.as_str() == "*" { // Star imports are only allowed at module level if self.tables.last().unwrap().typ != CompilerScope::Module { @@ -1775,14 +1775,14 @@ impl SymbolTableBuilder { // Don't register star imports as symbols } else { // `import module` or `from x import name` - let imported_name = name.name.split('.').next().unwrap(); + let imported_name = name.name.split('.').next().unwrap().into(); self.check_name( - imported_name, + &imported_name, ExpressionContext::Store, name.name.range, )?; self.register_name( - imported_name, + &imported_name, SymbolUsage::Imported, name.name.range, )?; @@ -1828,14 +1828,12 @@ impl SymbolTableBuilder { range: target_range, .. }) => { - let id_str = id.as_str(); - if *simple { let existing_flags = self.tables.last().and_then(|table| { let name = maybe_mangle_name( - self.class_name.as_deref(), + self.class_name.as_ref(), table.mangled_names.as_ref(), - id_str, + id, ); table.symbols.get(name.as_ref()).map(|symbol| symbol.flags) }); @@ -1854,9 +1852,7 @@ impl SymbolTableBuilder { "nonlocal" }; return Err(SymbolTableError { - error: format!( - "annotated name '{id_str}' can't be {usage}" - ), + error: format!("annotated name '{id}' can't be {usage}"), location: Some( self.source_file.to_source_code().source_location( range.start(), @@ -1867,7 +1863,7 @@ impl SymbolTableBuilder { } self.register_name( - id_str, + id, SymbolUsage::AnnotationAssigned, *target_range, )?; @@ -1876,14 +1872,14 @@ impl SymbolTableBuilder { match current_scope { Some(CompilerScope::Module) => { self.register_name( - "__annotate__", + &"__annotate__".into(), SymbolUsage::Assigned, *range, )?; } Some(CompilerScope::Class) => { self.register_name( - "__annotate_func__", + &"__annotate_func__".into(), SymbolUsage::Assigned, *range, )?; @@ -1891,7 +1887,7 @@ impl SymbolTableBuilder { _ => {} } } else if value.is_some() { - self.register_name(id_str, SymbolUsage::Assigned, *target_range)?; + self.register_name(id, SymbolUsage::Assigned, *target_range)?; } } _ => { @@ -1955,11 +1951,7 @@ impl SymbolTableBuilder { self.scan_expression(expression, ExpressionContext::Load)?; } if let Some(name) = name { - self.register_name( - name.as_str(), - SymbolUsage::Assigned, - handler.range(), - )?; + self.register_name(name.id(), SymbolUsage::Assigned, handler.range())?; } self.scan_statements(body)?; } @@ -2006,7 +1998,7 @@ impl SymbolTableBuilder { ), }); }; - let alias_name = name_expr.id.to_string(); + let alias_name = name_expr.id(); self.scan_expression(name, ExpressionContext::Store)?; // Check before entering any sub-scopes let in_class = self @@ -2015,23 +2007,27 @@ impl SymbolTableBuilder { .is_some_and(|t| t.typ == CompilerScope::Class); let is_generic = type_params.is_some(); if let Some(type_params) = type_params { - self.enter_type_param_block(&alias_name, *range, false, false, false)?; + self.enter_type_param_block(alias_name, *range, false, false, false)?; self.scan_type_params(type_params)?; } // Value scope for lazy evaluation self.enter_scope( - &alias_name, + alias_name, CompilerScope::TypeAlias, self.line_index_start(*range), ); // Evaluator takes a format parameter - self.register_name(".format", SymbolUsage::Parameter, *range)?; - self.register_name(".format", SymbolUsage::Used, *range)?; + self.register_name(&".format".into(), SymbolUsage::Parameter, *range)?; + self.register_name(&".format".into(), SymbolUsage::Used, *range)?; if in_class { if let Some(table) = self.tables.last_mut() { table.can_see_class_scope = true; } - self.register_name("__classdict__", SymbolUsage::Used, value.range())?; + self.register_name( + &"__classdict__".into(), + SymbolUsage::Used, + value.range(), + )?; } self.scan_expression(value, ExpressionContext::Load)?; self.leave_scope(); @@ -2159,7 +2155,7 @@ impl SymbolTableBuilder { Expr::Attribute(ExprAttribute { value, attr, range, .. }) => { - self.check_name(attr.as_str(), context, *range)?; + self.check_name(attr.id(), context, *range)?; self.scan_expression(value, ExpressionContext::Load)?; } Expr::Dict(ExprDict { items, .. }) => { @@ -2273,7 +2269,14 @@ impl SymbolTableBuilder { self.in_iter_def_exp = true; } // Generator expression - is_generator = true - self.scan_comprehension("", elt, None, generators, *range, true)?; + self.scan_comprehension( + &"".into(), + elt, + None, + generators, + *range, + true, + )?; self.in_iter_def_exp = was_in_iter_def_exp; } Expr::ListComp(ExprListComp { @@ -2287,7 +2290,14 @@ impl SymbolTableBuilder { self.in_iter_def_exp = true; } // List comprehension - is_generator = false (can be inlined) - self.scan_comprehension("", elt, None, generators, *range, false)?; + self.scan_comprehension( + &"".into(), + elt, + None, + generators, + *range, + false, + )?; self.in_iter_def_exp = was_in_iter_def_exp; } Expr::SetComp(ExprSetComp { @@ -2301,7 +2311,14 @@ impl SymbolTableBuilder { self.in_iter_def_exp = true; } // Set comprehension - is_generator = false (can be inlined) - self.scan_comprehension("", elt, None, generators, *range, false)?; + self.scan_comprehension( + &"".into(), + elt, + None, + generators, + *range, + false, + )?; self.in_iter_def_exp = was_in_iter_def_exp; } Expr::DictComp(ExprDictComp { @@ -2318,7 +2335,7 @@ impl SymbolTableBuilder { // Dict comprehension - is_generator = false (can be inlined) let key = key.as_ref(); self.scan_comprehension( - "", + &"".into(), key, Some(value), generators, @@ -2342,7 +2359,7 @@ impl SymbolTableBuilder { self.scan_expressions(&arguments.args, ExpressionContext::Load)?; for keyword in &arguments.keywords { if let Some(arg) = &keyword.arg { - self.check_name(arg.as_str(), ExpressionContext::Store, keyword.range)?; + self.check_name(arg.id(), ExpressionContext::Store, keyword.range)?; } } for keyword in &arguments.keywords { @@ -2350,8 +2367,6 @@ impl SymbolTableBuilder { } } Expr::Name(ExprName { id, range, .. }) => { - let id = id.as_str(); - self.check_name(id, context, *range)?; if !self @@ -2380,7 +2395,7 @@ impl SymbolTableBuilder { && Self::is_function_like_scope(self.tables.last().unwrap().typ) && id == "super" { - self.register_name("__class__", SymbolUsage::Used, *range)?; + self.register_name(&"__class__".into(), SymbolUsage::Used, *range)?; } } } @@ -2393,7 +2408,7 @@ impl SymbolTableBuilder { self.scan_parameter_defaults(parameters)?; } self.enter_scope_with_parameters( - "lambda", + &"lambda".into(), parameters, self.line_index_start(expression.range()), None, // lambdas have no return annotation @@ -2403,7 +2418,7 @@ impl SymbolTableBuilder { )?; } else { self.enter_scope( - "lambda", + &"lambda".into(), CompilerScope::Lambda, self.line_index_start(expression.range()), ); @@ -2512,7 +2527,6 @@ impl SymbolTableBuilder { .. }) = &**target { - let id = id.as_str(); self.check_name(id, ExpressionContext::Store, *target_range)?; let table = self.tables.last().unwrap(); if table.typ == CompilerScope::Comprehension { @@ -2556,7 +2570,7 @@ impl SymbolTableBuilder { fn scan_comprehension( &mut self, - scope_name: &str, + scope_name: &Name, elt1: &ast::Expr, elt2: Option<&ast::Expr>, generators: &[ast::Comprehension], @@ -2596,10 +2610,10 @@ impl SymbolTableBuilder { } // Register the passed argument to the generator function as the name ".0" - self.register_name(".0", SymbolUsage::Parameter, range)?; + self.register_name(&".0".into(), SymbolUsage::Parameter, range)?; let saved_comprehension_yield_context = self.comprehension_yield_context; - self.comprehension_yield_context = Some(match scope_name { + self.comprehension_yield_context = Some(match scope_name.as_ref() { "" => "list comprehension", "" => "set comprehension", "" => "dict comprehension", @@ -2669,7 +2683,7 @@ impl SymbolTableBuilder { fn scan_type_param_bound_or_default( &mut self, expr: &ast::Expr, - scope_name: &str, + scope_name: &Name, scope_info: &'static str, ) -> SymbolTableResult { // Bounds/defaults are compiled as annotation scopes. @@ -2677,14 +2691,14 @@ impl SymbolTableBuilder { let line_number = self.line_index_start(expr.range()); self.enter_scope(scope_name, CompilerScope::TypeVariable, line_number); // Evaluator takes a format parameter - self.register_name(".format", SymbolUsage::Parameter, expr.range())?; - self.register_name(".format", SymbolUsage::Used, expr.range())?; + self.register_name(&".format".into(), SymbolUsage::Parameter, expr.range())?; + self.register_name(&".format".into(), SymbolUsage::Used, expr.range())?; if in_class { if let Some(table) = self.tables.last_mut() { table.can_see_class_scope = true; } - self.register_name("__classdict__", SymbolUsage::Used, expr.range())?; + self.register_name(&"__classdict__".into(), SymbolUsage::Used, expr.range())?; } self.tables.last_mut().unwrap().scope_info = Some(scope_info); @@ -2716,7 +2730,7 @@ impl SymbolTableBuilder { default, .. }) => { - self.register_name(name.as_str(), SymbolUsage::TypeParam, *type_var_range)?; + self.register_name(name.id(), SymbolUsage::TypeParam, *type_var_range)?; if name.as_str() == "__classdict__" { return Err(SymbolTableError { error: format!( @@ -2737,18 +2751,14 @@ impl SymbolTableBuilder { } else { "a TypeVar bound" }; - self.scan_type_param_bound_or_default( - binding, - name.as_str(), - scope_info, - )?; + self.scan_type_param_bound_or_default(binding, name.id(), scope_info)?; } // Process default in a separate scope if let Some(default_value) = default { self.scan_type_param_bound_or_default( default_value, - name.as_str(), + name.id(), "a TypeVar default", )?; } @@ -2759,7 +2769,7 @@ impl SymbolTableBuilder { default, .. }) => { - self.register_name(name, SymbolUsage::TypeParam, *param_spec_range)?; + self.register_name(name.id(), SymbolUsage::TypeParam, *param_spec_range)?; if name == "__classdict__" { return Err(SymbolTableError { error: format!( @@ -2776,7 +2786,7 @@ impl SymbolTableBuilder { if let Some(default_value) = default { self.scan_type_param_bound_or_default( default_value, - name, + name.id(), "a ParamSpec default", )?; } @@ -2787,7 +2797,11 @@ impl SymbolTableBuilder { default, .. }) => { - self.register_name(name, SymbolUsage::TypeParam, *type_var_tuple_range)?; + self.register_name( + name.id(), + SymbolUsage::TypeParam, + *type_var_tuple_range, + )?; if name == "__classdict__" { return Err(SymbolTableError { error: format!( @@ -2804,7 +2818,7 @@ impl SymbolTableBuilder { if let Some(default_value) = default { self.scan_type_param_bound_or_default( default_value, - name, + name.id(), "a TypeVarTuple default", )?; } @@ -2866,7 +2880,7 @@ impl SymbolTableBuilder { ), }); } - self.register_name(rest.as_str(), SymbolUsage::Assigned, pattern.range())?; + self.register_name(rest.id(), SymbolUsage::Assigned, pattern.range())?; } } MatchClass(ast::PatternMatchClass { cls, arguments, .. }) => { @@ -2874,7 +2888,7 @@ impl SymbolTableBuilder { self.scan_patterns(&arguments.patterns)?; for kw in &arguments.keywords { self.check_name( - kw.attr.as_str(), + kw.attr.id(), ExpressionContext::Store, kw.pattern.range(), )?; @@ -2885,7 +2899,7 @@ impl SymbolTableBuilder { } MatchStar(ast::PatternMatchStar { name, .. }) => { if let Some(name) = name { - self.register_name(name.as_str(), SymbolUsage::Assigned, pattern.range())?; + self.register_name(name.id(), SymbolUsage::Assigned, pattern.range())?; } } MatchAs(ast::PatternMatchAs { @@ -2897,7 +2911,7 @@ impl SymbolTableBuilder { self.scan_pattern(as_pattern)?; } if let Some(name) = name { - self.register_name(name.as_str(), SymbolUsage::Assigned, pattern.range())?; + self.register_name(name.id(), SymbolUsage::Assigned, pattern.range())?; } } MatchOr(ast::PatternMatchOr { patterns, .. }) => self.scan_patterns(patterns)?, @@ -2943,7 +2957,7 @@ impl SymbolTableBuilder { )] fn enter_scope_with_parameters( &mut self, - name: &str, + name: &Name, parameters: &ast::Parameters, line_number: u32, returns: Option<&ast::Expr>, @@ -2980,12 +2994,12 @@ impl SymbolTableBuilder { } fn register_ident(&mut self, ident: &ast::Identifier, role: SymbolUsage) -> SymbolTableResult { - self.register_name(ident.as_str(), role, ident.range) + self.register_name(ident.id(), role, ident.range) } fn check_name( &self, - name: &str, + name: &Name, context: ExpressionContext, range: TextRange, ) -> SymbolTableResult { @@ -3017,7 +3031,7 @@ impl SymbolTableBuilder { // Mirrors symtable_extend_namedexpr_scope(): assignment expressions // inside comprehensions bind in the nearest function/module-like scope, not // in the synthetic comprehension scope itself. - fn extend_namedexpr_scope(&mut self, name: &str, range: TextRange) -> SymbolTableResult { + fn extend_namedexpr_scope(&mut self, name: &Name, range: TextRange) -> SymbolTableResult { let location = Some( self.source_file .to_source_code() @@ -3027,7 +3041,7 @@ impl SymbolTableBuilder { for table_idx in (0..self.tables.len()).rev() { let table_type = self.tables[table_idx].typ; let mangled = maybe_mangle_name( - self.class_name.as_deref(), + self.class_name.as_ref(), self.tables[table_idx].mangled_names.as_ref(), name, ) @@ -3059,7 +3073,7 @@ impl SymbolTableBuilder { let current_symbol = current .symbols .entry(mangled.clone()) - .or_insert_with(|| Symbol::new(mangled.as_str())); + .or_insert_with(|| Symbol::new(mangled.clone())); if parent_is_global { current_symbol.flags.insert(SymbolFlags::DEF_GLOBAL); current_symbol.scope = SymbolScope::GlobalExplicit; @@ -3071,7 +3085,7 @@ impl SymbolTableBuilder { let symbol = self.tables[table_idx] .symbols .entry(mangled.clone()) - .or_insert_with(|| Symbol::new(mangled.as_str())); + .or_insert_with(|| Symbol::new(mangled.clone())); symbol.flags.insert(SymbolFlags::DEF_LOCAL); return Ok(()); } @@ -3080,14 +3094,14 @@ impl SymbolTableBuilder { let current_symbol = current .symbols .entry(mangled.clone()) - .or_insert_with(|| Symbol::new(mangled.as_str())); + .or_insert_with(|| Symbol::new(mangled.clone())); current_symbol.flags.insert(SymbolFlags::DEF_GLOBAL); current_symbol.scope = SymbolScope::GlobalExplicit; let symbol = self.tables[table_idx] .symbols .entry(mangled.clone()) - .or_insert_with(|| Symbol::new(mangled.as_str())); + .or_insert_with(|| Symbol::new(mangled.clone())); symbol.flags.insert(SymbolFlags::DEF_GLOBAL); symbol.scope = SymbolScope::GlobalExplicit; return Ok(()); @@ -3130,7 +3144,7 @@ impl SymbolTableBuilder { fn register_name( &mut self, - name: &str, + name: &Name, role: SymbolUsage, range: TextRange, ) -> SymbolTableResult { @@ -3170,11 +3184,7 @@ impl SymbolTableBuilder { } let original_name = name; - let name = maybe_mangle_name( - self.class_name.as_deref(), - table.mangled_names.as_ref(), - name, - ); + let name = &maybe_mangle_name(self.class_name.as_ref(), table.mangled_names.as_ref(), name); // 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; @@ -3305,8 +3315,11 @@ impl SymbolTableBuilder { } } // Insert symbol when required: - let symbol = Symbol::new(name.as_ref()); - table.symbols.entry(name.into_owned()).or_insert(symbol) + let symbol = Symbol::new(name.clone().into_owned()); + table + .symbols + .entry(name.clone().into_owned()) + .or_insert(symbol) }; if matches!(role, SymbolUsage::Global | SymbolUsage::Nonlocal) { @@ -3391,38 +3404,38 @@ fn is_docstring_expr(expr: &ast::Expr) -> bool { ) } -pub(crate) fn mangle_name<'a>(class_name: Option<&str>, name: &'a str) -> Cow<'a, str> { - let class_name = match class_name { - Some(n) => n, - None => return name.into(), +pub(crate) fn mangle_name<'a>(class_name: Option<&Name>, name: &'a Name) -> Cow<'a, Name> { + let Some(class_name) = class_name else { + return Cow::Borrowed(name); }; + if !name.starts_with("__") || name.ends_with("__") || name.contains('.') { - return name.into(); + return Cow::Borrowed(name); } // Strip leading underscores from class name let class_name = class_name.trim_start_matches('_'); if class_name.is_empty() { - return name.into(); + return Cow::Borrowed(name); } let mut ret = String::with_capacity(1 + class_name.len() + name.len()); ret.push('_'); ret.push_str(class_name); ret.push_str(name); - ret.into() + Cow::Owned(ret.into()) } /// Selective mangling for type parameter scopes around generic classes. /// If `mangled_names` is Some, only mangle names that are in the set; /// other names are left unmangled. pub(crate) fn maybe_mangle_name<'a>( - class_name: Option<&str>, - mangled_names: Option<&IndexSet>, - name: &'a str, -) -> Cow<'a, str> { + class_name: Option<&Name>, + mangled_names: Option<&IndexSet>, + name: &'a Name, +) -> Cow<'a, Name> { if let Some(set) = mangled_names && !set.contains(name) { - return name.into(); + return Cow::Borrowed(name); } mangle_name(class_name, name) } @@ -3453,15 +3466,24 @@ mod tests { #[test] fn mangle_name_leaves_private_name_in_underscore_only_class() { - assert_eq!(mangle_name(Some("_"), "__a"), "__a"); - assert_eq!(mangle_name(Some("__"), "__a"), "__a"); - assert_eq!(mangle_name(Some("___"), "__a"), "__a"); + assert_eq!(mangle_name(Some(&"_".into()), &"__a".into()), "__a".into()); + assert_eq!(mangle_name(Some(&"__".into()), &"__a".into()), "__a".into()); + assert_eq!( + mangle_name(Some(&"___".into()), &"__a".into()), + "__a".into() + ); } #[test] fn mangle_name_strips_leading_class_underscores() { - assert_eq!(mangle_name(Some("_a"), "__a"), "_a__a"); - assert_eq!(mangle_name(Some("__a"), "__a"), "_a__a"); + assert_eq!( + mangle_name(Some(&"_a".into()), &"__a".into()), + "_a__a".into() + ); + assert_eq!( + mangle_name(Some(&"__a".into()), &"__a".into()), + "_a__a".into() + ); } #[test] @@ -3490,7 +3512,7 @@ mod tests { .expect("missing lambda scope"); assert!( - lambda.lookup("__class__").is_some(), + lambda.lookup(&"__class__".into()).is_some(), "CPython symtable Name_kind treats super as a __class__ use in any function-like scope" ); } @@ -3504,7 +3526,7 @@ mod tests { .find(|table| table.typ == CompilerScope::Comprehension) .expect("missing comprehension scope"); let symbol = comprehension - .lookup("i") + .lookup(&"i".into()) .expect("missing comprehension iteration target"); assert!( @@ -3552,7 +3574,7 @@ mod tests { .expect("CPython still creates an AnnotationBlock for future annotations"); assert!( - annotation_block.lookup("T").is_some(), + annotation_block.lookup(&"T".into()).is_some(), "CPython symtable_visit_annotation still visits the annotation expression with future annotations" ); } @@ -3566,7 +3588,7 @@ mod tests { .find(|table| table.typ == CompilerScope::Annotation) .expect("missing function annotation block"); let format = annotation_block - .lookup(".format") + .lookup(&".format".into()) .expect("missing annotation .format parameter"); assert!( format @@ -3582,7 +3604,7 @@ mod tests { .find(|table| table.typ == CompilerScope::TypeAlias) .expect("missing type alias scope"); let format = alias - .lookup(".format") + .lookup(&".format".into()) .expect("missing type alias .format parameter"); assert!( format @@ -3603,7 +3625,7 @@ mod tests { .find(|table| table.typ == CompilerScope::TypeVariable) .expect("missing type variable scope"); let format = type_variable - .lookup(".format") + .lookup(&".format".into()) .expect("missing type variable .format parameter"); assert!( format diff --git a/crates/vm/src/stdlib/_symtable.rs b/crates/vm/src/stdlib/_symtable.rs index c4a4a7a2051..d9e9fe0ef16 100644 --- a/crates/vm/src/stdlib/_symtable.rs +++ b/crates/vm/src/stdlib/_symtable.rs @@ -135,7 +135,7 @@ mod _symtable { impl PySymbolTable { #[pygetset] fn name(&self) -> String { - self.symtable.name.clone() + self.symtable.name.to_string() } #[pygetset(name = "type")] @@ -185,7 +185,7 @@ mod _symtable { fn symbols(&self, vm: &VirtualMachine) -> PyDictRef { let dict = vm.ctx.new_dict(); for (name, symbol) in &self.symtable.symbols { - dict.set_item(name, vm.new_pyobj(symbol.flags.bits()), vm) + dict.set_item(name.as_str(), vm.new_pyobj(symbol.flags.bits()), vm) .unwrap(); } dict