From 6499dbbfc5fe273deb7168de7d7c96e3511a015e Mon Sep 17 00:00:00 2001 From: HyeockJinKim Date: Mon, 7 Oct 2019 16:36:19 +0900 Subject: [PATCH 1/3] Fixed __hash__ of range Calculate hash value of range through hash of tuple Fixes #1482 --- vm/src/obj/objrange.rs | 23 +++++++++++++++++++++++ vm/src/obj/objtuple.rs | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/vm/src/obj/objrange.rs b/vm/src/obj/objrange.rs index 3b2c6e68e..acb38e6b8 100644 --- a/vm/src/obj/objrange.rs +++ b/vm/src/obj/objrange.rs @@ -5,6 +5,7 @@ use num_integer::Integer; use num_traits::{One, Signed, Zero}; use crate::function::{OptionalArg, PyFuncArgs}; +use crate::pyhash; use crate::pyobject::{ PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; @@ -14,6 +15,7 @@ use super::objint::{PyInt, PyIntRef}; use super::objiter; use super::objslice::{PySlice, PySliceRef}; use super::objtype::{self, PyClassRef}; +use crate::obj::objtuple::PyTuple; /// range(stop) -> range object /// range(start, stop[, step]) -> range object @@ -402,6 +404,27 @@ impl PyRange { } } + #[pymethod(name = "__hash__")] + fn hash(zelf: PyRef, vm: &VirtualMachine) -> PyResult { + let length = zelf.length(); + let len = length.as_bigint().clone(); + let a = length.into_ref(vm).into_object(); + let mut b = vm.get_none(); + let mut c = vm.get_none(); + if !len.is_zero() { + b = zelf.start.clone().into_object(); + if !len.is_one() { + c = zelf.step(vm).into_object(); + } + }; + let tuple = vm + .ctx + .new_tuple(vec![a, b, c]) + .downcast::() + .unwrap(); + tuple.hash(vm) + } + #[pyslot(new)] fn tp_new(args: PyFuncArgs, vm: &VirtualMachine) -> PyResult { let range = if args.args.len() <= 2 { diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 821871f29..6196b28c6 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -179,7 +179,7 @@ impl PyTuple { } #[pymethod(name = "__hash__")] - fn hash(&self, vm: &VirtualMachine) -> PyResult { + pub fn hash(&self, vm: &VirtualMachine) -> PyResult { pyhash::hash_iter(self.elements.iter(), vm) } From 145ad87b4156f9264fc5b461ef57f1503fb6e398 Mon Sep 17 00:00:00 2001 From: HyeockJinKim Date: Mon, 7 Oct 2019 16:44:39 +0900 Subject: [PATCH 2/3] Add tests for range's hash --- tests/snippets/builtin_range.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/snippets/builtin_range.py b/tests/snippets/builtin_range.py index 6772a87dc..e0918bd3f 100644 --- a/tests/snippets/builtin_range.py +++ b/tests/snippets/builtin_range.py @@ -113,3 +113,15 @@ assert range(10)[-2:4] == range(8, 4) assert range(10)[-6:-2] == range(4, 8) assert range(50, 0, -2)[-5] == 10 assert range(50, 0, -2)[-5:3:5] == range(10, 44, -10) + +assert hash(range(10)) == hash((10, 0, 1)) +assert hash(range(10)) == hash(range(10)) +assert hash(range(100)[20:30]) == hash(range(20, 30)) +assert hash(range(10, 10)) == hash(range(0, 0)) +assert hash(range(1, 2, 100)) == hash(range(1, 6, 100)) + +a = {} +for i in range(100): + a[range(10)] = 1 + +assert len(a.keys()) == 1 From f43ae4424eb3b32606608c613b5407e3deb79309 Mon Sep 17 00:00:00 2001 From: HyeockJinKim Date: Mon, 7 Oct 2019 22:18:03 +0900 Subject: [PATCH 3/3] Change return type of length to BigInt Change the return type of length to BigInt since most of the length's values are used as_bigint --- vm/src/obj/objrange.rs | 55 ++++++++++++++++++------------------------ vm/src/obj/objtuple.rs | 2 +- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/vm/src/obj/objrange.rs b/vm/src/obj/objrange.rs index acb38e6b8..e71d03255 100644 --- a/vm/src/obj/objrange.rs +++ b/vm/src/obj/objrange.rs @@ -15,7 +15,6 @@ use super::objint::{PyInt, PyIntRef}; use super::objiter; use super::objslice::{PySlice, PySliceRef}; use super::objtype::{self, PyClassRef}; -use crate::obj::objtuple::PyTuple; /// range(stop) -> range object /// range(start, stop[, step]) -> range object @@ -77,18 +76,13 @@ impl PyRange { #[inline] pub fn get(&self, index: &BigInt) -> Option { let start = self.start.as_bigint(); - let stop = self.stop.as_bigint(); let step = self.step.as_bigint(); let index = index.clone(); if self.is_empty() { return None; } - let length: BigInt = if start < stop { - (stop - start - 1) / step + 1 - } else { - (start - stop - 1) / (-step) + 1 - }; + let length = self.length(); let index = if index.is_negative() { let new_index: BigInt = &length + &index; @@ -108,15 +102,15 @@ impl PyRange { } #[inline] - fn length(&self) -> PyInt { + fn length(&self) -> BigInt { let start = self.start.as_bigint(); let stop = self.stop.as_bigint(); let step = self.step.as_bigint(); match step.sign() { - Sign::Plus if start < stop => PyInt::new((stop - start - 1usize) / step + 1), - Sign::Minus if start > stop => PyInt::new((start - stop - 1usize) / (-step) + 1), - Sign::Plus | Sign::Minus => PyInt::new(0), + Sign::Plus if start < stop => (stop - start - 1usize) / step + 1, + Sign::Minus if start > stop => (start - stop - 1usize) / (-step) + 1, + Sign::Plus | Sign::Minus => BigInt::zero(), Sign::NoSign => unreachable!(), } } @@ -216,7 +210,7 @@ impl PyRange { #[pymethod(name = "__len__")] fn len(&self, _vm: &VirtualMachine) -> PyInt { - self.length() + PyInt::new(self.length()) } #[pymethod(name = "__repr__")] @@ -250,11 +244,11 @@ impl PyRange { if objtype::isinstance(&rhs, &vm.ctx.range_type()) { let rhs = get_value(&rhs); - if self.length().as_bigint() != rhs.length().as_bigint() { + if self.length() != rhs.length() { return false; } - if self.length().as_bigint().is_zero() { + if self.length().is_zero() { return true; } @@ -323,8 +317,7 @@ impl PyRange { RangeIndex::Slice(slice) => { let range_start = self.start.as_bigint(); let range_step = self.step.as_bigint(); - let _tmp_len = self.length(); - let range_length = _tmp_len.as_bigint(); + let range_length = &self.length(); let substep = if let Some(slice_step) = slice.step_index(vm)? { if slice_step.is_zero() { @@ -407,22 +400,22 @@ impl PyRange { #[pymethod(name = "__hash__")] fn hash(zelf: PyRef, vm: &VirtualMachine) -> PyResult { let length = zelf.length(); - let len = length.as_bigint().clone(); - let a = length.into_ref(vm).into_object(); - let mut b = vm.get_none(); - let mut c = vm.get_none(); - if !len.is_zero() { - b = zelf.start.clone().into_object(); - if !len.is_one() { - c = zelf.step(vm).into_object(); - } + let elements = if length.is_zero() { + vec![vm.ctx.new_int(length), vm.get_none(), vm.get_none()] + } else if length.is_one() { + vec![ + vm.ctx.new_int(length), + zelf.start(vm).into_object(), + vm.get_none(), + ] + } else { + vec![ + vm.ctx.new_int(length), + zelf.start(vm).into_object(), + zelf.step(vm).into_object(), + ] }; - let tuple = vm - .ctx - .new_tuple(vec![a, b, c]) - .downcast::() - .unwrap(); - tuple.hash(vm) + pyhash::hash_iter(elements.iter(), vm) } #[pyslot(new)] diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index 6196b28c6..821871f29 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -179,7 +179,7 @@ impl PyTuple { } #[pymethod(name = "__hash__")] - pub fn hash(&self, vm: &VirtualMachine) -> PyResult { + fn hash(&self, vm: &VirtualMachine) -> PyResult { pyhash::hash_iter(self.elements.iter(), vm) }