From 6a4b60783f88989101d70e9cb336ede5fc753b4a Mon Sep 17 00:00:00 2001 From: Taehong Kim Date: Mon, 18 Jul 2022 09:05:57 +0900 Subject: [PATCH 1/2] Fix `set` inplace operation bug --- vm/src/builtins/set.rs | 97 +++++++++++++++++++++++++++++++++--------- 1 file changed, 77 insertions(+), 20 deletions(-) diff --git a/vm/src/builtins/set.rs b/vm/src/builtins/set.rs index 5b5db30905..b823f25843 100644 --- a/vm/src/builtins/set.rs +++ b/vm/src/builtins/set.rs @@ -380,12 +380,13 @@ impl PySetInner { fn intersection_update( &self, - others: impl std::iter::Iterator, + others: impl std::iter::Iterator, vm: &VirtualMachine, ) -> PyResult<()> { let mut temp_inner = self.copy(); self.clear(); for iterable in others { + let iterable = ArgIterable::::try_from_object(vm, iterable)?; for item in iterable.iter(vm)? { let obj = item?; if temp_inner.contains(&obj, vm)? { @@ -399,10 +400,11 @@ impl PySetInner { fn difference_update( &self, - others: impl std::iter::Iterator, + others: impl std::iter::Iterator, vm: &VirtualMachine, ) -> PyResult<()> { for iterable in others { + let iterable = ArgIterable::::try_from_object(vm, iterable)?; for item in iterable.iter(vm)? { self.content.delete_if_exists(vm, &*item?)?; } @@ -412,11 +414,12 @@ impl PySetInner { fn symmetric_difference_update( &self, - others: impl std::iter::Iterator, + others: impl std::iter::Iterator, vm: &VirtualMachine, ) -> PyResult<()> { for iterable in others { // We want to remove duplicates in iterable + let iterable = ArgIterable::::try_from_object(vm, iterable)?; let iterable_set = Self::from_iter(iterable.iter(vm)?, vm)?; for item in iterable_set.elements() { self.content.delete_or_insert(vm, &item, ())?; @@ -676,49 +679,103 @@ impl PySet { #[pymethod] fn intersection_update( - &self, - others: PosArgs, + zelf: PyRef, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult<()> { - self.inner.intersection_update(others.into_iter(), vm)?; + let arguments = others.into_vec(); + if arguments.len() == 1 { + if let Some(iterable) = arguments.first() { + if zelf.is(iterable) { + return Ok(()); + } + } + } else if arguments.is_empty() { + return Ok(()); + } + zelf.inner.intersection_update(arguments.into_iter(), vm)?; Ok(()) } #[pymethod(magic)] - fn iand(zelf: PyRef, set: AnySet, vm: &VirtualMachine) -> PyResult> { - zelf.inner - .intersection_update(std::iter::once(set.into_iterable(vm)?), vm)?; + fn iand(zelf: PyRef, set: PyObjectRef, vm: &VirtualMachine) -> PyResult> { + if !set.class().is(vm.ctx.types.set_type) { + return Err(vm.new_type_error("Type error".to_owned())); + } + if zelf.is(&set) { + return Ok(zelf); + } + zelf.inner.intersection_update(std::iter::once(set), vm)?; Ok(zelf) } #[pymethod] - fn difference_update(&self, others: PosArgs, vm: &VirtualMachine) -> PyResult<()> { - self.inner.difference_update(others.into_iter(), vm)?; + fn difference_update( + zelf: PyRef, + others: PosArgs, + vm: &VirtualMachine, + ) -> PyResult<()> { + let arguments = others.into_vec(); + if arguments.len() == 1 { + if let Some(iterable) = arguments.first() { + if zelf.is(iterable) { + zelf.inner.clear(); + return Ok(()); + } + } + } else if arguments.is_empty() { + return Ok(()); + } + zelf.inner.difference_update(arguments.into_iter(), vm)?; Ok(()) } #[pymethod(magic)] - fn isub(zelf: PyRef, set: AnySet, vm: &VirtualMachine) -> PyResult> { - zelf.inner - .difference_update(set.into_iterable_iter(vm)?, vm)?; + fn isub(zelf: PyRef, set: PyObjectRef, vm: &VirtualMachine) -> PyResult> { + if !set.class().is(vm.ctx.types.set_type) { + return Err(vm.new_type_error("Type error".to_owned())); + } + if zelf.is(&set) { + zelf.inner.clear(); + return Ok(zelf); + } + zelf.inner.difference_update(std::iter::once(set), vm)?; Ok(zelf) } #[pymethod] fn symmetric_difference_update( - &self, - others: PosArgs, + zelf: PyRef, + others: PosArgs, vm: &VirtualMachine, ) -> PyResult<()> { - self.inner - .symmetric_difference_update(others.into_iter(), vm)?; + let arguments = others.into_vec(); + if arguments.len() == 1 { + if let Some(iterable) = arguments.first() { + if zelf.is(iterable) { + zelf.inner.clear(); + return Ok(()); + } + } + } else if arguments.is_empty() { + return Ok(()); + } + zelf.inner + .symmetric_difference_update(arguments.into_iter(), vm)?; Ok(()) } #[pymethod(magic)] - fn ixor(zelf: PyRef, set: AnySet, vm: &VirtualMachine) -> PyResult> { + fn ixor(zelf: PyRef, set: PyObjectRef, vm: &VirtualMachine) -> PyResult> { + if !set.class().is(vm.ctx.types.set_type) { + return Err(vm.new_type_error("Type error".to_owned())); + } + if zelf.is(&set) { + zelf.inner.clear(); + return Ok(zelf); + } zelf.inner - .symmetric_difference_update(set.into_iterable_iter(vm)?, vm)?; + .symmetric_difference_update(std::iter::once(set), vm)?; Ok(zelf) } From 0312c8eb2c3db947ab75db7fc8f1186b61ba9ec7 Mon Sep 17 00:00:00 2001 From: Taehong Kim Date: Tue, 19 Jul 2022 23:35:41 +0900 Subject: [PATCH 2/2] Fix unexpected success test --- Lib/test/test_set.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py index 4284393ca5..060dbfa14a 100644 --- a/Lib/test/test_set.py +++ b/Lib/test/test_set.py @@ -582,8 +582,6 @@ def test_ixor(self): else: self.assertNotIn(c, self.s) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_inplace_on_self(self): t = self.s.copy() t |= t