Skip to content

Commit d9c81d9

Browse files
committed
Apply active builtin lookup consistently to method reductions [skip ci]
Preserve defining types and CPython lookup order, and use generic lookup once for builtin mapping subclasses. Assisted-by: OpenAI Codex:model identifier unavailable
1 parent 5e121f1 commit d9c81d9

5 files changed

Lines changed: 88 additions & 21 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: 8 additions & 6 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![
@@ -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: 76 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import _pickle
22
import io
33
import sys
4-
from types import MappingProxyType
4+
from types import MappingProxyType, MethodType
55

66
from testutils import assert_raises
77

@@ -37,17 +37,82 @@
3737
expected = f"{name} exceeds system's maximum size of {sys.maxsize} bytes"
3838
assert str(caught.exception) == expected
3939

40+
4041
# 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 = []
4151
replacement_getattr = object()
42-
for builtins_type in (dict, MappingProxyType):
43-
namespace = {
44-
"__builtins__": builtins_type({"getattr": replacement_getattr}),
45-
"descriptor": object.__str__,
46-
}
47-
exec("reduced = descriptor.__reduce__()", namespace)
48-
assert namespace["reduced"] == (replacement_getattr, (object, "__str__"))
49-
50-
namespace["__builtins__"] = builtins_type({})
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}
5195
with assert_raises(AttributeError) as caught:
5296
exec("descriptor.__reduce__()", namespace)
53-
assert caught.exception.args == ("getattr",)
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)