From b43c51154222e376be658032e2de1f3bdc90d085 Mon Sep 17 00:00:00 2001 From: silmeth Date: Tue, 5 Feb 2019 19:49:14 +0100 Subject: [PATCH 1/7] fix range len() for negative and non-divisible steps --- vm/src/obj/objrange.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/vm/src/obj/objrange.rs b/vm/src/obj/objrange.rs index 5924cd048..7e25fd058 100644 --- a/vm/src/obj/objrange.rs +++ b/vm/src/obj/objrange.rs @@ -4,7 +4,7 @@ use super::super::pyobject::{ use super::super::vm::VirtualMachine; use super::objint; use super::objtype; -use num_bigint::{BigInt, ToBigInt}; +use num_bigint::{BigInt, ToBigInt, Sign}; use num_traits::{One, Signed, ToPrimitive, Zero}; #[derive(Debug, Clone)] @@ -19,10 +19,13 @@ pub struct RangeType { impl RangeType { #[inline] pub fn len(&self) -> usize { - ((self.end.clone() - self.start.clone()) / self.step.clone()) - .abs() - .to_usize() - .unwrap() + match self.step.sign() { + Sign::Plus if self.start < self.end => + ((&self.end - &self.start - 1usize) / &self.step).to_usize().unwrap() + 1, + Sign::Minus if self.start > self.end => + ((&self.start - &self.end - 1usize) / (-&self.step)).to_usize().unwrap() + 1, + _ => 0, + } } #[inline] From 0a1eb4b91bb4ea1f796189b2a70cb8711d345b23 Mon Sep 17 00:00:00 2001 From: silmeth Date: Tue, 5 Feb 2019 20:58:38 +0100 Subject: [PATCH 2/7] make len(range) throw OverflowError on too big ints + impl __repr__ --- vm/src/exceptions.rs | 14 +++++++++++++ vm/src/obj/objrange.rs | 46 +++++++++++++++++++++++++++++++++--------- vm/src/vm.rs | 9 +++++++-- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs index 305297919..63e17e7c6 100644 --- a/vm/src/exceptions.rs +++ b/vm/src/exceptions.rs @@ -81,6 +81,7 @@ fn exception_str(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { #[derive(Debug)] pub struct ExceptionZoo { + pub arithmetic_error: PyObjectRef, pub assertion_error: PyObjectRef, pub attribute_error: PyObjectRef, pub base_exception_type: PyObjectRef, @@ -93,12 +94,14 @@ pub struct ExceptionZoo { pub name_error: PyObjectRef, pub not_implemented_error: PyObjectRef, pub os_error: PyObjectRef, + pub overflow_error: PyObjectRef, pub permission_error: PyObjectRef, pub runtime_error: PyObjectRef, pub stop_iteration: PyObjectRef, pub syntax_error: PyObjectRef, pub type_error: PyObjectRef, pub value_error: PyObjectRef, + pub zero_division_error: PyObjectRef, } impl ExceptionZoo { @@ -113,6 +116,8 @@ impl ExceptionZoo { let exception_type = create_type("Exception", &type_type, &base_exception_type, &dict_type); + let arithmetic_error = + create_type("ArithmeticError", &type_type, &exception_type, &dict_type); let assertion_error = create_type("AssertionError", &type_type, &exception_type, &dict_type); let attribute_error = @@ -128,8 +133,14 @@ impl ExceptionZoo { let type_error = create_type("TypeError", &type_type, &exception_type, &dict_type); let value_error = create_type("ValueError", &type_type, &exception_type, &dict_type); + let overflow_error = + create_type("OverflowError", &type_type, &arithmetic_error, &dict_type); + let zero_division_error = + create_type("ZeroDivisionError", &type_type, &arithmetic_error, &dict_type); + let module_not_found_error = create_type("ModuleNotFoundError", &type_type, &import_error, &dict_type); + let not_implemented_error = create_type( "NotImplementedError", &type_type, @@ -142,6 +153,7 @@ impl ExceptionZoo { let permission_error = create_type("PermissionError", &type_type, &os_error, &dict_type); ExceptionZoo { + arithmetic_error, assertion_error, attribute_error, base_exception_type, @@ -154,12 +166,14 @@ impl ExceptionZoo { name_error, not_implemented_error, os_error, + overflow_error, permission_error, runtime_error, stop_iteration, syntax_error, type_error, value_error, + zero_division_error, } } } diff --git a/vm/src/obj/objrange.rs b/vm/src/obj/objrange.rs index 7e25fd058..c79a26492 100644 --- a/vm/src/obj/objrange.rs +++ b/vm/src/obj/objrange.rs @@ -18,16 +18,21 @@ pub struct RangeType { impl RangeType { #[inline] - pub fn len(&self) -> usize { + pub fn try_len(&self) -> Option { match self.step.sign() { Sign::Plus if self.start < self.end => - ((&self.end - &self.start - 1usize) / &self.step).to_usize().unwrap() + 1, + ((&self.end - &self.start - 1usize) / &self.step).to_usize().map(|sz| sz + 1), Sign::Minus if self.start > self.end => - ((&self.start - &self.end - 1usize) / (-&self.step)).to_usize().unwrap() + 1, - _ => 0, + ((&self.start - &self.end - 1usize) / (-&self.step)).to_usize().map(|sz| sz + 1), + _ => Some(0), } } + #[inline] + pub fn len(&self) -> usize { + self.try_len().unwrap() + } + #[inline] pub fn is_empty(&self) -> bool { (self.start <= self.end && self.step.is_negative()) @@ -51,6 +56,15 @@ impl RangeType { None } } + + #[inline] + pub fn repr(&self) -> String { + if self.step == BigInt::one() { + format!("range({}, {})", self.start, self.end) + } else { + format!("range({}, {}, {})", self.start, self.end, self.step) + } + } } pub fn init(context: &PyContext) { @@ -63,6 +77,7 @@ pub fn init(context: &PyContext) { "__getitem__", context.new_rustfunc(range_getitem), ); + context.set_attr(&range_type, "__repr__", context.new_rustfunc(range_repr)); } fn range_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { @@ -121,12 +136,14 @@ fn range_iter(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { fn range_len(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { arg_check!(vm, args, required = [(zelf, Some(vm.ctx.range_type()))]); - let len = match zelf.borrow().payload { - PyObjectPayload::Range { ref range } => range.len(), + if let Some(len) = match zelf.borrow().payload { + PyObjectPayload::Range { ref range } => range.try_len(), _ => unreachable!(), - }; - - Ok(vm.ctx.new_int(len.to_bigint().unwrap())) + } { + Ok(vm.ctx.new_int(len.to_bigint().unwrap())) + } else { + Err(vm.new_overflow_error("Python int too large to convert to Rust usize".to_string())) + } } fn range_getitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { @@ -196,3 +213,14 @@ fn range_getitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { _ => Err(vm.new_type_error("range indices must be integer or slice".to_string())), } } + +fn range_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { + arg_check!(vm, args, required = [(zelf, Some(vm.ctx.range_type()))]); + + let s = match zelf.borrow().payload { + PyObjectPayload::Range { ref range } => range.repr(), + _ => unreachable!(), + }; + + Ok(vm.ctx.new_str(s)) +} diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 061c99605..56004271f 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -105,6 +105,11 @@ impl VirtualMachine { self.new_exception(os_error, msg) } + pub fn new_overflow_error(&mut self, msg: String) -> PyObjectRef { + let overflow_error = self.ctx.exceptions.overflow_error.clone(); + self.new_exception(overflow_error, msg) + } + /// Create a new python ValueError object. Useful for raising errors from /// python functions implemented in rust. pub fn new_value_error(&mut self, msg: String) -> PyObjectRef { @@ -123,8 +128,8 @@ impl VirtualMachine { } pub fn new_not_implemented_error(&mut self, msg: String) -> PyObjectRef { - let value_error = self.ctx.exceptions.not_implemented_error.clone(); - self.new_exception(value_error, msg) + let not_implemented_error = self.ctx.exceptions.not_implemented_error.clone(); + self.new_exception(not_implemented_error, msg) } pub fn new_scope(&mut self, parent_scope: Option) -> PyObjectRef { From d96e0ecf40bfda9288bf30a3f1be1b66250147c5 Mon Sep 17 00:00:00 2001 From: silmeth Date: Tue, 5 Feb 2019 21:28:48 +0100 Subject: [PATCH 3/7] make division and modulo by 0 throw ZeroDivisionError --- vm/src/obj/objint.rs | 32 +++++++++++++++++++++++--------- vm/src/vm.rs | 5 +++++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/vm/src/obj/objint.rs b/vm/src/obj/objint.rs index 083741023..e0e275dce 100644 --- a/vm/src/obj/objint.rs +++ b/vm/src/obj/objint.rs @@ -370,17 +370,25 @@ fn int_truediv(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { args, required = [(i, Some(vm.ctx.int_type())), (i2, None)] ); - let v1 = get_value(i); - if objtype::isinstance(i2, &vm.ctx.int_type()) { - Ok(vm - .ctx - .new_float(v1.to_f64().unwrap() / get_value(i2).to_f64().unwrap())) + + let v1 = get_value(i).to_f64() + .ok_or_else(|| vm.new_overflow_error("int too large to convert to float".to_string()))?; + + let v2 = if objtype::isinstance(i2, &vm.ctx.int_type()) { + get_value(i2).to_f64() + .ok_or_else(|| vm.new_overflow_error("int too large to convert to float".to_string()))? } else if objtype::isinstance(i2, &vm.ctx.float_type()) { + objfloat::get_value(i2) + } else { + return Err(vm.new_type_error(format!("Cannot divide {} and {}", i.borrow(), i2.borrow()))); + }; + + if v2 == 0.0 { + Err(vm.new_zero_division_error("integer division by zero".to_string())) + } else { Ok(vm .ctx - .new_float(v1.to_f64().unwrap() / objfloat::get_value(i2))) - } else { - Err(vm.new_type_error(format!("Cannot divide {} and {}", i.borrow(), i2.borrow()))) + .new_float(v1 / v2)) } } @@ -392,7 +400,13 @@ fn int_mod(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { ); let v1 = get_value(i); if objtype::isinstance(i2, &vm.ctx.int_type()) { - Ok(vm.ctx.new_int(v1 % get_value(i2))) + let v2 = get_value(i2); + + if v2 != BigInt::zero() { + Ok(vm.ctx.new_int(v1 % get_value(i2))) + } else { + Err(vm.new_zero_division_error("integer modulo by zero".to_string())) + } } else { Err(vm.new_type_error(format!("Cannot modulo {} and {}", i.borrow(), i2.borrow()))) } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 56004271f..3d56818c6 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -132,6 +132,11 @@ impl VirtualMachine { self.new_exception(not_implemented_error, msg) } + pub fn new_zero_division_error(&mut self, msg: String) -> PyObjectRef { + let zero_division_error = self.ctx.exceptions.zero_division_error.clone(); + self.new_exception(zero_division_error, msg) + } + pub fn new_scope(&mut self, parent_scope: Option) -> PyObjectRef { // let parent_scope = self.current_frame_mut().locals.clone(); self.ctx.new_scope(parent_scope) From 30c8e477e45a8d333556c64d03a9397ac024482c Mon Sep 17 00:00:00 2001 From: silmeth Date: Tue, 5 Feb 2019 21:54:03 +0100 Subject: [PATCH 4/7] style: rustfmt --- vm/src/exceptions.rs | 8 ++++++-- vm/src/obj/objint.rs | 10 +++++----- vm/src/obj/objrange.rs | 16 ++++++++++------ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs index 63e17e7c6..be3652a73 100644 --- a/vm/src/exceptions.rs +++ b/vm/src/exceptions.rs @@ -135,8 +135,12 @@ impl ExceptionZoo { let overflow_error = create_type("OverflowError", &type_type, &arithmetic_error, &dict_type); - let zero_division_error = - create_type("ZeroDivisionError", &type_type, &arithmetic_error, &dict_type); + let zero_division_error = create_type( + "ZeroDivisionError", + &type_type, + &arithmetic_error, + &dict_type, + ); let module_not_found_error = create_type("ModuleNotFoundError", &type_type, &import_error, &dict_type); diff --git a/vm/src/obj/objint.rs b/vm/src/obj/objint.rs index e0e275dce..c31709595 100644 --- a/vm/src/obj/objint.rs +++ b/vm/src/obj/objint.rs @@ -371,11 +371,13 @@ fn int_truediv(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { required = [(i, Some(vm.ctx.int_type())), (i2, None)] ); - let v1 = get_value(i).to_f64() + let v1 = get_value(i) + .to_f64() .ok_or_else(|| vm.new_overflow_error("int too large to convert to float".to_string()))?; let v2 = if objtype::isinstance(i2, &vm.ctx.int_type()) { - get_value(i2).to_f64() + get_value(i2) + .to_f64() .ok_or_else(|| vm.new_overflow_error("int too large to convert to float".to_string()))? } else if objtype::isinstance(i2, &vm.ctx.float_type()) { objfloat::get_value(i2) @@ -386,9 +388,7 @@ fn int_truediv(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { if v2 == 0.0 { Err(vm.new_zero_division_error("integer division by zero".to_string())) } else { - Ok(vm - .ctx - .new_float(v1 / v2)) + Ok(vm.ctx.new_float(v1 / v2)) } } diff --git a/vm/src/obj/objrange.rs b/vm/src/obj/objrange.rs index c79a26492..cd255b29a 100644 --- a/vm/src/obj/objrange.rs +++ b/vm/src/obj/objrange.rs @@ -4,7 +4,7 @@ use super::super::pyobject::{ use super::super::vm::VirtualMachine; use super::objint; use super::objtype; -use num_bigint::{BigInt, ToBigInt, Sign}; +use num_bigint::{BigInt, Sign, ToBigInt}; use num_traits::{One, Signed, ToPrimitive, Zero}; #[derive(Debug, Clone)] @@ -20,10 +20,14 @@ impl RangeType { #[inline] pub fn try_len(&self) -> Option { match self.step.sign() { - Sign::Plus if self.start < self.end => - ((&self.end - &self.start - 1usize) / &self.step).to_usize().map(|sz| sz + 1), - Sign::Minus if self.start > self.end => - ((&self.start - &self.end - 1usize) / (-&self.step)).to_usize().map(|sz| sz + 1), + Sign::Plus if self.start < self.end => ((&self.end - &self.start - 1usize) + / &self.step) + .to_usize() + .map(|sz| sz + 1), + Sign::Minus if self.start > self.end => ((&self.start - &self.end - 1usize) + / (-&self.step)) + .to_usize() + .map(|sz| sz + 1), _ => Some(0), } } @@ -56,7 +60,7 @@ impl RangeType { None } } - + #[inline] pub fn repr(&self) -> String { if self.step == BigInt::one() { From 8621f3ff2b24ec29ee14d3489c4ad79d85af1960 Mon Sep 17 00:00:00 2001 From: silmeth Date: Tue, 5 Feb 2019 22:48:33 +0100 Subject: [PATCH 5/7] fix: register arithmetic errors in builtins module --- vm/src/builtins.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs index 8b8c8690b..fb1d84c32 100644 --- a/vm/src/builtins.rs +++ b/vm/src/builtins.rs @@ -794,6 +794,11 @@ pub fn make_module(ctx: &PyContext) -> PyObjectRef { ctx.exceptions.base_exception_type.clone(), ); ctx.set_attr(&py_mod, "Exception", ctx.exceptions.exception_type.clone()); + ctx.set_attr( + &py_mod, + "ArithmeticError", + ctx.exceptions.arithmetic_error.clone(), + ); ctx.set_attr( &py_mod, "AssertionError", @@ -805,6 +810,11 @@ pub fn make_module(ctx: &PyContext) -> PyObjectRef { ctx.exceptions.attribute_error.clone(), ); ctx.set_attr(&py_mod, "NameError", ctx.exceptions.name_error.clone()); + ctx.set_attr( + &py_mod, + "OverflowError", + ctx.exceptions.overflow_error.clone(), + ); ctx.set_attr( &py_mod, "RuntimeError", @@ -819,6 +829,11 @@ pub fn make_module(ctx: &PyContext) -> PyObjectRef { ctx.set_attr(&py_mod, "ValueError", ctx.exceptions.value_error.clone()); ctx.set_attr(&py_mod, "IndexError", ctx.exceptions.index_error.clone()); ctx.set_attr(&py_mod, "ImportError", ctx.exceptions.import_error.clone()); + ctx.set_attr( + &py_mod, + "ZeroDivisionError", + ctx.exceptions.zero_division_error.clone(), + ); py_mod } From 77ae6621e91dbb1a6f572cc1400165bda3ed1fd0 Mon Sep 17 00:00:00 2001 From: silmeth Date: Tue, 5 Feb 2019 22:50:52 +0100 Subject: [PATCH 6/7] add tests for range length, and for division by zero --- tests/snippets/builtin_range.py | 4 ++++ tests/snippets/division_by_zero.py | 34 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/snippets/division_by_zero.py diff --git a/tests/snippets/builtin_range.py b/tests/snippets/builtin_range.py index 3284fa2b4..d1df0871b 100644 --- a/tests/snippets/builtin_range.py +++ b/tests/snippets/builtin_range.py @@ -1 +1,5 @@ assert range(2**63+1)[2**63] == 9223372036854775808 + +assert len(range(10, 5)) == 0, 'Range with no elements should have length = 0' +assert len(range(10, 5, -2)) == 3, 'Expected length 3, for elements: 10, 8, 6' +assert len(range(5, 10, 2)) == 3, 'Expected length 3, for elements: 5, 7, 9' diff --git a/tests/snippets/division_by_zero.py b/tests/snippets/division_by_zero.py new file mode 100644 index 000000000..b4b036c84 --- /dev/null +++ b/tests/snippets/division_by_zero.py @@ -0,0 +1,34 @@ +try: + 5 / 0 +except ZeroDivisionError: + pass +except: + assert False, 'Expected ZeroDivisionError' + +try: + 5 / -0.0 +except ZeroDivisionError: + pass +except: + assert False, 'Expected ZeroDivisionError' + +try: + 5 / (3-2) +except ZeroDivisionError: + pass +except: + assert False, 'Expected ZeroDivisionError' + +try: + 5 % 0 +except ZeroDivisionError: + pass +except: + assert False, 'Expected ZeroDivisionError' + +try: + raise ZeroDivisionError('Is an ArithmeticError subclass?') +except ArithmeticError: + pass +except: + assert False, 'Expected ZeroDivisionError to be a subclass of ArithmeticError' From af0fdcb9e33bde89d83275311c3989431ccfbafd Mon Sep 17 00:00:00 2001 From: silmeth Date: Wed, 6 Feb 2019 10:10:20 +0100 Subject: [PATCH 7/7] fix zero-division tests --- tests/snippets/division_by_zero.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/snippets/division_by_zero.py b/tests/snippets/division_by_zero.py index b4b036c84..7cb68cd76 100644 --- a/tests/snippets/division_by_zero.py +++ b/tests/snippets/division_by_zero.py @@ -2,33 +2,33 @@ try: 5 / 0 except ZeroDivisionError: pass -except: +else: assert False, 'Expected ZeroDivisionError' try: 5 / -0.0 except ZeroDivisionError: pass -except: +else: assert False, 'Expected ZeroDivisionError' try: - 5 / (3-2) + 5 / (2-2) except ZeroDivisionError: pass -except: +else: assert False, 'Expected ZeroDivisionError' try: 5 % 0 except ZeroDivisionError: pass -except: +else: assert False, 'Expected ZeroDivisionError' try: raise ZeroDivisionError('Is an ArithmeticError subclass?') except ArithmeticError: pass -except: - assert False, 'Expected ZeroDivisionError to be a subclass of ArithmeticError' +else: + assert False, 'Expected ZeroDivisionError'