From fedbd29051702d1f984f7c4dc335b79ecef13c25 Mon Sep 17 00:00:00 2001 From: ZapAnton Date: Fri, 24 May 2019 13:01:15 +0300 Subject: [PATCH 1/3] objstr: Refactored the 'mul' method - Used `str::repeat` instead of manually building the result string - Rewritten the int type check to return error at the beginning of the method - Replaced the `unwrap` calls with the single `unwrap_or` --- vm/src/obj/objstr.rs | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 44542a970..80b37d29a 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -183,22 +183,16 @@ impl PyString { #[pymethod(name = "__mul__")] fn mul(&self, val: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if objtype::isinstance(&val, &vm.ctx.int_type()) { - let value = &self.value; - let multiplier = objint::get_value(&val).to_i32().unwrap(); - let capacity = if multiplier > 0 { - multiplier.to_usize().unwrap() * value.len() - } else { - 0 - }; - let mut result = String::with_capacity(capacity); - for _x in 0..multiplier { - result.push_str(value.as_str()); - } - Ok(result) - } else { - Err(vm.new_type_error(format!("Cannot multiply {} and {}", self, val))) + if !objtype::isinstance(&val, &vm.ctx.int_type()) { + return Err(vm.new_type_error(format!("Cannot multiply {} and {}", self, val))); } + let value = &self.value; + let multiplier = objint::get_value(&val) + .to_i32() + .map(|multiplier| if multiplier < 0 { 0 } else { multiplier }) + .and_then(|multiplier| multiplier.to_usize()) + .unwrap_or(0); + Ok(value.repeat(multiplier)) } #[pymethod(name = "__rmul__")] From e38c54985e3fd40ff0c22263f53079b71cd9361a Mon Sep 17 00:00:00 2001 From: ZapAnton Date: Fri, 24 May 2019 20:37:47 +0300 Subject: [PATCH 2/3] objstr: Replaced the if-else construct with the max function --- vm/src/obj/objstr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 80b37d29a..3082a0c6e 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -189,7 +189,7 @@ impl PyString { let value = &self.value; let multiplier = objint::get_value(&val) .to_i32() - .map(|multiplier| if multiplier < 0 { 0 } else { multiplier }) + .map(|multiplier| multiplier.max(0)) .and_then(|multiplier| multiplier.to_usize()) .unwrap_or(0); Ok(value.repeat(multiplier)) From a2e64c0425f9d55310d4e69ab46bea88a5896dd8 Mon Sep 17 00:00:00 2001 From: ZapAnton Date: Sat, 25 May 2019 16:33:47 +0300 Subject: [PATCH 3/3] objstr: Replaced the unwrap_or with the OverflowError --- tests/snippets/strings.py | 2 ++ vm/src/obj/objstr.rs | 11 ++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/snippets/strings.py b/tests/snippets/strings.py index 2abd35da7..786622e9f 100644 --- a/tests/snippets/strings.py +++ b/tests/snippets/strings.py @@ -37,6 +37,8 @@ assert 3 * "xy" == "xyxyxy" assert 0 * "x" == "" assert -1 * "x" == "" +assert_raises(OverflowError, lambda: 'xy' * 234234234234234234234234234234) + a = 'Hallo' assert a.lower() == 'hallo' assert a.upper() == 'HALLO' diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 3082a0c6e..6545c9985 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -186,13 +186,14 @@ impl PyString { if !objtype::isinstance(&val, &vm.ctx.int_type()) { return Err(vm.new_type_error(format!("Cannot multiply {} and {}", self, val))); } - let value = &self.value; - let multiplier = objint::get_value(&val) - .to_i32() + objint::get_value(&val) + .to_isize() .map(|multiplier| multiplier.max(0)) .and_then(|multiplier| multiplier.to_usize()) - .unwrap_or(0); - Ok(value.repeat(multiplier)) + .map(|multiplier| self.value.repeat(multiplier)) + .ok_or_else(|| { + vm.new_overflow_error("cannot fit 'int' into an index-sized integer".to_string()) + }) } #[pymethod(name = "__rmul__")]