Merge pull request #1009 from mkurnikov/refactorings

Some cleanups and typo fixes

src/main.rs

@@ -159,7 +159,7 @@ fn shell_exec(vm: &VirtualMachine, source: &str, scope: Scope) -> Result<(), Com
                 Ok(value) => {
                     // Save non-None values as "_"
 
-                    use rustpython_vm::pyobject::{IdProtocol, IntoPyObject, ItemProtocol};
+                    use rustpython_vm::pyobject::{IdProtocol, IntoPyObject};
 
                     if !value.is(&vm.get_none()) {
                         let key = objstr::PyString::from("_").into_pyobject(vm);

vm/src/frame.rs

@@ -124,15 +124,15 @@ impl Scope {
         self.locals.iter().next().cloned()
     }
 
-    pub fn child_scope_with_locals(&self, locals: PyDictRef) -> Scope {
+    pub fn new_child_scope_with_locals(&self, locals: PyDictRef) -> Scope {
         Scope {
             locals: self.locals.clone().insert(locals),
             globals: self.globals.clone(),
         }
     }
 
-    pub fn child_scope(&self, ctx: &PyContext) -> Scope {
-        self.child_scope_with_locals(ctx.new_dict())
+    pub fn new_child_scope(&self, ctx: &PyContext) -> Scope {
+        self.new_child_scope_with_locals(ctx.new_dict())
     }
 }
 
@@ -651,22 +651,22 @@ impl Frame {
                 // pop argc arguments
                 // argument: name, args, globals
                 let scope = self.scope.clone();
-                let obj = vm
+                let func_obj = vm
                     .ctx
                     .new_function(code_obj, scope, defaults, kw_only_defaults);
 
                 let name = qualified_name.value.split('.').next_back().unwrap();
-                vm.set_attr(&obj, "__name__", vm.new_str(name.to_string()))?;
-                vm.set_attr(&obj, "__qualname__", qualified_name)?;
+                vm.set_attr(&func_obj, "__name__", vm.new_str(name.to_string()))?;
+                vm.set_attr(&func_obj, "__qualname__", qualified_name)?;
                 let module = self
                     .scope
                     .globals
                     .get_item_option("__name__", vm)?
                     .unwrap_or_else(|| vm.get_none());
-                vm.set_attr(&obj, "__module__", module)?;
-                vm.set_attr(&obj, "__annotations__", annotations)?;
+                vm.set_attr(&func_obj, "__module__", module)?;
+                vm.set_attr(&func_obj, "__annotations__", annotations)?;
 
-                self.push_value(obj);
+                self.push_value(func_obj);
                 Ok(None)
             }
             bytecode::Instruction::CallFunction { typ } => {
@@ -1318,14 +1318,14 @@ impl Frame {
             || objtype::isinstance(&exception, &vm.ctx.exceptions.base_exception_type)
         {
             Ok(exception)
-        } else if let Ok(exception) = PyClassRef::try_from_object(vm, exception) {
-            if objtype::issubclass(&exception, &vm.ctx.exceptions.base_exception_type) {
-                let exception = vm.new_empty_exception(exception)?;
+        } else if let Ok(exc_type) = PyClassRef::try_from_object(vm, exception) {
+            if objtype::issubclass(&exc_type, &vm.ctx.exceptions.base_exception_type) {
+                let exception = vm.new_empty_exception(exc_type)?;
                 Ok(exception)
             } else {
                 let msg = format!(
                     "Can only raise BaseException derived types, not {}",
-                    exception
+                    exc_type
                 );
                 Err(vm.new_type_error(msg))
             }

vm/src/stdlib/binascii.rs

@@ -68,7 +68,7 @@ fn binascii_crc32(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
     );
 
     let bytes = objbytes::get_value(data);
-    let mut crc = match value {
+    let crc = match value {
         None => 0u32,
         Some(value) => objint::get_value(&value).to_u32().unwrap(),
     };

vm/src/symboltable.rs

@@ -16,7 +16,8 @@ pub fn make_symbol_table(program: &ast::Program) -> Result<SymbolScope, SymbolTa
     let mut builder = SymbolTableBuilder::new();
     builder.enter_scope();
     builder.scan_program(program)?;
-    assert!(builder.scopes.len() == 1);
+    assert_eq!(builder.scopes.len(), 1);
+
     let symbol_table = builder.scopes.pop().unwrap();
     analyze_symbol_table(&symbol_table, None)?;
     Ok(symbol_table)
@@ -28,7 +29,8 @@ pub fn statements_to_symbol_table(
     let mut builder = SymbolTableBuilder::new();
     builder.enter_scope();
     builder.scan_statements(statements)?;
-    assert!(builder.scopes.len() == 1);
+    assert_eq!(builder.scopes.len(), 1);
+
     let symbol_table = builder.scopes.pop().unwrap();
     analyze_symbol_table(&symbol_table, None)?;
     Ok(symbol_table)
@@ -42,8 +44,7 @@ pub enum SymbolRole {
     Assigned,
 }
 
-/// Symbolscope captures all symbols in the current scope, and
-/// has a list of subscopes in this scope.
+/// Captures all symbols in the current scope, and has a list of subscopes in this scope.
 pub struct SymbolScope {
     /// A set of symbols present on this scope level.
     pub symbols: HashMap<String, SymbolRole>,
@@ -101,7 +102,6 @@ fn analyze_symbol_table(
     symbol_scope: &SymbolScope,
     parent_symbol_scope: Option<&SymbolScope>,
 ) -> SymbolTableResult {
-    // println!("Analyzing {:?}, parent={:?} symbols={:?}", symbol_scope,  parent_symbol_scope, symbol_scope.symbols);
     // Analyze sub scopes:
     for sub_scope in &symbol_scope.sub_scopes {
         analyze_symbol_table(&sub_scope, Some(symbol_scope))?;
@@ -157,7 +157,6 @@ impl SymbolTableBuilder {
     }
 
     pub fn enter_scope(&mut self) {
-        // Create new scope and push into scope stack.
         let scope = SymbolScope::new();
         self.scopes.push(scope);
     }
@@ -494,7 +493,7 @@ impl SymbolTableBuilder {
     }
 
     fn enter_function(&mut self, args: &ast::Parameters) -> SymbolTableResult {
-        // Evaulate eventual default parameters:
+        // Evaluate eventual default parameters:
         self.scan_expressions(&args.defaults)?;
         for kw_default in &args.kw_defaults {
             if let Some(expression) = kw_default {
@@ -545,9 +544,8 @@ impl SymbolTableBuilder {
         let scope_depth = self.scopes.len();
         let current_scope = self.scopes.last_mut().unwrap();
         let location = Default::default();
-        if let Some(_old_role) = current_scope.symbols.get(name) {
+        if current_scope.symbols.contains_key(name) {
             // Role already set..
-            // debug!("TODO: {:?}", old_role);
             match role {
                 SymbolRole::Global => {
                     return Err(SymbolTableError {

vm/src/vm.rs

@@ -171,14 +171,13 @@ impl VirtualMachine {
         self.invoke(exc_type.into_object(), args)
     }
 
+    /// Create Python instance of `exc_type` with message
     pub fn new_exception(&self, exc_type: PyClassRef, msg: String) -> PyObjectRef {
         // TODO: exc_type may be user-defined exception, so we should return PyResult
         // TODO: maybe there is a clearer way to create an instance:
         info!("New exception created: {}", msg);
         let pymsg = self.new_str(msg);
         let args: Vec<PyObjectRef> = vec![pymsg];
-
-        // Call function:
         self.invoke(exc_type.into_object(), args).unwrap()
     }
 
@@ -402,13 +401,13 @@ impl VirtualMachine {
         scope: &Scope,
         defaults: &Option<PyTupleRef>,
         kw_only_defaults: &Option<PyDictRef>,
-        args: PyFuncArgs,
+        func_args: PyFuncArgs,
     ) -> PyResult {
-        let scope = scope.child_scope(&self.ctx);
+        let scope = scope.new_child_scope(&self.ctx);
         self.fill_locals_from_args(
             &code.code,
             &scope.get_locals(),
-            args,
+            func_args,
             defaults,
             kw_only_defaults,
         )?;
@@ -432,8 +431,8 @@ impl VirtualMachine {
     ) -> PyResult {
         if let Some(PyFunction { code, scope, .. }) = &function.payload() {
             let scope = scope
-                .child_scope_with_locals(cells)
-                .child_scope_with_locals(locals);
+                .new_child_scope_with_locals(cells)
+                .new_child_scope_with_locals(locals);
             let frame = Frame::new(code.clone(), scope).into_ref(self);
             return self.run_frame_full(frame);
         }
@@ -447,11 +446,11 @@ impl VirtualMachine {
         &self,
         code_object: &bytecode::CodeObject,
         locals: &PyDictRef,
-        args: PyFuncArgs,
+        func_args: PyFuncArgs,
         defaults: &Option<PyTupleRef>,
         kw_only_defaults: &Option<PyDictRef>,
     ) -> PyResult<()> {
-        let nargs = args.args.len();
+        let nargs = func_args.args.len();
         let nexpected_args = code_object.arg_names.len();
 
         // This parses the arguments from args and kwargs into
@@ -469,7 +468,7 @@ impl VirtualMachine {
         // Copy positional arguments into local variables
         for i in 0..n {
             let arg_name = &code_object.arg_names[i];
-            let arg = &args.args[i];
+            let arg = &func_args.args[i];
             locals.set_item(arg_name, arg.clone(), self)?;
         }
 
@@ -478,7 +477,7 @@ impl VirtualMachine {
             bytecode::Varargs::Named(ref vararg_name) => {
                 let mut last_args = vec![];
                 for i in n..nargs {
-                    let arg = &args.args[i];
+                    let arg = &func_args.args[i];
                     last_args.push(arg.clone());
                 }
                 let vararg_value = self.ctx.new_tuple(last_args);
@@ -508,7 +507,7 @@ impl VirtualMachine {
         };
 
         // Handle keyword arguments
-        for (name, value) in args.kwargs {
+        for (name, value) in func_args.kwargs {
             // Check if we have a parameter with this name:
             if code_object.arg_names.contains(&name) || code_object.kwonlyarg_names.contains(&name)
             {