Merge pull request RustPython#1501 from youknowone/bool-equal

windelbouwman · web-flow · commit 23075e542060 · 2019-10-12T12:47:57.000+02:00
introduce and adapt vm.identical_or_equal to test `is` or else `__eq__`
diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs
@@ -1,4 +1,3 @@
-use crate::obj::objbool;
 use crate::obj::objstr::PyString;
 use crate::pyhash;
 use crate::pyobject::{IdProtocol, IntoPyObject, PyObjectRef, PyResult};
@@ -305,8 +304,7 @@ impl DictKey for &PyObjectRef {
     }
 
     fn do_eq(self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {
-        let result = vm._eq(self.clone(), other_key.clone())?;
-        objbool::boolval(vm, result)
+        vm.identical_or_equal(self, other_key)
     }
 }
 
@@ -399,7 +397,7 @@ mod tests {
         assert_eq!(true, dict.contains(&vm, "x").unwrap());
 
         let val = dict.get(&vm, "x").unwrap().unwrap();
-        vm._eq(val, value2)
+        vm.bool_eq(val, value2)
             .expect("retrieved value must be equal to inserted value.");
     }
 
diff --git a/vm/src/obj/objdict.rs b/vm/src/obj/objdict.rs
@@ -1,7 +1,6 @@
 use std::cell::{Cell, RefCell};
 use std::fmt;
 
-use super::objbool;
 use super::objiter;
 use super::objstr;
 use super::objtype::{self, PyClassRef};
@@ -121,8 +120,7 @@ impl PyDictRef {
                     if v1.is(&v2) {
                         continue;
                     }
-                    let value = objbool::boolval(vm, vm._eq(v1, v2)?)?;
-                    if !value {
+                    if !vm.bool_eq(v1, v2)? {
                         return Ok(false);
                     }
                 }
diff --git a/vm/src/obj/objlist.rs b/vm/src/obj/objlist.rs
@@ -447,25 +447,16 @@ impl PyListRef {
     fn count(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
         let mut count: usize = 0;
         for element in self.elements.borrow().iter() {
-            if needle.is(element) {
+            if vm.identical_or_equal(element, &needle)? {
                 count += 1;
-            } else {
-                let py_equal = vm._eq(element.clone(), needle.clone())?;
-                if objbool::boolval(vm, py_equal)? {
-                    count += 1;
-                }
             }
         }
         Ok(count)
     }
 
     fn contains(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
         for element in self.elements.borrow().iter() {
-            if needle.is(element) {
-                return Ok(true);
-            }
-            let py_equal = vm._eq(element.clone(), needle.clone())?;
-            if objbool::boolval(vm, py_equal)? {
+            if vm.identical_or_equal(element, &needle)? {
                 return Ok(true);
             }
         }
@@ -475,11 +466,7 @@ impl PyListRef {
 
     fn index(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
         for (index, element) in self.elements.borrow().iter().enumerate() {
-            if needle.is(element) {
-                return Ok(index);
-            }
-            let py_equal = vm._eq(needle.clone(), element.clone())?;
-            if objbool::boolval(vm, py_equal)? {
+            if vm.identical_or_equal(element, &needle)? {
                 return Ok(index);
             }
         }
@@ -505,12 +492,7 @@ impl PyListRef {
     fn remove(self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
         let mut ri: Option<usize> = None;
         for (index, element) in self.elements.borrow().iter().enumerate() {
-            if needle.is(element) {
-                ri = Some(index);
-                break;
-            }
-            let py_equal = vm._eq(needle.clone(), element.clone())?;
-            if objbool::get_value(&py_equal) {
+            if vm.identical_or_equal(element, &needle)? {
                 ri = Some(index);
                 break;
             }
diff --git a/vm/src/obj/objslice.rs b/vm/src/obj/objslice.rs
@@ -117,13 +117,13 @@ impl PySlice {
     }
 
     fn inner_eq(&self, other: &PySlice, vm: &VirtualMachine) -> PyResult<bool> {
-        if !vm.bool_eq(self.start(vm), other.start(vm))? {
+        if !vm.identical_or_equal(&self.start(vm), &other.start(vm))? {
             return Ok(false);
         }
-        if !vm.bool_eq(self.stop(vm), other.stop(vm))? {
+        if !vm.identical_or_equal(&self.stop(vm), &other.stop(vm))? {
             return Ok(false);
         }
-        if !vm.bool_eq(self.step(vm), other.step(vm))? {
+        if !vm.identical_or_equal(&self.step(vm), &other.step(vm))? {
             return Ok(false);
         }
         Ok(true)
diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs
@@ -1,7 +1,6 @@
 use std::cell::Cell;
 use std::fmt;
 
-use super::objbool;
 use super::objiter;
 use super::objsequence::{
     get_elements_tuple, get_item, seq_equal, seq_ge, seq_gt, seq_le, seq_lt, seq_mul,
@@ -10,7 +9,7 @@ use super::objtype::{self, PyClassRef};
 use crate::function::OptionalArg;
 use crate::pyhash;
 use crate::pyobject::{
-    IdProtocol, IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue,
+    IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue,
 };
 use crate::vm::{ReprGuard, VirtualMachine};
 
@@ -142,13 +141,8 @@ impl PyTuple {
     fn count(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
         let mut count: usize = 0;
         for element in self.elements.iter() {
-            if element.is(&needle) {
+            if vm.identical_or_equal(element, &needle)? {
                 count += 1;
-            } else {
-                let is_eq = vm._eq(element.clone(), needle.clone())?;
-                if objbool::boolval(vm, is_eq)? {
-                    count += 1;
-                }
             }
         }
         Ok(count)
@@ -236,11 +230,7 @@ impl PyTuple {
     #[pymethod(name = "index")]
     fn index(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
         for (index, element) in self.elements.iter().enumerate() {
-            if element.is(&needle) {
-                return Ok(index);
-            }
-            let is_eq = vm._eq(needle.clone(), element.clone())?;
-            if objbool::boolval(vm, is_eq)? {
+            if vm.identical_or_equal(element, &needle)? {
                 return Ok(index);
             }
         }
@@ -250,11 +240,7 @@ impl PyTuple {
     #[pymethod(name = "__contains__")]
     fn contains(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
         for element in self.elements.iter() {
-            if element.is(&needle) {
-                return Ok(true);
-            }
-            let is_eq = vm._eq(needle.clone(), element.clone())?;
-            if objbool::boolval(vm, is_eq)? {
+            if vm.identical_or_equal(element, &needle)? {
                 return Ok(true);
             }
         }
diff --git a/vm/src/stdlib/collections.rs b/vm/src/stdlib/collections.rs
@@ -1,5 +1,5 @@
 use crate::function::OptionalArg;
-use crate::obj::{objbool, objiter, objsequence, objtype::PyClassRef};
+use crate::obj::{objiter, objsequence, objtype::PyClassRef};
 use crate::pyobject::{IdProtocol, PyClassImpl, PyIterable, PyObjectRef, PyRef, PyResult, PyValue};
 use crate::vm::ReprGuard;
 use crate::VirtualMachine;
@@ -78,7 +78,7 @@ impl PyDeque {
     fn count(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
         let mut count = 0;
         for elem in self.deque.borrow().iter() {
-            if objbool::boolval(vm, vm._eq(elem.clone(), obj.clone())?)? {
+            if vm.identical_or_equal(elem, &obj)? {
                 count += 1;
             }
         }
@@ -114,7 +114,7 @@ impl PyDeque {
         let start = start.unwrap_or(0);
         let stop = stop.unwrap_or_else(|| deque.len());
         for (i, elem) in deque.iter().skip(start).take(stop - start).enumerate() {
-            if objbool::boolval(vm, vm._eq(elem.clone(), obj.clone())?)? {
+            if vm.identical_or_equal(elem, &obj)? {
                 return Ok(i);
             }
         }
@@ -171,7 +171,7 @@ impl PyDeque {
         let mut deque = self.deque.borrow_mut();
         let mut idx = None;
         for (i, elem) in deque.iter().enumerate() {
-            if objbool::boolval(vm, vm._eq(elem.clone(), obj.clone())?)? {
+            if vm.identical_or_equal(elem, &obj)? {
                 idx = Some(i);
                 break;
             }
diff --git a/vm/src/vm.rs b/vm/src/vm.rs
@@ -1238,8 +1238,7 @@ impl VirtualMachine {
         let iter = objiter::get_iter(self, &haystack)?;
         loop {
             if let Some(element) = objiter::get_next_object(self, &iter)? {
-                let equal = self._eq(needle.clone(), element.clone())?;
-                if objbool::get_value(&equal) {
+                if self.bool_eq(needle.clone(), element.clone())? {
                     return Ok(self.new_bool(true));
                 } else {
                     continue;
@@ -1272,11 +1271,19 @@ impl VirtualMachine {
     }
 
     pub fn bool_eq(&self, a: PyObjectRef, b: PyObjectRef) -> PyResult<bool> {
-        let eq = self._eq(a.clone(), b.clone())?;
+        let eq = self._eq(a, b)?;
         let value = objbool::boolval(self, eq)?;
         Ok(value)
     }
 
+    pub fn identical_or_equal(&self, a: &PyObjectRef, b: &PyObjectRef) -> PyResult<bool> {
+        if a.is(b) {
+            Ok(true)
+        } else {
+            self.bool_eq(a.clone(), b.clone())
+        }
+    }
+
     pub fn bool_seq_lt(&self, a: PyObjectRef, b: PyObjectRef) -> PyResult<Option<bool>> {
         let value = if objbool::boolval(self, self._lt(a.clone(), b.clone())?)? {
             Some(true)

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-use crate::obj::objbool;`
`2`	`1`	`use crate::obj::objstr::PyString;`
`3`	`2`	`use crate::pyhash;`
`4`	`3`	`use crate::pyobject::{IdProtocol, IntoPyObject, PyObjectRef, PyResult};`
`@@ -305,8 +304,7 @@ impl DictKey for &PyObjectRef {`
`305`	`304`	`}`
`306`	`305`
`307`	`306`	`fn do_eq(self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> {`
`308`		`- let result = vm._eq(self.clone(), other_key.clone())?;`
`309`		`- objbool::boolval(vm, result)`
	`307`	`+ vm.identical_or_equal(self, other_key)`
`310`	`308`	`}`
`311`	`309`	`}`
`312`	`310`
`@@ -399,7 +397,7 @@ mod tests {`
`399`	`397`	`assert_eq!(true, dict.contains(&vm, "x").unwrap());`
`400`	`398`
`401`	`399`	`let val = dict.get(&vm, "x").unwrap().unwrap();`
`402`		`- vm._eq(val, value2)`
	`400`	`+ vm.bool_eq(val, value2)`
`403`	`401`	`.expect("retrieved value must be equal to inserted value.");`
`404`	`402`	`}`
`405`	`403`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,6 @@`
`1`	`1`	`use std::cell::{Cell, RefCell};`
`2`	`2`	`use std::fmt;`
`3`	`3`
`4`		`-use super::objbool;`
`5`	`4`	`use super::objiter;`
`6`	`5`	`use super::objstr;`
`7`	`6`	`use super::objtype::{self, PyClassRef};`
`@@ -121,8 +120,7 @@ impl PyDictRef {`
`121`	`120`	`if v1.is(&v2) {`
`122`	`121`	`continue;`
`123`	`122`	`}`
`124`		`- let value = objbool::boolval(vm, vm._eq(v1, v2)?)?;`
`125`		`- if !value {`
	`123`	`+ if !vm.bool_eq(v1, v2)? {`
`126`	`124`	`return Ok(false);`
`127`	`125`	`}`
`128`	`126`	`}`
Original file line number	Diff line number	Diff line change
`@@ -117,13 +117,13 @@ impl PySlice {`
`117`	`117`	`}`
`118`	`118`
`119`	`119`	`fn inner_eq(&self, other: &PySlice, vm: &VirtualMachine) -> PyResult<bool> {`
`120`		`- if !vm.bool_eq(self.start(vm), other.start(vm))? {`
	`120`	`+ if !vm.identical_or_equal(&self.start(vm), &other.start(vm))? {`
`121`	`121`	`return Ok(false);`
`122`	`122`	`}`
`123`		`- if !vm.bool_eq(self.stop(vm), other.stop(vm))? {`
	`123`	`+ if !vm.identical_or_equal(&self.stop(vm), &other.stop(vm))? {`
`124`	`124`	`return Ok(false);`
`125`	`125`	`}`
`126`		`- if !vm.bool_eq(self.step(vm), other.step(vm))? {`
	`126`	`+ if !vm.identical_or_equal(&self.step(vm), &other.step(vm))? {`
`127`	`127`	`return Ok(false);`
`128`	`128`	`}`
`129`	`129`	`Ok(true)`