From b7a4248837728562d40760ca85ad102ffce13914 Mon Sep 17 00:00:00 2001 From: Nic Ficca Date: Thu, 7 Feb 2019 19:28:55 -0500 Subject: [PATCH 1/4] Add range.count --- vm/src/obj/objrange.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/vm/src/obj/objrange.rs b/vm/src/obj/objrange.rs index 221e98da5..2b6f79617 100644 --- a/vm/src/obj/objrange.rs +++ b/vm/src/obj/objrange.rs @@ -66,6 +66,14 @@ impl RangeType { } } + #[inline] + pub fn count(&self, value: &BigInt) -> usize { + match self.index_of(value).is_some() { + true => 1, + false => 0, + } + } + #[inline] pub fn is_empty(&self) -> bool { (self.start <= self.end && self.step.is_negative()) @@ -156,6 +164,7 @@ pub fn init(context: &PyContext) { context.new_rustfunc(range_contains), ); context.set_attr(&range_type, "index", context.new_rustfunc(range_index)); + context.set_attr(&range_type, "count", context.new_rustfunc(range_count)); } fn range_new(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { @@ -366,3 +375,20 @@ fn range_index(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { unreachable!() } } + +fn range_count(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { + arg_check!( + vm, + args, + required = [(zelf, Some(vm.ctx.range_type())), (needle, None)] + ); + + if let PyObjectPayload::Range { ref range } = zelf.borrow().payload { + match needle.borrow().payload { + PyObjectPayload::Integer { ref value } => Ok(vm.ctx.new_int(range.count(value))), + _ => Ok(vm.ctx.new_int(0)), + } + } else { + unreachable!() + } +} From 316e94bd42f7c550de385b9619cc2157fe207f60 Mon Sep 17 00:00:00 2001 From: Nic Ficca Date: Thu, 7 Feb 2019 20:04:53 -0500 Subject: [PATCH 2/4] Add tests --- tests/snippets/builtin_range.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/snippets/builtin_range.py b/tests/snippets/builtin_range.py index 667e776a2..4c186d8a3 100644 --- a/tests/snippets/builtin_range.py +++ b/tests/snippets/builtin_range.py @@ -39,6 +39,15 @@ assert_raises(lambda _: range(4, 10, 2).index(5), ValueError) # index raises value error if needle is not an int assert_raises(lambda _: range(10).index('foo'), ValueError) +# count tests +assert range(10).count(2) == 1 +assert range(10).count(11) == 0 +assert range(10).count(-1) == 0 +assert range(9, 12).count(10) == 1 +assert range(4, 10, 2).count(4) == 1 +assert range(4, 10, 2).count(7) == 0 +assert range(10).count("foo") == 0 + # __bool__ assert bool(range(1)) assert bool(range(1, 2)) From 2e9892029a00a43f3d80cbb19b12a5d0ddcbf33f Mon Sep 17 00:00:00 2001 From: Nic Ficca Date: Sat, 9 Feb 2019 16:49:44 -0500 Subject: [PATCH 3/4] Add objrange::get_value - Refactor range_count to use get_value and isinstance for pulling out the range and argument respectively --- vm/src/obj/objrange.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/vm/src/obj/objrange.rs b/vm/src/obj/objrange.rs index 2b6f79617..23c572589 100644 --- a/vm/src/obj/objrange.rs +++ b/vm/src/obj/objrange.rs @@ -127,6 +127,14 @@ impl RangeType { } } +pub fn get_value(obj: &PyObjectRef) -> RangeType { + if let PyObjectPayload::Range { range } = &obj.borrow().payload { + range.clone() + } else { + panic!("Inner error getting range {:?}", obj); + } +} + pub fn init(context: &PyContext) { let ref range_type = context.range_type; @@ -380,15 +388,14 @@ fn range_count(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { arg_check!( vm, args, - required = [(zelf, Some(vm.ctx.range_type())), (needle, None)] + required = [(zelf, Some(vm.ctx.range_type())), (item, None)] ); - if let PyObjectPayload::Range { ref range } = zelf.borrow().payload { - match needle.borrow().payload { - PyObjectPayload::Integer { ref value } => Ok(vm.ctx.new_int(range.count(value))), - _ => Ok(vm.ctx.new_int(0)), - } + let range = get_value(zelf); + + if objtype::isinstance(item, &vm.ctx.int_type()) { + Ok(vm.ctx.new_int(range.count(&objint::get_value(item)))) } else { - unreachable!() + Ok(vm.ctx.new_int(0)) } } From 0cb661f3a26a0f6705f2ca34efeb08212380d5cf Mon Sep 17 00:00:00 2001 From: Nic Ficca Date: Sat, 9 Feb 2019 16:54:14 -0500 Subject: [PATCH 4/4] Dry up objrange - Use get_value in all appropriate spots --- vm/src/obj/objrange.rs | 79 ++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 46 deletions(-) diff --git a/vm/src/obj/objrange.rs b/vm/src/obj/objrange.rs index 23c572589..21f17f670 100644 --- a/vm/src/obj/objrange.rs +++ b/vm/src/obj/objrange.rs @@ -231,10 +231,7 @@ fn range_iter(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { fn range_reversed(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { arg_check!(vm, args, required = [(zelf, Some(vm.ctx.range_type()))]); - let range = match zelf.borrow().payload { - PyObjectPayload::Range { ref range } => range.reversed(), - _ => unreachable!(), - }; + let range = get_value(zelf).reversed(); Ok(PyObject::new( PyObjectPayload::Iterator { @@ -248,10 +245,7 @@ fn range_reversed(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()))]); - if let Some(len) = match zelf.borrow().payload { - PyObjectPayload::Range { ref range } => range.try_len(), - _ => unreachable!(), - } { + if let Some(len) = get_value(zelf).try_len() { Ok(vm.ctx.new_int(len)) } else { Err(vm.new_overflow_error("Python int too large to convert to Rust usize".to_string())) @@ -264,15 +258,12 @@ fn range_getitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { args, required = [(zelf, Some(vm.ctx.range_type())), (subscript, None)] ); - let zrange = if let PyObjectPayload::Range { ref range } = zelf.borrow().payload { - range.clone() - } else { - unreachable!() - }; + + let range = get_value(zelf); match subscript.borrow().payload { PyObjectPayload::Integer { ref value } => { - if let Some(int) = zrange.get(value) { + if let Some(int) = range.get(value) { Ok(vm.ctx.new_int(int)) } else { Err(vm.new_index_error("range object index out of range".to_string())) @@ -284,29 +275,29 @@ fn range_getitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { ref step, } => { let new_start = if let Some(int) = start { - if let Some(i) = zrange.get(int) { + if let Some(i) = range.get(int) { i } else { - zrange.start.clone() + range.start.clone() } } else { - zrange.start.clone() + range.start.clone() }; let new_end = if let Some(int) = stop { - if let Some(i) = zrange.get(int) { + if let Some(i) = range.get(int) { i } else { - zrange.end + range.end } } else { - zrange.end + range.end }; let new_step = if let Some(int) = step { - int * zrange.step + int * range.step } else { - zrange.step + range.step }; Ok(PyObject::new( @@ -328,21 +319,15 @@ fn range_getitem(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { 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!(), - }; + let repr = get_value(zelf).repr(); - Ok(vm.ctx.new_str(s)) + Ok(vm.ctx.new_str(repr)) } fn range_bool(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(), - _ => unreachable!(), - }; + let len = get_value(zelf).len(); Ok(vm.ctx.new_bool(len > 0)) } @@ -354,14 +339,15 @@ fn range_contains(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { required = [(zelf, Some(vm.ctx.range_type())), (needle, None)] ); - if let PyObjectPayload::Range { ref range } = zelf.borrow().payload { - Ok(vm.ctx.new_bool(match needle.borrow().payload { - PyObjectPayload::Integer { ref value } => range.contains(value), - _ => false, - })) + let range = get_value(zelf); + + let result = if objtype::isinstance(needle, &vm.ctx.int_type()) { + range.contains(&objint::get_value(needle)) } else { - unreachable!() - } + false + }; + + Ok(vm.ctx.new_bool(result)) } fn range_index(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { @@ -371,16 +357,17 @@ fn range_index(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult { required = [(zelf, Some(vm.ctx.range_type())), (needle, None)] ); - if let PyObjectPayload::Range { ref range } = zelf.borrow().payload { - match needle.borrow().payload { - PyObjectPayload::Integer { ref value } => match range.index_of(value) { - Some(idx) => Ok(vm.ctx.new_int(idx)), - None => Err(vm.new_value_error(format!("{} is not in range", value))), - }, - _ => Err(vm.new_value_error("sequence.index(x): x not in sequence".to_string())), + let range = get_value(zelf); + + if objtype::isinstance(needle, &vm.ctx.int_type()) { + let needle = objint::get_value(needle); + + match range.index_of(&needle) { + Some(idx) => Ok(vm.ctx.new_int(idx)), + None => Err(vm.new_value_error(format!("{} is not in range", needle))), } } else { - unreachable!() + Err(vm.new_value_error("sequence.index(x): x not in sequence".to_string())) } }