Skip to content

Relocate PyObject_Generic* into object protocol #3662

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 3 commits into from
Apr 26, 2022
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
2 changes: 1 addition & 1 deletion vm/src/builtins/genericalias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ impl GetAttr for PyGenericAlias {
fn getattro(zelf: PyRef<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
for exc in &ATTR_EXCEPTIONS {
if *(*exc) == attr.to_string() {
return vm.generic_getattribute(zelf.as_object().to_owned(), attr);
return zelf.as_object().generic_getattr(attr, vm);
}
}
zelf.origin().get_attr(attr, vm)
Expand Down
8 changes: 5 additions & 3 deletions vm/src/builtins/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ impl PyModule {
}

fn getattr_inner(zelf: &Py<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
if let Some(attr) =
vm.generic_getattribute_opt(zelf.to_owned().into(), name.clone(), None)?
if let Some(attr) = zelf
.as_object()
.generic_getattr_opt(name.clone(), None, vm)?
{
return Ok(attr);
}
Expand All @@ -66,7 +67,8 @@ impl PyModule {
}

fn name(zelf: PyRef<Self>, vm: &VirtualMachine) -> Option<PyStrRef> {
vm.generic_getattribute_opt(zelf.into(), PyStr::from("__name__").into_ref(vm), None)
zelf.as_object()
.generic_getattr_opt(PyStr::from("__name__").into_ref(vm), None, vm)
.unwrap_or(None)
.and_then(|obj| obj.downcast::<PyStr>().ok())
}
Expand Down
54 changes: 4 additions & 50 deletions vm/src/builtins/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,13 @@ impl PyBaseObject {
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<()> {
generic_setattr(&obj, name, Some(value), vm)
obj.generic_setattr(name, Some(value), vm)
}

/// Implement delattr(self, name).
#[pymethod]
fn __delattr__(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
generic_setattr(&obj, name, None, vm)
obj.generic_setattr(name, None, vm)
}

#[pyslot]
Expand All @@ -178,7 +178,7 @@ impl PyBaseObject {
value: Option<PyObjectRef>,
vm: &VirtualMachine,
) -> PyResult<()> {
generic_setattr(&*obj, attr_name, value, vm)
obj.generic_setattr(attr_name, value, vm)
}

/// Return str(self).
Expand Down Expand Up @@ -288,7 +288,7 @@ impl PyBaseObject {
#[pyslot]
pub(crate) fn getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
vm_trace!("object.__getattribute__({:?}, {:?})", obj, name);
vm.generic_getattribute(obj, name)
obj.as_object().generic_getattr(name, vm)
}

#[pymethod(magic)]
Expand Down Expand Up @@ -330,52 +330,6 @@ pub fn object_set_dict(obj: PyObjectRef, dict: PyDictRef, vm: &VirtualMachine) -
.map_err(|_| vm.new_attribute_error("This object has no __dict__".to_owned()))
}

pub fn generic_getattr(obj: PyObjectRef, attr_name: PyStrRef, vm: &VirtualMachine) -> PyResult {
vm.generic_getattribute(obj, attr_name)
}

#[cfg_attr(feature = "flame-it", flame)]
pub fn generic_setattr(
obj: &PyObject,
attr_name: PyStrRef,
value: Option<PyObjectRef>,
vm: &VirtualMachine,
) -> PyResult<()> {
vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value);

if let Some(attr) = obj.get_class_attr(attr_name.as_str()) {
let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load());
if let Some(descriptor) = descr_set {
return descriptor(attr, obj.to_owned(), value, vm);
}
}

if let Some(dict) = obj.dict() {
if let Some(value) = value {
dict.set_item(attr_name, value, vm)?;
} else {
dict.del_item(attr_name.clone(), vm).map_err(|e| {
if e.fast_isinstance(&vm.ctx.exceptions.key_error) {
vm.new_attribute_error(format!(
"'{}' object has no attribute '{}'",
obj.class().name(),
attr_name,
))
} else {
e
}
})?;
}
Ok(())
} else {
Err(vm.new_attribute_error(format!(
"'{}' object has no attribute '{}'",
obj.class().name(),
attr_name,
)))
}
}

pub fn init(ctx: &Context) {
PyBaseObject::extend_class(ctx, &ctx.types.object_type);
}
Expand Down
2 changes: 1 addition & 1 deletion vm/src/builtins/super.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl PySuper {

impl GetAttr for PySuper {
fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
let skip = |zelf: PyRef<Self>, name| vm.generic_getattribute(zelf.into(), name);
let skip = |zelf: PyRef<Self>, name| zelf.as_object().generic_getattr(name, vm);
let (obj, start_type): (PyObjectRef, PyTypeRef) = match zelf.obj.clone() {
Some(o) => o,
None => return skip(zelf, name),
Expand Down
2 changes: 1 addition & 1 deletion vm/src/builtins/union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ impl GetAttr for PyUnion {
fn getattro(zelf: PyRef<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
for &exc in CLS_ATTRS {
if *exc == attr.to_string() {
return vm.generic_getattribute(zelf.as_object().to_owned(), attr);
return zelf.as_object().generic_getattr(attr, vm);
}
}
zelf.as_object().to_pyobject(vm).get_attr(attr, vm)
Expand Down
109 changes: 105 additions & 4 deletions vm/src/protocol/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

use crate::{
builtins::{
pystr::IntoPyStrRef, PyBytes, PyDict, PyGenericAlias, PyInt, PyStrRef, PyTupleRef,
PyTypeRef,
pystr::IntoPyStrRef, PyBytes, PyDict, PyDictRef, PyGenericAlias, PyInt, PyStrRef,
PyTupleRef, PyTypeRef,
},
bytesinner::ByteInnerNewOptions,
common::{hash::PyHash, str::to_ascii},
Expand Down Expand Up @@ -118,8 +118,6 @@ impl PyObject {
setattro(self, attr_name, attr_value, vm)
}

// PyObject *PyObject_GenericGetAttr(PyObject *o, PyObject *name)

pub fn set_attr(
&self,
attr_name: impl IntoPyStrRef,
Expand All @@ -131,6 +129,109 @@ impl PyObject {
}

// int PyObject_GenericSetAttr(PyObject *o, PyObject *name, PyObject *value)
#[cfg_attr(feature = "flame-it", flame)]
pub fn generic_setattr(
&self,
attr_name: PyStrRef,
value: Option<PyObjectRef>,
vm: &VirtualMachine,
) -> PyResult<()> {
vm_trace!("object.__setattr__({:?}, {}, {:?})", obj, attr_name, value);

if let Some(attr) = self.get_class_attr(attr_name.as_str()) {
let descr_set = attr.class().mro_find_map(|cls| cls.slots.descr_set.load());
if let Some(descriptor) = descr_set {
return descriptor(attr, self.to_owned(), value, vm);
}
}

if let Some(dict) = self.dict() {
if let Some(value) = value {
dict.set_item(attr_name, value, vm)?;
} else {
dict.del_item(attr_name.clone(), vm).map_err(|e| {
if e.fast_isinstance(&vm.ctx.exceptions.key_error) {
vm.new_attribute_error(format!(
"'{}' object has no attribute '{}'",
self.class().name(),
attr_name,
))
} else {
e
}
})?;
}
Ok(())
} else {
Err(vm.new_attribute_error(format!(
"'{}' object has no attribute '{}'",
self.class().name(),
attr_name,
)))
}
}

pub fn generic_getattr(&self, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
self.generic_getattr_opt(name.clone(), None, vm)?
.ok_or_else(|| vm.new_attribute_error(format!("{} has no attribute '{}'", self, name)))
}

/// CPython _PyObject_GenericGetAttrWithDict
pub fn generic_getattr_opt(
&self,
name_str: PyStrRef,
dict: Option<PyDictRef>,
vm: &VirtualMachine,
) -> PyResult<Option<PyObjectRef>> {
let name = name_str.as_str();
let obj_cls = self.class();
let cls_attr = match obj_cls.get_attr(name) {
Some(descr) => {
let descr_cls = descr.class();
let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load());
if let Some(descr_get) = descr_get {
if descr_cls
.mro_find_map(|cls| cls.slots.descr_set.load())
.is_some()
{
drop(descr_cls);
let cls = obj_cls.into_owned().into();
return descr_get(descr, Some(self.to_pyobject(vm)), Some(cls), vm)
.map(Some);
}
}
drop(descr_cls);
Some((descr, descr_get))
}
None => None,
};

let dict = dict.or_else(|| self.dict());

let attr = if let Some(dict) = dict {
dict.get_item_opt(name, vm)?
} else {
None
};

if let Some(obj_attr) = attr {
Ok(Some(obj_attr))
} else if let Some((attr, descr_get)) = cls_attr {
match descr_get {
Some(descr_get) => {
let cls = obj_cls.into_owned().into();
descr_get(attr, Some(self.to_pyobject(vm)), Some(cls), vm).map(Some)
}
None => Ok(Some(attr)),
}
} else if let Some(getter) = obj_cls.get_attr("__getattr__") {
drop(obj_cls);
vm.invoke(&getter, (self.to_pyobject(vm), name_str))
.map(Some)
} else {
Ok(None)
}
}

pub fn del_attr(&self, attr_name: impl IntoPyStrRef, vm: &VirtualMachine) -> PyResult<()> {
let attr_name = attr_name.into_pystr_ref(vm);
Expand Down
3 changes: 2 additions & 1 deletion vm/src/stdlib/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,8 @@ pub(crate) mod _thread {
if attr.as_str() == "__dict__" {
Ok(ldict.into())
} else {
vm.generic_getattribute_opt(zelf.clone().into(), attr.clone(), Some(ldict))?
zelf.as_object()
.generic_getattr_opt(attr.clone(), Some(ldict), vm)?
.ok_or_else(|| {
vm.new_attribute_error(format!(
"{} has no attribute '{}'",
Expand Down
1 change: 0 additions & 1 deletion vm/src/types/slot.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
pub use crate::builtins::object::{generic_getattr, generic_setattr};
use crate::common::{hash::PyHash, lock::PyRwLock};
use crate::{
builtins::{PyInt, PyStrRef, PyType, PyTypeRef},
Expand Down
63 changes: 1 addition & 62 deletions vm/src/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ mod vm_ops;
use crate::{
builtins::{
code::{self, PyCode},
object,
pystr::IntoPyStrRef,
tuple::{PyTuple, PyTupleTyped},
PyBaseExceptionRef, PyDictRef, PyList, PyModule, PyStrRef, PyTypeRef,
Expand Down Expand Up @@ -594,66 +593,6 @@ impl VirtualMachine {
Some(self.call_if_get_descriptor(method, obj))
}

pub fn generic_getattribute(&self, obj: PyObjectRef, name: PyStrRef) -> PyResult {
self.generic_getattribute_opt(obj.clone(), name.clone(), None)?
.ok_or_else(|| self.new_attribute_error(format!("{} has no attribute '{}'", obj, name)))
}

/// CPython _PyObject_GenericGetAttrWithDict
pub fn generic_getattribute_opt(
&self,
obj: PyObjectRef,
name_str: PyStrRef,
dict: Option<PyDictRef>,
) -> PyResult<Option<PyObjectRef>> {
let name = name_str.as_str();
let obj_cls = obj.class();
let cls_attr = match obj_cls.get_attr(name) {
Some(descr) => {
let descr_cls = descr.class();
let descr_get = descr_cls.mro_find_map(|cls| cls.slots.descr_get.load());
if let Some(descr_get) = descr_get {
if descr_cls
.mro_find_map(|cls| cls.slots.descr_set.load())
.is_some()
{
drop(descr_cls);
let cls = obj_cls.into_owned().into();
return descr_get(descr, Some(obj), Some(cls), self).map(Some);
}
}
drop(descr_cls);
Some((descr, descr_get))
}
None => None,
};

let dict = dict.or_else(|| obj.dict());

let attr = if let Some(dict) = dict {
dict.get_item_opt(name, self)?
} else {
None
};

if let Some(obj_attr) = attr {
Ok(Some(obj_attr))
} else if let Some((attr, descr_get)) = cls_attr {
match descr_get {
Some(descr_get) => {
let cls = obj_cls.into_owned().into();
descr_get(attr, Some(obj), Some(cls), self).map(Some)
}
None => Ok(Some(attr)),
}
} else if let Some(getter) = obj_cls.get_attr("__getattr__") {
drop(obj_cls);
self.invoke(&getter, (obj, name_str)).map(Some)
} else {
Ok(None)
}
}

pub fn is_callable(&self, obj: &PyObject) -> bool {
obj.class()
.mro_find_map(|cls| cls.slots.call.load())
Expand Down Expand Up @@ -741,6 +680,6 @@ impl VirtualMachine {
attr_value: impl Into<PyObjectRef>,
) -> PyResult<()> {
let val = attr_value.into();
object::generic_setattr(module, attr_name.into_pystr_ref(self), Some(val), self)
module.generic_setattr(attr_name.into_pystr_ref(self), Some(val), self)
}
}