Check operand types in bool or, and, xor to be PyInt (RustPython#5461)

theshubhamp · web-flow · commit 38593fbd85bd · 2024-12-30T16:44:27.000+09:00
* Added Tests for Bitwise or, and, xor type error

* Sync binary operator order comment with actual implementation

* Check operand types in bool or, and, xor to be PyInt

PyNumber methods are expected to type check both arguments.

Dispatch is not done by inverting parameter order for __r&lt;op&gt;__ (example __ror__) when calls are handled via PyNumberMethods
diff --git a/extra_tests/snippets/operator_arithmetic.py b/extra_tests/snippets/operator_arithmetic.py
@@ -32,3 +32,23 @@
 
 # Right shift raises value error on negative
 assert_raises(ValueError, lambda: 1 >> -1)
+
+# Bitwise or, and, xor raises value error on incompatible types
+assert_raises(TypeError, lambda: "abc" | True)
+assert_raises(TypeError, lambda: "abc" & True)
+assert_raises(TypeError, lambda: "abc" ^ True)
+assert_raises(TypeError, lambda: True | "abc")
+assert_raises(TypeError, lambda: True & "abc")
+assert_raises(TypeError, lambda: True ^ "abc")
+assert_raises(TypeError, lambda: "abc" | 1.5)
+assert_raises(TypeError, lambda: "abc" & 1.5)
+assert_raises(TypeError, lambda: "abc" ^ 1.5)
+assert_raises(TypeError, lambda: 1.5 | "abc")
+assert_raises(TypeError, lambda: 1.5 & "abc")
+assert_raises(TypeError, lambda: 1.5 ^ "abc")
+assert_raises(TypeError, lambda: True | 1.5)
+assert_raises(TypeError, lambda: True & 1.5)
+assert_raises(TypeError, lambda: True ^ 1.5)
+assert_raises(TypeError, lambda: 1.5 | True)
+assert_raises(TypeError, lambda: 1.5 & True)
+assert_raises(TypeError, lambda: 1.5 ^ True)
diff --git a/vm/src/builtins/bool.rs b/vm/src/builtins/bool.rs
@@ -127,8 +127,10 @@ impl PyBool {
             let lhs = get_value(&lhs);
             let rhs = get_value(&rhs);
             (lhs || rhs).to_pyobject(vm)
+        } else if let Some(lhs) = lhs.payload::<PyInt>() {
+            lhs.or(rhs, vm).to_pyobject(vm)
         } else {
-            get_py_int(&lhs).or(rhs, vm).to_pyobject(vm)
+            vm.ctx.not_implemented()
         }
     }
 
@@ -141,8 +143,10 @@ impl PyBool {
             let lhs = get_value(&lhs);
             let rhs = get_value(&rhs);
             (lhs && rhs).to_pyobject(vm)
+        } else if let Some(lhs) = lhs.payload::<PyInt>() {
+            lhs.and(rhs, vm).to_pyobject(vm)
         } else {
-            get_py_int(&lhs).and(rhs, vm).to_pyobject(vm)
+            vm.ctx.not_implemented()
         }
     }
 
@@ -155,8 +159,10 @@ impl PyBool {
             let lhs = get_value(&lhs);
             let rhs = get_value(&rhs);
             (lhs ^ rhs).to_pyobject(vm)
+        } else if let Some(lhs) = lhs.payload::<PyInt>() {
+            lhs.xor(rhs, vm).to_pyobject(vm)
         } else {
-            get_py_int(&lhs).xor(rhs, vm).to_pyobject(vm)
+            vm.ctx.not_implemented()
         }
     }
 }
@@ -207,7 +213,3 @@ pub(crate) fn init(context: &Context) {
 pub(crate) fn get_value(obj: &PyObject) -> bool {
     !obj.payload::<PyInt>().unwrap().as_bigint().is_zero()
 }
-
-fn get_py_int(obj: &PyObject) -> &PyInt {
-    obj.payload::<PyInt>().unwrap()
-}

Original file line number	Diff line number	Diff line change
`@@ -127,8 +127,10 @@ impl PyBool {`
`127`	`127`	`let lhs = get_value(&lhs);`
`128`	`128`	`let rhs = get_value(&rhs);`
`129`	`129`	`(lhs \|\| rhs).to_pyobject(vm)`
	`130`	`+ } else if let Some(lhs) = lhs.payload::<PyInt>() {`
	`131`	`+ lhs.or(rhs, vm).to_pyobject(vm)`
`130`	`132`	`} else {`
`131`		`- get_py_int(&lhs).or(rhs, vm).to_pyobject(vm)`
	`133`	`+ vm.ctx.not_implemented()`
`132`	`134`	`}`
`133`	`135`	`}`
`134`	`136`
`@@ -141,8 +143,10 @@ impl PyBool {`
`141`	`143`	`let lhs = get_value(&lhs);`
`142`	`144`	`let rhs = get_value(&rhs);`
`143`	`145`	`(lhs && rhs).to_pyobject(vm)`
	`146`	`+ } else if let Some(lhs) = lhs.payload::<PyInt>() {`
	`147`	`+ lhs.and(rhs, vm).to_pyobject(vm)`
`144`	`148`	`} else {`
`145`		`- get_py_int(&lhs).and(rhs, vm).to_pyobject(vm)`
	`149`	`+ vm.ctx.not_implemented()`
`146`	`150`	`}`
`147`	`151`	`}`
`148`	`152`
`@@ -155,8 +159,10 @@ impl PyBool {`
`155`	`159`	`let lhs = get_value(&lhs);`
`156`	`160`	`let rhs = get_value(&rhs);`
`157`	`161`	`(lhs ^ rhs).to_pyobject(vm)`
	`162`	`+ } else if let Some(lhs) = lhs.payload::<PyInt>() {`
	`163`	`+ lhs.xor(rhs, vm).to_pyobject(vm)`
`158`	`164`	`} else {`
`159`		`- get_py_int(&lhs).xor(rhs, vm).to_pyobject(vm)`
	`165`	`+ vm.ctx.not_implemented()`
`160`	`166`	`}`
`161`	`167`	`}`
`162`	`168`	`}`
`@@ -207,7 +213,3 @@ pub(crate) fn init(context: &Context) {`
`207`	`213`	`pub(crate) fn get_value(obj: &PyObject) -> bool {`
`208`	`214`	`!obj.payload::<PyInt>().unwrap().as_bigint().is_zero()`
`209`	`215`	`}`
`210`		`-`
`211`		`-fn get_py_int(obj: &PyObject) -> &PyInt {`
`212`		`- obj.payload::<PyInt>().unwrap()`
`213`		`-}`