Skip to content

Only use real isinstance/issubclass for built-ins #594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions vm/src/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ fn builtin_isinstance(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
required = [(obj, None), (typ, Some(vm.get_type()))]
);

let isinstance = objtype::real_isinstance(vm, obj, typ)?;
let isinstance = vm.isinstance(obj, typ)?;
Ok(vm.new_bool(isinstance))
}

Expand All @@ -358,7 +358,7 @@ fn builtin_issubclass(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
required = [(subclass, Some(vm.get_type())), (cls, Some(vm.get_type()))]
);

let issubclass = objtype::real_issubclass(vm, subclass, cls)?;
let issubclass = vm.issubclass(subclass, cls)?;
Ok(vm.context().new_bool(issubclass))
}

Expand Down Expand Up @@ -814,7 +814,7 @@ pub fn builtin_build_class_(vm: &mut VirtualMachine, mut args: PyFuncArgs) -> Py
let mut metaclass = args.get_kwarg("metaclass", vm.get_type());

for base in bases.clone() {
if objtype::real_issubclass(vm, &base.typ(), &metaclass)? {
if objtype::issubclass(&base.typ(), &metaclass) {
metaclass = base.typ();
} else if !objtype::issubclass(&metaclass, &base.typ()) {
return Err(vm.new_type_error("metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases".to_string()));
Expand Down
2 changes: 1 addition & 1 deletion vm/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ macro_rules! type_check {
if let Some(expected_type) = $arg_type {
let arg = &$args.args[$arg_count];

if !$crate::obj::objtype::real_isinstance($vm, arg, &expected_type)? {
if !$crate::obj::objtype::isinstance(arg, &expected_type) {
let arg_typ = arg.typ();
let expected_type_name = $vm.to_pystr(&expected_type)?;
let actual_type = $vm.to_pystr(&arg_typ)?;
Expand Down
4 changes: 1 addition & 3 deletions vm/src/obj/objsuper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ fn super_init(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
};

// Check obj type:
if !(objtype::real_isinstance(vm, &py_obj, &py_type)?
|| objtype::real_issubclass(vm, &py_obj, &py_type)?)
{
if !(objtype::isinstance(&py_obj, &py_type) || objtype::issubclass(&py_obj, &py_type)) {
return Err(vm.new_type_error(
"super(type, obj): obj must be an instance or subtype of type".to_string(),
));
Expand Down
30 changes: 0 additions & 30 deletions vm/src/obj/objtype.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use super::objbool;
use super::objdict;
use super::objstr;
use super::objtype; // Required for arg_check! to use isinstance
Expand All @@ -9,7 +8,6 @@ use crate::pyobject::{
use crate::vm::VirtualMachine;
use std::cell::RefCell;
use std::collections::HashMap;
use std::rc::Rc;

/*
* The magical type type
Expand Down Expand Up @@ -108,23 +106,6 @@ pub fn isinstance(obj: &PyObjectRef, cls: &PyObjectRef) -> bool {
mro.into_iter().any(|c| c.is(&cls))
}

/// Determines if `obj` is an instance of `cls`, either directly, indirectly or virtually via the
/// __instancecheck__ magic method.
pub fn real_isinstance(
vm: &mut VirtualMachine,
obj: &PyObjectRef,
cls: &PyObjectRef,
) -> PyResult<bool> {
// cpython first does an exact check on the type, although documentation doesn't state that
// https://github.com/python/cpython/blob/a24107b04c1277e3c1105f98aff5bfa3a98b33a0/Objects/abstract.c#L2408
if Rc::ptr_eq(&obj.typ(), cls) {
Ok(true)
} else {
let ret = vm.call_method(cls, "__instancecheck__", vec![obj.clone()])?;
objbool::boolval(vm, ret)
}
}

fn type_instance_check(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(
vm,
Expand All @@ -142,17 +123,6 @@ pub fn issubclass(subclass: &PyObjectRef, cls: &PyObjectRef) -> bool {
mro.into_iter().any(|c| c.is(&cls))
}

/// Determines if `subclass` is a subclass of `cls`, either directly, indirectly or virtually via
/// the __subclasscheck__ magic method.
pub fn real_issubclass(
vm: &mut VirtualMachine,
subclass: &PyObjectRef,
cls: &PyObjectRef,
) -> PyResult<bool> {
let ret = vm.call_method(cls, "__subclasscheck__", vec![subclass.clone()])?;
objbool::boolval(vm, ret)
}

fn type_subclass_check(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(
vm,
Expand Down
10 changes: 2 additions & 8 deletions vm/src/pyobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,7 @@ impl PyFuncArgs {
) -> Result<Option<PyObjectRef>, PyObjectRef> {
match self.get_optional_kwarg(key) {
Some(kwarg) => {
if objtype::real_isinstance(vm, &kwarg, &ty)? {
if objtype::isinstance(&kwarg, &ty) {
Ok(Some(kwarg))
} else {
let expected_ty_name = vm.to_pystr(&ty)?;
Expand Down Expand Up @@ -1003,13 +1003,7 @@ where
Ok(value) => Some(T::try_from_object(self.vm, value)),
Err(err) => {
let stop_ex = self.vm.ctx.exceptions.stop_iteration.clone();
let stop = match objtype::real_isinstance(self.vm, &err, &stop_ex) {
Ok(stop) => stop,
Err(e) => {
return Some(Err(e));
}
};
if stop {
if objtype::isinstance(&err, &stop_ex) {
None
} else {
Some(Err(err))
Expand Down
20 changes: 20 additions & 0 deletions vm/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,26 @@ impl VirtualMachine {
self.call_method(obj, "__repr__", vec![])
}

/// Determines if `obj` is an instance of `cls`, either directly, indirectly or virtually via
/// the __instancecheck__ magic method.
pub fn isinstance(&mut self, obj: &PyObjectRef, cls: &PyObjectRef) -> PyResult<bool> {
// cpython first does an exact check on the type, although documentation doesn't state that
// https://github.com/python/cpython/blob/a24107b04c1277e3c1105f98aff5bfa3a98b33a0/Objects/abstract.c#L2408
if Rc::ptr_eq(&obj.typ(), cls) {
Ok(true)
} else {
let ret = self.call_method(cls, "__instancecheck__", vec![obj.clone()])?;
objbool::boolval(self, ret)
}
}

/// Determines if `subclass` is a subclass of `cls`, either directly, indirectly or virtually
/// via the __subclasscheck__ magic method.
pub fn issubclass(&mut self, subclass: &PyObjectRef, cls: &PyObjectRef) -> PyResult<bool> {
let ret = self.call_method(cls, "__subclasscheck__", vec![subclass.clone()])?;
objbool::boolval(self, ret)
}

pub fn call_get_descriptor(&mut self, attr: PyObjectRef, obj: PyObjectRef) -> PyResult {
let attr_class = attr.typ();
if let Some(descriptor) = attr_class.get_attr("__get__") {
Expand Down