Skip to content

Commit 1a1605d

Browse files
authored
Use active builtins in descriptor and method reductions (#9057)
Use frame-aware getattr lookup consistently across descriptor and method reductions. Preserve defining types and exception order, and perform generic builtin-mapping subclass lookup only once. Assisted-by: OpenAI Codex:model identifier unavailable
1 parent 9ea138c commit 1a1605d

5 files changed

Lines changed: 94 additions & 11 deletions

File tree

‎crates/vm/src/builtins/builtin_func.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ impl PyNativeFunction {
215215
.as_ref()
216216
.filter(|bound| !bound.class().is_subtype(vm.ctx.types.module_type))
217217
{
218-
let getattr = vm.builtins.get_attr("getattr", vm)?;
218+
let getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
219219
Ok(vm
220220
.new_tuple((getattr, (bound.clone(), zelf.value.name)))
221221
.into())

‎crates/vm/src/builtins/descriptor.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,12 @@ impl Py<PyMethodDescriptor> {
155155
fn __reduce__(
156156
&self,
157157
vm: &VirtualMachine,
158-
) -> (Option<PyObjectRef>, (Option<PyObjectRef>, &'static str)) {
159-
let builtins_getattr = vm.builtins.get_attr("getattr", vm).ok();
160-
let classname = vm.builtins.get_attr(&self.common.typ.__name__(vm), vm).ok();
161-
(builtins_getattr, (classname, self.method.name))
158+
) -> PyResult<(PyObjectRef, (PyTypeRef, &'static str))> {
159+
let builtins_getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
160+
Ok((
161+
builtins_getattr,
162+
(self.common.typ.to_owned(), self.method.name),
163+
))
162164
}
163165
}
164166

@@ -851,7 +853,7 @@ impl Py<PyMemberDescriptor> {
851853

852854
#[pymethod]
853855
fn __reduce__(&self, vm: &VirtualMachine) -> PyResult {
854-
let builtins_getattr = vm.builtins.get_attr("getattr", vm)?;
856+
let builtins_getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
855857
Ok(vm
856858
.ctx
857859
.new_tuple(vec![
@@ -2064,7 +2066,7 @@ impl Callable for PyWrapper {
20642066
impl Py<PyWrapper> {
20652067
#[pymethod]
20662068
fn __reduce__(&self, vm: &VirtualMachine) -> PyResult {
2067-
let getattr = vm.builtins.get_attr("getattr", vm)?;
2069+
let getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
20682070
let args = vm.ctx.new_tuple(vec![
20692071
self.typ.to_owned().into(),
20702072
self.name.to_owned().into(),
@@ -2181,7 +2183,7 @@ impl Py<PyMethodWrapper> {
21812183

21822184
#[pymethod]
21832185
fn __reduce__(zelf: PyRef<PyMethodWrapper>, vm: &VirtualMachine) -> PyResult {
2184-
let builtins_getattr = vm.builtins.get_attr("getattr", vm)?;
2186+
let builtins_getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
21852187
Ok(vm
21862188
.ctx
21872189
.new_tuple(vec![

‎crates/vm/src/builtins/function.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,9 +1470,9 @@ impl Py<PyBoundMethod> {
14701470
&self,
14711471
vm: &VirtualMachine,
14721472
) -> PyResult<(PyObjectRef, (PyObjectRef, PyObjectRef))> {
1473-
let builtins_getattr = vm.builtins.get_attr("getattr", vm)?;
1474-
let func_self = self.object.clone();
14751473
let func_name = self.function.get_attr("__name__", vm)?;
1474+
let builtins_getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
1475+
let func_self = self.object.clone();
14761476
Ok((builtins_getattr, (func_self, func_name)))
14771477
}
14781478

‎crates/vm/src/vm/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3170,7 +3170,7 @@ impl VirtualMachine {
31703170
pub fn eval_get_builtin(&self, name: &'static PyStrInterned) -> PyResult {
31713171
let builtins =
31723172
crate::frame::current_builtins().unwrap_or_else(|| self.builtins.dict().into());
3173-
if let Some(dict) = builtins.downcast_ref::<PyDict>() {
3173+
if let Some(dict) = builtins.downcast_ref_if_exact::<PyDict>(self) {
31743174
match dict.get_item_opt(name, self)? {
31753175
Some(value) => Ok(value),
31763176
None => Err(self.new_attribute_error(name.to_string())),

‎extra_tests/snippets/stdlib_pickle.py‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import _pickle
22
import io
33
import sys
4+
from types import MappingProxyType, MethodType
45

56
from testutils import assert_raises
67

@@ -35,3 +36,83 @@
3536
else:
3637
expected = f"{name} exceeds system's maximum size of {sys.maxsize} bytes"
3738
assert str(caught.exception) == expected
39+
40+
41+
# Reduction returns getattr from the active frame's builtins without calling it.
42+
class ReductionOwner:
43+
__slots__ = ("slot",)
44+
45+
def method(self):
46+
pass
47+
48+
49+
owner = ReductionOwner()
50+
items = []
51+
replacement_getattr = object()
52+
for descriptor, receiver, name in (
53+
(object.__str__, object, "__str__"),
54+
(str.join, str, "join"),
55+
(io.BytesIO.getvalue, io.BytesIO, "getvalue"),
56+
(ReductionOwner.slot, ReductionOwner, "slot"),
57+
(owner.__str__, owner, "__str__"),
58+
(items.append, items, "append"),
59+
(dict.fromkeys, dict, "fromkeys"),
60+
(owner.method, owner, "method"),
61+
):
62+
for builtins_type in (dict, MappingProxyType):
63+
namespace = {
64+
"__builtins__": builtins_type({"getattr": replacement_getattr}),
65+
"descriptor": descriptor,
66+
}
67+
exec("reduced = descriptor.__reduce__()", namespace)
68+
assert namespace["reduced"] == (replacement_getattr, (receiver, name))
69+
70+
namespace["__builtins__"] = builtins_type({})
71+
with assert_raises(AttributeError) as caught:
72+
exec("descriptor.__reduce__()", namespace)
73+
assert caught.exception.args == ("getattr",)
74+
75+
# Module-bound functions reduce to a name without looking up getattr.
76+
namespace = {"__builtins__": {}, "function": len}
77+
exec("reduced = function.__reduce__()", namespace)
78+
assert namespace["reduced"] == "len"
79+
80+
81+
class MissingBuiltin(dict):
82+
def __getitem__(self, key):
83+
raise KeyError(key)
84+
85+
def __missing__(self, key):
86+
raise AssertionError("must not retry __missing__ after custom __getitem__")
87+
88+
89+
for descriptor, name in (
90+
(object.__str__, "getattr"),
91+
(iter([1]), "iter"),
92+
(reversed([1]), "reversed"),
93+
):
94+
namespace = {"__builtins__": MissingBuiltin(), "descriptor": descriptor}
95+
with assert_raises(AttributeError) as caught:
96+
exec("descriptor.__reduce__()", namespace)
97+
assert caught.exception.args == (name,)
98+
99+
100+
name_error = RuntimeError("function name first")
101+
102+
103+
class NamedCallable:
104+
def __call__(self, *args):
105+
pass
106+
107+
@property
108+
def __name__(self):
109+
raise name_error
110+
111+
112+
namespace = {
113+
"__builtins__": {},
114+
"method": MethodType(NamedCallable(), owner),
115+
}
116+
with assert_raises(RuntimeError) as caught:
117+
exec("method.__reduce__()", namespace)
118+
assert caught.exception is name_error

0 commit comments

Comments
 (0)