Fix some set operation to compare self with args #3912
Conversation
d5e969b to
ada0063
Compare
|
It is up to you, but you don't need to put everything in single PR. You can submit splat small PRs for each functions. |
vm/src/builtins/set.rs
Outdated
| @@ -658,7 +658,11 @@ impl PySet { | |||
| } | |||
|
|
|||
| #[pymethod(magic)] | |||
| fn iand(zelf: PyRef<Self>, set: AnySet, vm: &VirtualMachine) -> PyResult<PyRef<Self>> { | |||
| fn iand(zelf: PyRef<Self>, set: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyRef<Self>> { | |||
| if zelf.get_id() == set.get_id() { | |||
There was a problem hiding this comment.
| if zelf.get_id() == set.get_id() { | |
| if zelf.is(&set) { |
vm/src/builtins/set.rs
Outdated
| if zelf.get_id() == set.get_id() { | ||
| return Ok(zelf); | ||
| } |
There was a problem hiding this comment.
intersection_update also needs to have a check like this. As iand and intersection_update share the same PySetInner::intersection_update, we can do better 😊
|
@youknowone I think these operations are related closely. @Snowapril Thanks for your advice! I'll try to improve it! 👍 |
2da603f to
b378f64
Compare
b378f64 to
2930c84
Compare
|
@youknowone @Snowapril I tried to remove duplicated codes. But there are some different constraints between inplace operation methods. (Please See NOTE below) NOTE:
Same issue on |
| zelf.inner | ||
| .intersection_update(std::iter::once(set.into_iterable(vm)?), vm)?; | ||
| fn iand(zelf: PyRef<Self>, set: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyRef<Self>> { | ||
| if !set.class().is(vm.ctx.types.set_type) { |
There was a problem hiding this comment.
how about frozenset? if frozenset is allowed, maybe AnySet check?
There was a problem hiding this comment.
yeah, frozen sets are allowed here. Similarly for isub and ixor.
| if let Some(iterable) = arguments.first() { | ||
| if zelf.is(iterable) { | ||
| return Ok(()); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this check is not only adaptable for len() == 1 case but also do for every argument.
For example, how about s = set(['a']); s.intersection_update(s, s, s, s, s, s)?
| .intersection_update(std::iter::once(set.into_iterable(vm)?), vm)?; | ||
| fn iand(zelf: PyRef<Self>, set: PyObjectRef, vm: &VirtualMachine) -> PyResult<PyRef<Self>> { | ||
| if !set.class().is(vm.ctx.types.set_type) { | ||
| return Err(vm.new_type_error("Type error".to_owned())); |
There was a problem hiding this comment.
Should be "unsupported operand type(s) for +=: 'type1' and 'type2'", similarly for the rest of the type errors.
2930c84 to
0312c8e
Compare
Fix #3881