Skip to content

Typetype clean #660

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.

Sign up for GitHub

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 3 commits into from
Mar 11, 2019
Merged

Typetype clean #660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3711881
Add type_pyref() to TypeProtocol.
cthulahoops Mar 11, 2019
1e39512
Convert _mro to iterator.
cthulahoops Mar 11, 2019
633a9b0
Move more methods onto PyClassRef.
cthulahoops Mar 11, 2019
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
18 changes: 8 additions & 10 deletions vm/src/obj/objobject.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::objlist::PyList;
use super::objstr;
use super::objtype;
use crate::obj::objproperty::PropertyBuilder;
Expand Down Expand Up @@ -133,16 +134,13 @@ fn object_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
Ok(vm.new_str(format!("<{} object at 0x{:x}>", type_name, address)))
}

pub fn object_dir(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(vm, args, required = [(obj, None)]);

pub fn object_dir(obj: PyObjectRef, vm: &mut VirtualMachine) -> PyList {
let attributes = get_attributes(&obj);
Ok(vm.ctx.new_list(
attributes
.keys()
.map(|k| vm.ctx.new_str(k.to_string()))
.collect(),
))
let attributes: Vec<PyObjectRef> = attributes
.keys()
.map(|k| vm.ctx.new_str(k.to_string()))
.collect();
PyList::from(attributes)
}

fn object_format(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
Expand Down Expand Up @@ -260,7 +258,7 @@ fn object_getattribute(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {

pub fn get_attributes(obj: &PyObjectRef) -> PyAttributes {
// Get class attributes:
let mut attributes = objtype::get_attributes(&obj.typ());
let mut attributes = objtype::get_attributes(obj.type_pyref());

// Get instance attributes:
if let Some(dict) = &obj.dict {
Expand Down
155 changes: 84 additions & 71 deletions vm/src/obj/objtype.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ use std::cell::RefCell;
use std::collections::HashMap;

use crate::pyobject::{
AttributeProtocol, IdProtocol, PyAttributes, PyContext, PyFuncArgs, PyObject, PyObjectRef,
PyRef, PyResult, PyValue, TypeProtocol,
AttributeProtocol, FromPyObjectRef, IdProtocol, PyAttributes, PyContext, PyFuncArgs, PyObject,
PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
};
use crate::vm::VirtualMachine;

use super::objdict;
use super::objstr;
use super::objlist::PyList;
use super::objstr::{self, PyStringRef};
use super::objtuple::PyTuple;

#[derive(Clone, Debug)]
pub struct PyClass {
Expand All @@ -24,6 +26,70 @@ impl PyValue for PyClass {
}
}

struct IterMro<'a> {
cls: &'a PyClassRef,
offset: Option<usize>,
}

impl<'a> Iterator for IterMro<'a> {
type Item = &'a PyObjectRef;

fn next(&mut self) -> Option<Self::Item> {
match self.offset {
None => {
self.offset = Some(0);
Some(&self.cls.as_object())
}
Some(offset) => {
if offset < self.cls.mro.len() {
self.offset = Some(offset + 1);
Some(&self.cls.mro[offset])
} else {
None
}
}
}
}
}

impl PyClassRef {
fn iter_mro(&self) -> IterMro {
IterMro {
cls: self,
offset: None,
}
}

fn mro(self, _vm: &mut VirtualMachine) -> PyTuple {
PyTuple::from(_mro(&self))
}

fn dir(self, vm: &mut VirtualMachine) -> PyList {
let attributes = get_attributes(self);
let attributes: Vec<PyObjectRef> = attributes
.keys()
.map(|k| vm.ctx.new_str(k.to_string()))
.collect();
PyList::from(attributes)
}

fn instance_check(self, obj: PyObjectRef, _vm: &mut VirtualMachine) -> bool {
isinstance(&obj, self.as_object())
}

fn subclass_check(self, subclass: PyObjectRef, _vm: &mut VirtualMachine) -> bool {
issubclass(&subclass, self.as_object())
}

fn repr(self, _vm: &mut VirtualMachine) -> String {
format!("<class '{}'>", self.name)
}

fn prepare(_name: PyStringRef, _bases: PyObjectRef, vm: &mut VirtualMachine) -> PyObjectRef {
vm.new_dict()
}
}

/*
* The magical type type
*/
Expand All @@ -49,33 +115,19 @@ pub fn init(ctx: &PyContext) {
extend_class!(&ctx, &ctx.type_type, {
"__call__" => ctx.new_rustfunc(type_call),
"__new__" => ctx.new_rustfunc(type_new),
"__mro__" => ctx.new_property(type_mro),
"__repr__" => ctx.new_rustfunc(type_repr),
"__prepare__" => ctx.new_rustfunc(type_prepare),
"__mro__" => ctx.new_property(PyClassRef::mro),
"__repr__" => ctx.new_rustfunc(PyClassRef::repr),
"__prepare__" => ctx.new_rustfunc(PyClassRef::prepare),
"__getattribute__" => ctx.new_rustfunc(type_getattribute),
"__instancecheck__" => ctx.new_rustfunc(type_instance_check),
"__subclasscheck__" => ctx.new_rustfunc(type_subclass_check),
"__instancecheck__" => ctx.new_rustfunc(PyClassRef::instance_check),
"__subclasscheck__" => ctx.new_rustfunc(PyClassRef::subclass_check),
"__doc__" => ctx.new_str(type_doc.to_string()),
"__dir__" => ctx.new_rustfunc(type_dir),
"__dir__" => ctx.new_rustfunc(PyClassRef::dir),
});
}

fn type_mro(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(vm, args, required = [(cls, Some(vm.ctx.type_type()))]);
match _mro(cls.clone()) {
Some(mro) => Ok(vm.context().new_tuple(mro)),
None => Err(vm.new_type_error("Only classes have an MRO.".to_string())),
}
}

fn _mro(cls: PyObjectRef) -> Option<Vec<PyObjectRef>> {
if let Some(PyClass { ref mro, .. }) = cls.payload::<PyClass>() {
let mut mro = mro.clone();
mro.insert(0, cls.clone());
Some(mro)
} else {
None
}
fn _mro(cls: &PyClassRef) -> Vec<PyObjectRef> {
cls.iter_mro().cloned().collect()
}

/// Determines if `obj` actually an instance of `cls`, this doesn't call __instancecheck__, so only
Expand All @@ -84,15 +136,6 @@ pub fn isinstance(obj: &PyObjectRef, cls: &PyObjectRef) -> bool {
issubclass(obj.type_ref(), &cls)
}

fn type_instance_check(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(
vm,
args,
required = [(typ, Some(vm.ctx.type_type())), (obj, None)]
);
Ok(vm.new_bool(isinstance(obj, typ)))
}

/// Determines if `subclass` is actually a subclass of `cls`, this doesn't call __subclasscheck__,
/// so only use this if `cls` is known to have not overridden the base __subclasscheck__ magic
/// method.
Expand All @@ -101,18 +144,6 @@ pub fn issubclass(subclass: &PyObjectRef, cls: &PyObjectRef) -> bool {
subclass.is(&cls) || mro.iter().any(|c| c.is(&cls))
}

fn type_subclass_check(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(
vm,
args,
required = [
(cls, Some(vm.ctx.type_type())),
(subclass, Some(vm.ctx.type_type()))
]
);
Ok(vm.new_bool(issubclass(subclass, cls)))
}

pub fn get_type_name(typ: &PyObjectRef) -> String {
if let Some(PyClass { name, .. }) = &typ.payload::<PyClass>() {
name.clone()
Expand Down Expand Up @@ -212,24 +243,13 @@ pub fn type_getattribute(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult
}
}

pub fn type_dir(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(vm, args, required = [(obj, None)]);

let attributes = get_attributes(&obj);
Ok(vm.ctx.new_list(
attributes
.keys()
.map(|k| vm.ctx.new_str(k.to_string()))
.collect(),
))
}

pub fn get_attributes(obj: &PyObjectRef) -> PyAttributes {
pub fn get_attributes(cls: PyClassRef) -> PyAttributes {
// Gather all members here:
let mut attributes = PyAttributes::new();

let mut base_classes = _mro(obj.clone()).expect("Type get_attributes on non-type");
let mut base_classes: Vec<&PyObjectRef> = cls.iter_mro().collect();
base_classes.reverse();

for bc in base_classes {
if let Some(ref dict) = &bc.dict {
for (name, value) in dict.borrow().iter() {
Expand Down Expand Up @@ -294,7 +314,10 @@ pub fn new(
bases: Vec<PyObjectRef>,
dict: HashMap<String, PyObjectRef>,
) -> PyResult {
let mros = bases.into_iter().map(|x| _mro(x).unwrap()).collect();
let mros = bases
.into_iter()
.map(|x| _mro(&FromPyObjectRef::from_pyobj(&x)))
.collect();
let mro = linearise_mro(mros).unwrap();
Ok(PyObject {
payload: Box::new(PyClass {
Expand All @@ -307,16 +330,6 @@ pub fn new(
.into_ref())
}

fn type_repr(vm: &mut VirtualMachine, args: PyFuncArgs) -> PyResult {
arg_check!(vm, args, required = [(obj, Some(vm.ctx.type_type()))]);
let type_name = get_type_name(&obj);
Ok(vm.new_str(format!("<class '{}'>", type_name)))
}

fn type_prepare(vm: &mut VirtualMachine, _args: PyFuncArgs) -> PyResult {
Ok(vm.new_dict())
}

#[cfg(test)]
mod tests {
use super::{linearise_mro, new};
Expand Down
16 changes: 16 additions & 0 deletions vm/src/pyobject.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,9 @@ pub trait TypeProtocol {
fn typ(&self) -> PyObjectRef {
self.type_ref().clone()
}
fn type_pyref(&self) -> PyClassRef {
FromPyObjectRef::from_pyobj(self.type_ref())
}
fn type_ref(&self) -> &PyObjectRef;
}

Expand Down Expand Up @@ -1548,6 +1551,19 @@ pub trait PyValue: Any + fmt::Debug {
fn required_type(ctx: &PyContext) -> PyObjectRef;
}

impl FromPyObjectRef for PyRef<PyClass> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was planning on deleting FromPyObjectRef since it overlaps considerably with TryFromObject - could you use the latter instead? Should be more or less the same, just with the unwrap/panic on the caller instead (which we will eventually be able to get rid of).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get rid of by using !?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, from the trait's perspective, it can fail. I was referring to the fact that the mro could be a Vec of PyClassRef rather than PyObjectRef so that no downcasting is necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this might be the controversial bit.

I tried with TryFromObject but it required me to pass the VirtualMachine into a bunch of code that doesn't currently have a VM. The panic should never happen as an object's type should always be a type. I'd be happy with an interface that doesn't require VM but gives me an option/result to deal with?

Will I pass the VM through?

I suppose the alternative would be to make PyObject.typ a PyClassRef, but that's a pretty invasive change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I forget it takes a vm - we can work this out later then.

I'd be happy with an interface that doesn't require VM but gives me an option/result to deal with?

This will exist soon! (PyObjectRef::downcast in my prototype)

I suppose the alternative would be to make PyObject.typ a PyClassRef, but that's a pretty invasive change.

It is but I definitely think we should do this at some point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll merge. Once we have PyClassRef everywhere this will go away anyway.

fn from_pyobj(obj: &PyObjectRef) -> Self {
if let Some(_) = obj.payload::<PyClass>() {
PyRef {
obj: obj.clone(),
_payload: PhantomData,
}
} else {
panic!("Error getting inner type.")
}
}
}

#[cfg(test)]
mod tests {
use super::PyContext;
Expand Down