From 5afbfafa8b8f872dd4fc708f1edf34350c4187ed Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Tue, 5 Mar 2019 15:05:52 +0000 Subject: [PATCH 1/4] Remove special case for None payload from boolval. --- tests/snippets/bools.py | 3 +++ vm/src/obj/objbool.rs | 3 +-- vm/src/obj/objnone.rs | 6 ++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/snippets/bools.py b/tests/snippets/bools.py index 2aa817ca4..0c277143b 100644 --- a/tests/snippets/bools.py +++ b/tests/snippets/bools.py @@ -16,6 +16,9 @@ assert bool() == False assert bool(1) == True assert bool({}) == False +assert bool(NotImplemented) == True +assert bool(...) == True + if not 1: raise BaseException diff --git a/vm/src/obj/objbool.rs b/vm/src/obj/objbool.rs index e45018ea6..7a96effd2 100644 --- a/vm/src/obj/objbool.rs +++ b/vm/src/obj/objbool.rs @@ -14,7 +14,7 @@ impl IntoPyObject for bool { } } -pub fn boolval(vm: &mut VirtualMachine, obj: PyObjectRef) -> Result { +pub fn boolval(vm: &mut VirtualMachine, obj: PyObjectRef) -> PyResult { if let Some(s) = obj.payload::() { return Ok(!s.value.is_empty()); } @@ -27,7 +27,6 @@ pub fn boolval(vm: &mut VirtualMachine, obj: PyObjectRef) -> Result !value.is_zero(), PyObjectPayload::Sequence { ref elements } => !elements.borrow().is_empty(), - PyObjectPayload::None { .. } => false, _ => { if let Ok(f) = vm.get_method(obj.clone(), "__bool__") { let bool_res = vm.invoke(f, PyFuncArgs::default())?; diff --git a/vm/src/obj/objnone.rs b/vm/src/obj/objnone.rs index 2aceea915..bc94f9cb9 100644 --- a/vm/src/obj/objnone.rs +++ b/vm/src/obj/objnone.rs @@ -5,6 +5,7 @@ pub fn init(context: &PyContext) { let none_type = &context.none.typ(); context.set_attr(&none_type, "__new__", context.new_rustfunc(none_new)); context.set_attr(&none_type, "__repr__", context.new_rustfunc(none_repr)); + context.set_attr(&none_type, "__bool__", context.new_rustfunc(none_bool)); } fn none_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { @@ -20,3 +21,8 @@ fn none_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { arg_check!(vm, args, required = [(_zelf, Some(vm.ctx.none().typ()))]); Ok(vm.ctx.new_str("None".to_string())) } + +fn none_bool(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { + arg_check!(vm, args, required = [(_zelf, Some(vm.ctx.none().typ()))]); + Ok(vm.ctx.new_bool(false)) +} From a72af4d9674c0f7a4f6c74bb3f25c75a66d7d202 Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Tue, 5 Mar 2019 15:50:07 +0000 Subject: [PATCH 2/4] Remove None and NotImplemented payloads and replace with NoPayload --- vm/src/frame.rs | 33 +++++++++++++++++---------------- vm/src/pyobject.rs | 14 ++++++-------- vm/src/stdlib/json.rs | 4 ++-- vm/src/vm.rs | 10 ++++++---- 4 files changed, 31 insertions(+), 30 deletions(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 977759607..f5dc38ff0 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -286,10 +286,14 @@ impl Frame { let mut out: Vec> = elements .into_iter() - .map(|x| match x.payload { - PyObjectPayload::Integer { ref value } => Some(value.clone()), - PyObjectPayload::None => None, - _ => panic!("Expect Int or None as BUILD_SLICE arguments, got {:?}", x), + .map(|x| { + if x.is(&vm.get_none()) { + None + } else if let PyObjectPayload::Integer { ref value } = x.payload { + Some(value.clone()) + } else { + panic!("Expect Int or None as BUILD_SLICE arguments, got {:?}", x); + } }) .collect(); @@ -575,18 +579,15 @@ impl Frame { } bytecode::Instruction::PrintExpr => { let expr = self.pop_value(); - match expr.payload { - PyObjectPayload::None => (), - _ => { - let repr = vm.to_repr(&expr)?; - builtins::builtin_print( - vm, - PyFuncArgs { - args: vec![repr], - kwargs: vec![], - }, - )?; - } + if !expr.is(&vm.get_none()) { + let repr = vm.to_repr(&expr)?; + builtins::builtin_print( + vm, + PyFuncArgs { + args: vec![repr], + kwargs: vec![], + }, + )?; } Ok(None) } diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 6e0ab2701..f644e1698 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -155,7 +155,7 @@ pub struct PyContext { fn _nothing() -> PyObjectRef { PyObject { - payload: PyObjectPayload::None, + payload: PyObjectPayload::NoPayload, typ: None, } .into_ref() @@ -223,14 +223,14 @@ impl PyContext { let exceptions = exceptions::ExceptionZoo::new(&type_type, &object_type, &dict_type); let none = PyObject::new( - PyObjectPayload::None, + PyObjectPayload::NoPayload, create_type("NoneType", &type_type, &object_type, &dict_type), ); - let ellipsis = PyObject::new(PyObjectPayload::None, ellipsis_type.clone()); + let ellipsis = PyObject::new(PyObjectPayload::NoPayload, ellipsis_type.clone()); let not_implemented = PyObject::new( - PyObjectPayload::NotImplemented, + PyObjectPayload::NoPayload, create_type("NotImplementedType", &type_type, &object_type, &dict_type), ); @@ -1484,8 +1484,7 @@ pub enum PyObjectPayload { name: String, scope: ScopeRef, }, - None, - NotImplemented, + NoPayload, Class { name: String, dict: RefCell, @@ -1526,8 +1525,7 @@ impl fmt::Debug for PyObjectPayload { ref object, } => write!(f, "bound-method: {:?} of {:?}", function, object), PyObjectPayload::Module { .. } => write!(f, "module"), - PyObjectPayload::None => write!(f, "None"), - PyObjectPayload::NotImplemented => write!(f, "NotImplemented"), + PyObjectPayload::NoPayload => write!(f, "NoPayload"), PyObjectPayload::Class { ref name, .. } => write!(f, "class {:?}", name), PyObjectPayload::Instance { .. } => write!(f, "instance"), PyObjectPayload::RustFunction { .. } => write!(f, "rust function"), diff --git a/vm/src/stdlib/json.rs b/vm/src/stdlib/json.rs index 7403c8d8c..0d0bc23dc 100644 --- a/vm/src/stdlib/json.rs +++ b/vm/src/stdlib/json.rs @@ -11,7 +11,7 @@ use crate::obj::{ objtype, }; use crate::pyobject::{ - create_type, DictProtocol, PyContext, PyFuncArgs, PyObjectPayload, PyObjectRef, PyResult, + create_type, DictProtocol, IdProtocol, PyContext, PyFuncArgs, PyObjectRef, PyResult, TypeProtocol, }; use crate::VirtualMachine; @@ -69,7 +69,7 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> { map.serialize_entry(&key, &self.clone_with_object(&e.1))?; } map.end() - } else if let PyObjectPayload::None = self.pyobject.payload { + } else if self.pyobject.is(&self.vm.get_none()) { serializer.serialize_none() } else { Err(serde::ser::Error::custom(format!( diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 6c6659d4d..1d4aab0a1 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -473,10 +473,12 @@ impl VirtualMachine { // Add missing positional arguments, if we have fewer positional arguments than the // function definition calls for if nargs < nexpected_args { - let available_defaults = match defaults.payload { - PyObjectPayload::Sequence { ref elements } => elements.borrow().clone(), - PyObjectPayload::None => vec![], - _ => panic!("function defaults not tuple or None"), + let available_defaults = if defaults.is(&self.get_none()) { + vec![] + } else if let PyObjectPayload::Sequence { ref elements } = defaults.payload { + elements.borrow().clone() + } else { + panic!("function defaults not tuple or None"); }; // Given the number of defaults available, check all the arguments for which we From e0c398cbd5eb67087bae5a8d627d42c923d97d44 Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Wed, 6 Mar 2019 20:57:03 +0000 Subject: [PATCH 3/4] Eliminate NoPayload and just use a boxed (). --- vm/src/pyobject.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index f644e1698..0e8d31e65 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -155,7 +155,9 @@ pub struct PyContext { fn _nothing() -> PyObjectRef { PyObject { - payload: PyObjectPayload::NoPayload, + payload: PyObjectPayload::AnyRustValue { + value: Box::new(()), + }, typ: None, } .into_ref() @@ -223,14 +225,23 @@ impl PyContext { let exceptions = exceptions::ExceptionZoo::new(&type_type, &object_type, &dict_type); let none = PyObject::new( - PyObjectPayload::NoPayload, + PyObjectPayload::AnyRustValue { + value: Box::new(()), + }, create_type("NoneType", &type_type, &object_type, &dict_type), ); - let ellipsis = PyObject::new(PyObjectPayload::NoPayload, ellipsis_type.clone()); + let ellipsis = PyObject::new( + PyObjectPayload::AnyRustValue { + value: Box::new(()), + }, + ellipsis_type.clone(), + ); let not_implemented = PyObject::new( - PyObjectPayload::NoPayload, + PyObjectPayload::AnyRustValue { + value: Box::new(()), + }, create_type("NotImplementedType", &type_type, &object_type, &dict_type), ); @@ -1484,7 +1495,6 @@ pub enum PyObjectPayload { name: String, scope: ScopeRef, }, - NoPayload, Class { name: String, dict: RefCell, @@ -1525,7 +1535,6 @@ impl fmt::Debug for PyObjectPayload { ref object, } => write!(f, "bound-method: {:?} of {:?}", function, object), PyObjectPayload::Module { .. } => write!(f, "module"), - PyObjectPayload::NoPayload => write!(f, "NoPayload"), PyObjectPayload::Class { ref name, .. } => write!(f, "class {:?}", name), PyObjectPayload::Instance { .. } => write!(f, "instance"), PyObjectPayload::RustFunction { .. } => write!(f, "rust function"), From 4a000c424c66c27c4f6318622de2817abb82da23 Mon Sep 17 00:00:00 2001 From: Joey Date: Wed, 6 Mar 2019 19:53:03 -0800 Subject: [PATCH 4/4] Improvements to OptionalArg extractor --- vm/src/obj/objstr.rs | 20 ++++++++++---------- vm/src/pyobject.rs | 29 +++++++++++++++++++---------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 4ee7f38f5..6d9d622d0 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -4,8 +4,8 @@ use super::objtype; use crate::format::{FormatParseError, FormatPart, FormatString}; use crate::function::PyRef; use crate::pyobject::{ - IntoPyObject, OptArg, PyContext, PyFuncArgs, PyIterable, PyObjectPayload, PyObjectPayload2, - PyObjectRef, PyResult, TypeProtocol, + IntoPyObject, OptionalArg, PyContext, PyFuncArgs, PyIterable, PyObjectPayload, + PyObjectPayload2, PyObjectRef, PyResult, TypeProtocol, }; use crate::vm::VirtualMachine; use num_traits::ToPrimitive; @@ -160,24 +160,24 @@ impl PyStringRef { fn endswith( self, suffix: PyStringRef, - start: OptArg, - end: OptArg, + start: OptionalArg, + end: OptionalArg, _vm: &mut VirtualMachine, ) -> bool { - let start = start.unwrap_or(0); - let end = end.unwrap_or(self.value.len()); + let start = start.into_option().unwrap_or(0); + let end = end.into_option().unwrap_or(self.value.len()); self.value[start..end].ends_with(&suffix.value) } fn startswith( self, prefix: PyStringRef, - start: OptArg, - end: OptArg, + start: OptionalArg, + end: OptionalArg, _vm: &mut VirtualMachine, ) -> bool { - let start = start.unwrap_or(0); - let end = end.unwrap_or(self.value.len()); + let start = start.into_option().unwrap_or(0); + let end = end.into_option().unwrap_or(self.value.len()); self.value[start..end].starts_with(&prefix.value) } diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 6a618cd7b..925c56481 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -1212,17 +1212,26 @@ where } } -pub struct OptArg(Option); +/// An argument that may or may not be provided by the caller. +/// +/// This style of argument is not possible in pure Python. +pub enum OptionalArg { + Present(T), + Missing, +} -impl std::ops::Deref for OptArg { - type Target = Option; +use self::OptionalArg::*; - fn deref(&self) -> &Option { - &self.0 +impl OptionalArg { + pub fn into_option(self) -> Option { + match self { + Present(value) => Some(value), + Missing => None, + } } } -impl FromArgs for OptArg +impl FromArgs for OptionalArg where T: TryFromObject, { @@ -1237,16 +1246,16 @@ where where I: Iterator, { - Ok(OptArg(if let Some(PyArg::Positional(_)) = args.peek() { + Ok(if let Some(PyArg::Positional(_)) = args.peek() { let value = if let Some(PyArg::Positional(value)) = args.next() { value } else { unreachable!() }; - Some(T::try_from_object(vm, value)?) + Present(T::try_from_object(vm, value)?) } else { - None - })) + Missing + }) } }