From 34c5157e47293348b65ba84ef517ab883a822db9 Mon Sep 17 00:00:00 2001 From: Jones Beach Date: Mon, 16 Sep 2024 12:26:20 -0400 Subject: [PATCH] Treewalk: Add hash() builtin and fix bug to support hashing of classes --- docs/SUPPORTED.md | 2 +- src/treewalk/interpreter.rs | 511 +++++++++++++--------- src/treewalk/scope_manager.rs | 28 +- src/treewalk/types/class.rs | 13 +- src/treewalk/types/domain/builtins.rs | 46 +- src/treewalk/types/object.rs | 20 +- src/treewalk/types/result.rs | 6 +- src/treewalk/types/utils/contextual.rs | 40 +- src/treewalk/types/utils/resolved_args.rs | 3 +- 9 files changed, 416 insertions(+), 253 deletions(-) diff --git a/docs/SUPPORTED.md b/docs/SUPPORTED.md index c6e2fd6..372bf6f 100644 --- a/docs/SUPPORTED.md +++ b/docs/SUPPORTED.md @@ -82,7 +82,7 @@ |`getattr`|✅| |`globals`|✅| |`hasattr`|| -|`hash`|| +|`hash`|✅| |`help`|| |`hex`|| |`id`|| diff --git a/src/treewalk/interpreter.rs b/src/treewalk/interpreter.rs index e579f52..dd86315 100644 --- a/src/treewalk/interpreter.rs +++ b/src/treewalk/interpreter.rs @@ -42,96 +42,175 @@ impl Interpreter { callable: Container>, arguments: &ResolvedArguments, ) -> Result { - match self._call(callable, arguments) { + let mut bound_args = arguments.clone(); + if let Some(receiver) = callable.borrow().receiver() { + bound_args.bind(receiver); + } + + match callable.borrow().call(self, bound_args) { Err(InterpreterError::EncounteredReturn(result)) => Ok(result), Err(e) => Err(e), Ok(result) => Ok(result), } } - fn _call( + pub fn call_function( &self, - callable: Container>, + name: &str, arguments: &ResolvedArguments, ) -> Result { - let mut bound_args = arguments.clone(); - if let Some(receiver) = callable.borrow().receiver() { - bound_args.bind(receiver); - } - - callable.borrow().call(self, bound_args) + let function = self.read_callable(name)?; + self.call_function_inner(function, arguments) } - fn import_module(&self, import_path: &ImportPath) -> Result { - // is this useful? is it valuable to read a module directly from the scope as opposed from - // the module cache - if let Some(module) = self.state.read(&import_path.as_str()) { - return Ok(module); + pub fn invoke_function( + &self, + function: Container, + scope: Container, + ) -> Result { + let cross_module = !self + .state + .current_module() + .same_identity(&function.borrow().module); + if cross_module { + self.state.push_module(function.borrow().module.clone()); } + self.state + .push_captured_env(function.borrow().captured_env.clone()); + self.state.push_local(scope); + self.state + .push_context(StackFrame::new_function(function.borrow().clone())); + self.state.push_function(function.clone()); - #[cfg(feature = "c_stdlib")] - if BUILTIN_MODULE_NAMES.contains(&import_path.as_str().as_str()) { - return Ok(ExprResult::CPythonModule( - self.state.import_builtin_module(import_path), - )); + // We do not propagate errors here because we still must restore the scopes and things + // before returning. + let result = self.evaluate_block(&function.borrow().body); + + // If an error is thrown, we should return that immediately without restoring any state. + if matches!(result, Ok(_) | Err(InterpreterError::EncounteredReturn(_))) { + self.state.pop_context(); + self.state.pop_function(); + self.state.pop_local(); + self.state.pop_captured_env(); + if cross_module { + self.state.pop_module(); + } } - Ok(ExprResult::Module(Module::import(self, import_path)?)) + result } - fn evaluate_variable(&self, name: &str) -> Result { - self.state.read(name).ok_or(InterpreterError::NameError( - name.to_owned(), - self.state.call_stack(), - )) + pub fn resolve_method( + &self, + receiver: ExprResult, + name: &str, + ) -> Result>, InterpreterError> { + self.evaluate_member_access_inner(&receiver, name)? + .as_callable() + .ok_or(InterpreterError::MethodNotFound( + name.to_string(), + self.state.call_stack(), + )) } - fn evaluate_unary_operation( + pub fn invoke_method( &self, - op: &UnaryOp, - right: &Expr, + receiver: ExprResult, + name: &str, + arguments: &ResolvedArguments, ) -> Result { - let right = self.evaluate_expr(right)?; - evaluators::evaluate_unary_operation(op, right, self.state.call_stack()) + log(LogLevel::Debug, || { + format!("Calling method {}.{}", receiver, name) + }); + log(LogLevel::Trace, || { + format!("... from module: {}", self.state.current_module()) + }); + log(LogLevel::Trace, || { + format!( + "... from path: {}", + self.state.current_module().borrow().path().display() + ) + }); + if let Some(class) = self.state.current_class() { + log(LogLevel::Trace, || format!("... from class: {}", class)); + } + + let method = self.resolve_method(receiver, name)?; + self.call(method, arguments) } - fn evaluate_ternary_operation( + fn call_function_inner( &self, - condition: &Expr, - if_value: &Expr, - else_value: &Expr, + function: Container>, + arguments: &ResolvedArguments, ) -> Result { - if self.evaluate_expr(condition)?.as_boolean() { - self.evaluate_expr(if_value) - } else { - self.evaluate_expr(else_value) + let function_type = function.borrow().function_type(); + match function_type { + FunctionType::Generator => { + // TODO we may want to support builtin generators in the future. For now, we only + // support user-defined so we are safe to downcast to `Container`. + let function = function + .borrow() + .as_any() + .downcast_ref::>() + .cloned() + .ok_or(InterpreterError::ExpectedFunction(self.state.call_stack()))?; + let scope = Scope::new(self, &function, arguments)?; + let generator_function = Generator::new(scope, function); + let generator_iterator = GeneratorIterator::new(generator_function, self.clone()); + Ok(ExprResult::Generator(Container::new(generator_iterator))) + } + FunctionType::Async => { + let function = function + .borrow() + .as_any() + .downcast_ref::>() + .cloned() + .ok_or(InterpreterError::ExpectedFunction(self.state.call_stack()))?; + let scope = Scope::new(self, &function, arguments)?; + let coroutine = Coroutine::new(scope, function); + Ok(ExprResult::Coroutine(Container::new(coroutine))) + } + FunctionType::Regular => self.call(function, arguments), } } - fn evaluate_logical_operation( - &self, - left: &Expr, - op: &LogicalOp, - right: &Expr, - ) -> Result { - let left = self.evaluate_expr(left)?.as_boolean(); - let right = self.evaluate_expr(right)?.as_boolean(); - evaluators::evaluate_logical_op(left, op, right) + // ----------------------------- + // End of higher-order functions + // ----------------------------- + + fn read_callable(&self, name: &str) -> Result>, InterpreterError> { + self.state + .read(name) + .and_then(|val| val.as_callable()) + .ok_or(InterpreterError::FunctionNotFound( + name.to_string(), + self.state.call_stack(), + )) } - fn evaluate_binary_operation_outer( + fn read_index( &self, - left: &Expr, - op: &BinOp, - right: &Expr, + object: ExprResult, + index: ExprResult, ) -> Result { - let left = self.evaluate_expr(left)?; - let right = self.evaluate_expr(right)?; - - self.evaluate_binary_operation(left, op, right) + object + .as_index_read(self) + .ok_or(InterpreterError::TypeError( + Some(format!( + "'{}' object is not subscriptable", + object.get_type() + )), + self.state.call_stack(), + ))? + .getitem(self, index.clone())? + .ok_or(InterpreterError::KeyError( + index.to_string(), + self.state.call_stack(), + )) } - fn evaluate_binary_operation( + fn evaluate_binary_operation_inner( &self, left: ExprResult, op: &BinOp, @@ -194,21 +273,12 @@ impl Interpreter { && Dunder::try_from(op).is_ok() { let dunder = Dunder::try_from(op).unwrap_or_else(|_| unreachable!()); - self.evaluate_method(left, &dunder, &resolved_args!(right)) + self.invoke_method(left, &dunder, &resolved_args!(right)) } else { evaluators::evaluate_object_comparison(left, op, right) } } - fn evaluate_member_access( - &self, - object: &Expr, - field: &str, - ) -> Result { - let result = self.evaluate_expr(object)?; - self.evaluate_member_access_inner(&result, field) - } - fn evaluate_member_access_inner( &self, result: &ExprResult, @@ -227,6 +297,91 @@ impl Interpreter { )) } + // ----------------------------- + // End of medium-order functions + // ----------------------------- + + fn evaluate_module_import( + &self, + import_path: &ImportPath, + ) -> Result { + // is this useful? is it valuable to read a module directly from the scope as opposed from + // the module cache + if let Some(module) = self.state.read(&import_path.as_str()) { + return Ok(module); + } + + #[cfg(feature = "c_stdlib")] + if BUILTIN_MODULE_NAMES.contains(&import_path.as_str().as_str()) { + return Ok(ExprResult::CPythonModule( + self.state.import_builtin_module(import_path), + )); + } + + Ok(ExprResult::Module(Module::import(self, import_path)?)) + } + + fn evaluate_variable(&self, name: &str) -> Result { + self.state.read(name).ok_or(InterpreterError::NameError( + name.to_owned(), + self.state.call_stack(), + )) + } + + fn evaluate_unary_operation( + &self, + op: &UnaryOp, + right: &Expr, + ) -> Result { + let right = self.evaluate_expr(right)?; + evaluators::evaluate_unary_operation(op, right, self.state.call_stack()) + } + + fn evaluate_ternary_operation( + &self, + condition: &Expr, + if_value: &Expr, + else_value: &Expr, + ) -> Result { + if self.evaluate_expr(condition)?.as_boolean() { + self.evaluate_expr(if_value) + } else { + self.evaluate_expr(else_value) + } + } + + fn evaluate_logical_operation( + &self, + left: &Expr, + op: &LogicalOp, + right: &Expr, + ) -> Result { + let left = self.evaluate_expr(left)?.as_boolean(); + let right = self.evaluate_expr(right)?.as_boolean(); + evaluators::evaluate_logical_op(left, op, right) + } + + fn evaluate_binary_operation( + &self, + left: &Expr, + op: &BinOp, + right: &Expr, + ) -> Result { + let left = self.evaluate_expr(left)?; + let right = self.evaluate_expr(right)?; + + self.evaluate_binary_operation_inner(left, op, right) + } + + fn evaluate_member_access( + &self, + object: &Expr, + field: &str, + ) -> Result { + let result = self.evaluate_expr(object)?; + self.evaluate_member_access_inner(&result, field) + } + fn evaluate_slice_operation( &self, object: &Expr, @@ -235,20 +390,7 @@ impl Interpreter { let object_result = self.evaluate_expr(object)?; let slice = Slice::resolve(self, params)?; - object_result - .as_index_read(self) - .ok_or(InterpreterError::TypeError( - Some(format!( - "'{}' object is not subscriptable", - object_result.get_type() - )), - self.state.call_stack(), - ))? - .getitem(self, ExprResult::Slice(slice.clone()))? - .ok_or(InterpreterError::KeyError( - slice.to_string(), - self.state.call_stack(), - )) + self.read_index(object_result, ExprResult::Slice(slice)) } fn evaluate_index_access( @@ -256,23 +398,10 @@ impl Interpreter { object: &Expr, index: &Expr, ) -> Result { - let index_result = self.evaluate_expr(index)?; let object_result = self.evaluate_expr(object)?; + let index_result = self.evaluate_expr(index)?; - object_result - .as_index_read(self) - .ok_or(InterpreterError::TypeError( - Some(format!( - "'{}' object is not subscriptable", - object_result.get_type() - )), - self.state.call_stack(), - ))? - .getitem(self, index_result.clone())? - .ok_or(InterpreterError::KeyError( - index_result.to_string(), - self.state.call_stack(), - )) + self.read_index(object_result, index_result) } fn evaluate_list(&self, items: &[Expr]) -> Result { @@ -356,7 +485,7 @@ impl Interpreter { } } - fn evaluate_delete(&self, exprs: &Vec) -> Result<(), InterpreterError> { + fn evaluate_delete(&self, exprs: &[Expr]) -> Result<(), InterpreterError> { for expr in exprs { match expr { Expr::Variable(name) => { @@ -458,114 +587,10 @@ impl Interpreter { InterpreterError::FunctionNotFound("".into(), self.state.call_stack()), )? } else { - self.state - .read(name) - .and_then(|val| val.as_callable()) - .ok_or(InterpreterError::FunctionNotFound( - name.to_string(), - self.state.call_stack(), - ))? + self.read_callable(name)? }; - let function_type = function.borrow().function_type(); - match function_type { - FunctionType::Generator => { - // TODO we may want to support builtin generators in the future. For now, we only - // support user-defined so we are safe to downcast to `Container`. - let function = function - .borrow() - .as_any() - .downcast_ref::>() - .cloned() - .ok_or(InterpreterError::ExpectedFunction(self.state.call_stack()))?; - let scope = Scope::new(self, &function, &arguments)?; - let generator_function = Generator::new(scope, function); - let generator_iterator = GeneratorIterator::new(generator_function, self.clone()); - Ok(ExprResult::Generator(Container::new(generator_iterator))) - } - FunctionType::Async => { - let function = function - .borrow() - .as_any() - .downcast_ref::>() - .cloned() - .ok_or(InterpreterError::ExpectedFunction(self.state.call_stack()))?; - let scope = Scope::new(self, &function, &arguments)?; - let coroutine = Coroutine::new(scope, function); - Ok(ExprResult::Coroutine(Container::new(coroutine))) - } - FunctionType::Regular => self.call(function, &arguments), - } - } - - pub fn invoke_function( - &self, - function: Container, - scope: Container, - ) -> Result { - let cross_module = !self - .state - .current_module() - .same_identity(&function.borrow().module); - if cross_module { - self.state.push_module(function.borrow().module.clone()); - } - self.state - .push_captured_env(function.borrow().captured_env.clone()); - self.state.push_local(scope); - self.state - .push_context(StackFrame::new_function(function.borrow().clone())); - self.state.push_function(function.clone()); - - // We do not propagate errors here because we still must restore the scopes and things - // before returning. - let result = self.evaluate_block(&function.borrow().body); - - // If an error is thrown, we should return that immediately without restoring any state. - if matches!(result, Ok(_) | Err(InterpreterError::EncounteredReturn(_))) { - self.state.pop_context(); - self.state.pop_function(); - self.state.pop_local(); - self.state.pop_captured_env(); - if cross_module { - self.state.pop_module(); - } - } - - result - } - - pub fn evaluate_method( - &self, - receiver: ExprResult, - name: &str, - arguments: &ResolvedArguments, - ) -> Result { - log(LogLevel::Debug, || { - format!("Calling method {}.{}", receiver, name) - }); - log(LogLevel::Trace, || { - format!("... from module: {}", self.state.current_module()) - }); - log(LogLevel::Trace, || { - format!( - "... from path: {}", - self.state.current_module().borrow().path().display() - ) - }); - if let Some(class) = self.state.current_class() { - log(LogLevel::Trace, || format!("... from class: {}", class)); - } - - let function = self - .evaluate_member_access_inner(&receiver, name)? - .as_callable() - .ok_or(InterpreterError::MethodNotFound( - name.to_string(), - self.state.call_stack(), - ))?; - - self.call(function, arguments) + self.call_function_inner(function, &arguments) } fn evaluate_method_call( @@ -577,7 +602,7 @@ impl Interpreter { let arguments = ResolvedArguments::from(self, arguments)?; let result = self.evaluate_expr(obj)?; - self.evaluate_method(result, name, &arguments) + self.invoke_method(result, name, &arguments) } fn evaluate_class_instantiation( @@ -623,7 +648,7 @@ impl Interpreter { value: &Expr, ) -> Result<(), InterpreterError> { let bin_op = BinOp::from(operator); - let result = self.evaluate_binary_operation_outer(target, &bin_op, value)?; + let result = self.evaluate_binary_operation(target, &bin_op, value)?; self.evaluate_assignment_inner(target, result) } @@ -999,7 +1024,7 @@ impl Interpreter { alias: &Option, ) -> Result<(), InterpreterError> { // A mutable ExprResult::Module that will be updated on each loop iteration - let mut inner_module = self.import_module(import_path)?; + let mut inner_module = self.evaluate_module_import(import_path)?; // This is a case where it's simpler if we have an alias: just make the module available // at the alias. @@ -1035,12 +1060,13 @@ impl Interpreter { arguments: &[ImportedItem], wildcard: &bool, ) -> Result<(), InterpreterError> { - let module = self.import_module(import_path)?.as_module().ok_or( - InterpreterError::ModuleNotFound( + let module = self + .evaluate_module_import(import_path)? + .as_module() + .ok_or(InterpreterError::ModuleNotFound( import_path.as_str().to_string(), self.state.call_stack(), - ), - )?; + ))?; let mapped_imports = arguments .iter() @@ -1096,15 +1122,14 @@ impl Interpreter { )); } - let result = - self.evaluate_method(expr_result.clone(), &Dunder::Enter, &resolved_args!())?; + let result = self.invoke_method(expr_result.clone(), &Dunder::Enter, &resolved_args!())?; if let Some(variable) = variable { self.state.write(variable, result); } let block_result = self.evaluate_block(block); - self.evaluate_method( + self.invoke_method( expr_result.clone(), &Dunder::Exit, &resolved_args!(ExprResult::None, ExprResult::None, ExprResult::None), @@ -1263,7 +1288,7 @@ impl Interpreter { } => self.evaluate_dict_comprehension(key, value, range, key_body, value_body), Expr::UnaryOperation { op, right } => self.evaluate_unary_operation(op, right), Expr::BinaryOperation { left, op, right } => { - self.evaluate_binary_operation_outer(left, op, right) + self.evaluate_binary_operation(left, op, right) } Expr::Await { right } => self.evaluate_await(right), Expr::FunctionCall { name, args, callee } => { @@ -7140,7 +7165,7 @@ except Exception as e: interpreter.state.call_stack() )) ), - _ => panic!(), + _ => panic!("Expected error!"), } } } @@ -8947,4 +8972,62 @@ del my['one'] } } } + + #[test] + fn hash_builtin() { + let input = r#" +a = hash(5) +b = hash(complex) + +class Foo: + def __hash__(self): + return 4.5 + +c = hash(Foo) + +the_dict = {} +the_dict[complex] = True +d = the_dict[complex] + +try: + hash(Foo()) +except Exception as e: + the_exp = e +"#; + + let (mut parser, mut interpreter) = init(input); + + match interpreter.run(&mut parser) { + Err(e) => panic!("Interpreter error: {:?}", e), + Ok(_) => { + assert_eq!( + interpreter.state.read("a"), + Some(ExprResult::Integer(5.store())) + ); + match interpreter.state.read("b") { + Some(ExprResult::Integer(a)) => { + assert!(a.borrow().clone() != 0); + } + _ => panic!("Unexpected type!"), + } + match interpreter.state.read("c") { + Some(ExprResult::Integer(a)) => { + assert!(a.borrow().clone() != 0); + } + _ => panic!("Unexpected type!"), + } + assert_eq!(interpreter.state.read("d"), Some(ExprResult::Boolean(true))); + match interpreter.state.read("the_exp") { + Some(ExprResult::Exception(e)) => assert_eq!( + e, + Box::new(InterpreterError::TypeError( + Some("__hash__ method should return an integer".into()), + interpreter.state.call_stack() + )) + ), + _ => panic!("Expected error!"), + } + } + } + } } diff --git a/src/treewalk/scope_manager.rs b/src/treewalk/scope_manager.rs index 1baebfa..72fedae 100644 --- a/src/treewalk/scope_manager.rs +++ b/src/treewalk/scope_manager.rs @@ -1,16 +1,21 @@ -use crate::core::{Container, Stack}; -use crate::domain::Context; -use crate::treewalk::executor::{AsyncioCreateTaskBuiltin, AsyncioRunBuiltin, AsyncioSleepBuiltin}; -use crate::treewalk::types::{ - domain::{ - builtins::{ - CallableBuiltin, DirBuiltin, GetattrBuiltin, GlobalsBuiltin, IsinstanceBuiltin, - IssubclassBuiltin, IterBuiltin, LenBuiltin, NextBuiltin, PrintBuiltin, +use crate::{ + core::{Container, Stack}, + domain::Context, + treewalk::{ + executor::{AsyncioCreateTaskBuiltin, AsyncioRunBuiltin, AsyncioSleepBuiltin}, + types::{ + domain::{ + builtins::{ + CallableBuiltin, DirBuiltin, GetattrBuiltin, GlobalsBuiltin, HashBuiltin, + IsinstanceBuiltin, IssubclassBuiltin, IterBuiltin, LenBuiltin, NextBuiltin, + PrintBuiltin, + }, + traits::Callable, + }, + utils::EnvironmentFrame, + ExprResult, Module, }, - traits::Callable, }, - utils::EnvironmentFrame, - ExprResult, Module, }; use super::{LoadedModule, Scope, TypeRegistry}; @@ -29,6 +34,7 @@ fn get_builtins() -> Vec> { Box::new(DirBuiltin), Box::new(GetattrBuiltin), Box::new(GlobalsBuiltin), + Box::new(HashBuiltin), Box::new(IsinstanceBuiltin), Box::new(IssubclassBuiltin), Box::new(IterBuiltin), diff --git a/src/treewalk/types/class.rs b/src/treewalk/types/class.rs index f88440a..92616b9 100644 --- a/src/treewalk/types/class.rs +++ b/src/treewalk/types/class.rs @@ -60,7 +60,7 @@ impl Class { ExprResult::Dict(Scope::default().as_dict(interpreter.clone())) ); interpreter - .evaluate_method(ExprResult::Class(metaclass), &Dunder::New, args)? + .invoke_method(ExprResult::Class(metaclass), &Dunder::New, args)? .as_class() .ok_or(InterpreterError::ExpectedClass( interpreter.state.call_stack(), @@ -220,7 +220,7 @@ impl Container { /// Use the class MRO to search for an attribute. This does not consider metaclasses but it /// does consider the class itself. - fn search(&self, iterable: Vec>, name: &str) -> Option { + fn search(iterable: &[Container], name: &str) -> Option { for class in iterable { if let Some(attr) = class.borrow().scope.get(name) { return Some(attr); @@ -231,7 +231,7 @@ impl Container { } fn search_mro(&self, name: &str) -> Option { - self.search(self.mro(), name) + Self::search(&self.mro(), name) } pub fn get_from_class(&self, name: &str) -> Option { @@ -250,6 +250,8 @@ impl Container { self.borrow().metaclass().search_mro(name) } + /// Insert into the class scope. Used by `MemberWriter` or anywhere we do not have an + /// `Interpreter`, like in the `TypeRegistry` on startup. pub fn set_on_class(&self, name: &str, value: ExprResult) { self.borrow_mut().scope.insert(name, value); } @@ -268,7 +270,7 @@ impl MemberReader for Container { ) -> Result, InterpreterError> { if let Some(attr) = self.get_from_class(name) { log(LogLevel::Debug, || { - format!("Found: {}::{} on class", self, name) + format!("Found: {}::{} on class [from class]", self, name) }); return Ok(Some(attr.resolve_nondata_descriptor( interpreter, @@ -301,8 +303,7 @@ impl MemberWriter for Container { self.borrow_mut().scope.delete(name); // TODO support delete attributes from parent classes? - - Ok(()) + todo!(); } fn set_member( diff --git a/src/treewalk/types/domain/builtins.rs b/src/treewalk/types/domain/builtins.rs index 3bdd2b8..6dac690 100644 --- a/src/treewalk/types/domain/builtins.rs +++ b/src/treewalk/types/domain/builtins.rs @@ -1,8 +1,11 @@ use crate::{ - core::Container, + core::{Container, Storable}, + resolved_args, treewalk::{ types::{ - domain::traits::Callable, iterators::StringIterator, utils::ResolvedArguments, + domain::traits::Callable, + iterators::StringIterator, + utils::{Dunder, ResolvedArguments}, ExprResult, List, Str, }, Interpreter, @@ -14,6 +17,7 @@ pub struct CallableBuiltin; pub struct DirBuiltin; pub struct GetattrBuiltin; pub struct GlobalsBuiltin; +pub struct HashBuiltin; pub struct IsinstanceBuiltin; pub struct IssubclassBuiltin; pub struct IterBuiltin; @@ -122,8 +126,34 @@ impl Callable for GlobalsBuiltin { } } -fn has_overlap(vec1: &[T], vec2: &[T]) -> bool { - vec1.iter().any(|item| vec2.contains(item)) +impl Callable for HashBuiltin { + fn call( + &self, + interpreter: &Interpreter, + args: ResolvedArguments, + ) -> Result { + utils::validate_args(&args, 1, interpreter.state.call_stack())?; + + let arg = args.get_arg(0); + if arg.as_class().is_some() { + return Ok(ExprResult::Integer((arg.hash() as i64).store())); + } + + let result = interpreter.invoke_method(arg, &Dunder::Hash, &resolved_args!())?; + + if let ExprResult::Integer(_) = result { + Ok(result) + } else { + Err(InterpreterError::TypeError( + Some(format!("{} method should return an integer", Dunder::Hash)), + interpreter.state.call_stack(), + )) + } + } + + fn name(&self) -> String { + "hash".into() + } } impl Callable for IsinstanceBuiltin { @@ -160,9 +190,9 @@ impl Callable for IsinstanceBuiltin { }; let isinstance = if args.get_arg(0).is_class() { - has_overlap(&reference_class, &instance_class.borrow().metaclass().mro()) + utils::has_overlap(&reference_class, &instance_class.borrow().metaclass().mro()) } else { - has_overlap(&reference_class, &instance_class.mro()) + utils::has_overlap(&reference_class, &instance_class.mro()) }; Ok(ExprResult::Boolean(isinstance)) @@ -357,4 +387,8 @@ pub mod utils { Ok(()) } } + + pub(crate) fn has_overlap(vec1: &[T], vec2: &[T]) -> bool { + vec1.iter().any(|item| vec2.contains(item)) + } } diff --git a/src/treewalk/types/object.rs b/src/treewalk/types/object.rs index 3bbce1c..56f041f 100644 --- a/src/treewalk/types/object.rs +++ b/src/treewalk/types/object.rs @@ -75,7 +75,7 @@ impl IndexWrite for Container { index: ExprResult, value: ExprResult, ) -> Result<(), InterpreterError> { - let _ = interpreter.evaluate_method( + let _ = interpreter.invoke_method( ExprResult::Object(self.clone()), &Dunder::SetItem, &resolved_args!(index, value), @@ -89,7 +89,7 @@ impl IndexWrite for Container { interpreter: &Interpreter, index: ExprResult, ) -> Result<(), InterpreterError> { - let _ = interpreter.evaluate_method( + let _ = interpreter.invoke_method( ExprResult::Object(self.clone()), &Dunder::DelItem, &resolved_args!(index), @@ -105,7 +105,7 @@ impl IndexRead for Container { interpreter: &Interpreter, index: ExprResult, ) -> Result, InterpreterError> { - let result = interpreter.evaluate_method( + let result = interpreter.invoke_method( ExprResult::Object(self.clone()), &Dunder::GetItem, &resolved_args!(index), @@ -136,7 +136,11 @@ impl MemberReader for Container { if let Some(attr) = self.borrow().class.get_from_class(name) { log(LogLevel::Debug, || { - format!("Found: {}::{} on class", self.borrow().class, name) + format!( + "Found: {}::{} on class [from object]", + self.borrow().class, + name + ) }); let instance = ExprResult::Object(self.clone()); let owner = instance.get_class(interpreter); @@ -255,7 +259,7 @@ impl NonDataDescriptor for Container { instance: Option, owner: Container, ) -> Result { - interpreter.evaluate_method( + interpreter.invoke_method( ExprResult::Object(self.clone()), &Dunder::Get, &resolved_args!( @@ -280,7 +284,7 @@ impl DataDescriptor for Container { instance: ExprResult, value: ExprResult, ) -> Result<(), InterpreterError> { - interpreter.evaluate_method( + interpreter.invoke_method( ExprResult::Object(self.clone()), &Dunder::Set, &resolved_args!(instance, value), @@ -294,7 +298,7 @@ impl DataDescriptor for Container { interpreter: &Interpreter, instance: ExprResult, ) -> Result<(), InterpreterError> { - interpreter.evaluate_method( + interpreter.invoke_method( ExprResult::Object(self.clone()), &Dunder::Delete, &resolved_args!(instance), @@ -418,7 +422,7 @@ impl Callable for NeBuiltin { interpreter.state.call_stack(), ))?; let result = - interpreter.evaluate_method(receiver, &Dunder::Eq, &resolved_args!(args.get_arg(0)))?; + interpreter.invoke_method(receiver, &Dunder::Eq, &resolved_args!(args.get_arg(0)))?; Ok(result.inverted()) } diff --git a/src/treewalk/types/result.rs b/src/treewalk/types/result.rs index c7e27eb..895862a 100644 --- a/src/treewalk/types/result.rs +++ b/src/treewalk/types/result.rs @@ -196,9 +196,9 @@ impl ExprResult { let mut new_args = arguments.clone(); new_args.bind_new(ExprResult::Class(class.clone())); let object = - interpreter.evaluate_method(ExprResult::Class(class), &Dunder::New, &new_args)?; + interpreter.invoke_method(ExprResult::Class(class), &Dunder::New, &new_args)?; - interpreter.evaluate_method(object.clone(), &Dunder::Init, &arguments)?; + interpreter.invoke_method(object.clone(), &Dunder::Init, &arguments)?; Ok(object) } @@ -206,6 +206,8 @@ impl ExprResult { pub fn hash(&self) -> usize { match self { ExprResult::Object(o) => o.address(), + ExprResult::Class(o) => o.address(), + ExprResult::Integer(i) => *i.borrow() as usize, _ => 0, } } diff --git a/src/treewalk/types/utils/contextual.rs b/src/treewalk/types/utils/contextual.rs index 9944e10..0396b8d 100644 --- a/src/treewalk/types/utils/contextual.rs +++ b/src/treewalk/types/utils/contextual.rs @@ -70,7 +70,39 @@ where impl Contextual { /// Use the interpreter to evaluate equality fn equals(&self, other: &Self) -> bool { - let result = self.interpreter.evaluate_method( + // This isn't technically correct, but the idea here is that we should fallback to the + // default behavior (comparing identity) if we are unable to call Dunder::Eq. We are unable + // to call this because for class objects, this is unbound: complex.__eq__(complex). The + // way Python actually detects this is not whether the method call is unbound or not, but + // whether that method has been overridden (i.e. exists before 'object' in the MRO). + // + // The correct pseudocode is: + // + // if obj1 has an overridden __eq__ method: + // return obj1.__eq__(obj2) + // else: + // return id(obj1) == id(obj2) + // + // Python handles an extra optimization here by comparing identity for objects without an + // overridden __eq__ method, but we just handle that as the fallback implementation of + // object.__eq__ which is discovered via the MRO. + // + // "obj1 has an overridden __eq__ method" will always evaluate to false when obj1 is a + // class, which is what we detect by checking for the unbound case (receiver().is_none()). + let eq = match self + .interpreter + .resolve_method(self.value.clone(), &Dunder::Eq) + { + Err(e) => self + .interpreter + .handle_runtime_error(MemphisError::Interpreter(e)), + Ok(eq) => eq, + }; + if eq.borrow().receiver().is_none() { + return self.value == other.value; + } + + let result = self.interpreter.invoke_method( self.value.clone(), &Dunder::Eq, &resolved_args!(other.value.clone()), @@ -87,9 +119,9 @@ impl Contextual { /// Use the interpreter to evaluate the hash fn hash(&self) -> u64 { - let result = - self.interpreter - .evaluate_method(self.value.clone(), &Dunder::Hash, &resolved_args!()); + let result = self + .interpreter + .call_function("hash", &resolved_args![self.value.clone()]); match result { Ok(ExprResult::Integer(hash_val)) => *hash_val.borrow() as u64, diff --git a/src/treewalk/types/utils/resolved_args.rs b/src/treewalk/types/utils/resolved_args.rs index aacd987..749ee40 100644 --- a/src/treewalk/types/utils/resolved_args.rs +++ b/src/treewalk/types/utils/resolved_args.rs @@ -158,7 +158,8 @@ impl ResolvedArguments { } } -/// This macro is useful when you only need positional arguments. +/// This macro is useful to create `ResolvedArguments` when you only need positional arguments. +/// When kwargs are needed, you can use `ResolvedArguments::new`. #[macro_export] macro_rules! resolved_args { // Double curly braces ensure that the entire macro expands into a single expression, which is