Skip to content

Commit 87576c6

Browse files
[mypyc] Fix shadow vtable misalignment for @property getters/setters (#21010)
Two bugs fixed in classes with `allow_interpreted_subclasses=True`: 1. Shadow vtable missing `getter` glue methods: When a property has both a `getter` and `setter`, the shadow glue for both was stored with `fdef.name` as key, causing the setter's glue to overwrite the getter's. Fix: Use `func_ir.decl.name` (unique per method, e.g. `"prop"` vs `"__mypyc_setter__prop"`) as the key. 2. Property`setter` shadow glue calling internal method name: The generic `gen_glue_method` was used for setter shadow glue, which tried to call `__mypyc_setter__<name>` via the Python API. This internal name doesn't exist on interpreted subclasses. Fix: add `gen_glue_property_setter` that uses Python's `setattr` to go through the standard descriptor protocol. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 5369080 commit 87576c6

File tree

3 files changed

+154
-3
lines changed

3 files changed

+154
-3
lines changed

mypyc/irbuild/function.py

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,11 @@ def handle_ext_method(builder: IRBuilder, cdef: ClassDef, fdef: FuncDef) -> None
540540
# children.
541541
if class_ir.allow_interpreted_subclasses:
542542
f = gen_glue(builder, func_ir.sig, func_ir, class_ir, class_ir, fdef, do_py_ops=True)
543-
class_ir.glue_methods[(class_ir, name)] = f
543+
# Use func_ir.decl.name (unique) rather than fdef.name, because for properties
544+
# the getter and setter share the same fdef.name but have distinct decl names
545+
# (e.g. "prop" vs "__mypyc_setter__prop"). Using fdef.name would cause the
546+
# setter's glue to overwrite the getter's glue in the shadow vtable.
547+
class_ir.glue_methods[(class_ir, func_ir.decl.name)] = f
544548
builder.functions.append(f)
545549

546550
if fdef.name == "__getattr__":
@@ -653,8 +657,9 @@ def gen_glue(
653657
"""
654658
if fdef.is_property:
655659
return gen_glue_property(builder, base_sig, target, cls, base, fdef.line, do_py_ops)
656-
else:
657-
return gen_glue_method(builder, base_sig, target, cls, base, fdef.line, do_py_ops)
660+
if do_py_ops and target.name.startswith(PROPSET_PREFIX):
661+
return gen_glue_property_setter(builder, base_sig, target, cls, base, fdef.line)
662+
return gen_glue_method(builder, base_sig, target, cls, base, fdef.line, do_py_ops)
658663

659664

660665
class ArgInfo(NamedTuple):
@@ -846,6 +851,56 @@ def gen_glue_property(
846851
)
847852

848853

854+
def gen_glue_property_setter(
855+
builder: IRBuilder, sig: FuncSignature, target: FuncIR, cls: ClassIR, base: ClassIR, line: int
856+
) -> FuncIR:
857+
"""Generate a shadow glue method for a property setter.
858+
859+
For interpreted subclasses, property setters can't be called via the
860+
internal __mypyc_setter__<name> method. Instead, use Python's setattr
861+
to set the property via the standard descriptor protocol.
862+
"""
863+
builder.enter()
864+
builder.ret_types[-1] = sig.ret_type
865+
866+
rt_args = list(sig.args)
867+
rt_args[0] = RuntimeArg(sig.args[0].name, RInstance(cls))
868+
869+
arg_info = get_args(builder, rt_args, line)
870+
args = arg_info.args
871+
872+
self_arg = args[0]
873+
value_arg = args[1]
874+
875+
# Extract the property name from "__mypyc_setter__<name>"
876+
assert target.name.startswith(PROPSET_PREFIX)
877+
prop_name = target.name[len(PROPSET_PREFIX) :]
878+
879+
builder.primitive_op(
880+
py_setattr_op,
881+
[
882+
self_arg,
883+
builder.load_str(prop_name),
884+
builder.coerce(value_arg, object_rprimitive, line),
885+
],
886+
line,
887+
)
888+
retval = builder.coerce(builder.none(), sig.ret_type, line)
889+
builder.add(Return(retval))
890+
891+
arg_regs, _, blocks, return_type, _ = builder.leave()
892+
return FuncIR(
893+
FuncDecl(
894+
target.name + "__" + base.name + "_glue",
895+
cls.name,
896+
builder.module_name,
897+
FuncSignature(rt_args, return_type),
898+
),
899+
arg_regs,
900+
blocks,
901+
)
902+
903+
849904
def get_func_target(builder: IRBuilder, fdef: FuncDef) -> AssignmentTarget:
850905
"""Given a FuncDef, return the target for the instance of its callable class.
851906

mypyc/test-data/irbuild-glue-methods.test

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,3 +453,59 @@ class EE(E):
453453
main:7: error: An argument with type "i64" cannot be given a default value in a method override
454454
main:13: error: Incompatible argument type "i64" (base class has type "int")
455455
main:18: error: Incompatible argument type "int" (base class has type "i64")
456+
457+
[case testPropertySetterShadowGlue]
458+
import mypy_extensions
459+
460+
@mypy_extensions.mypyc_attr(allow_interpreted_subclasses=True)
461+
class Base:
462+
_x: str = ""
463+
464+
@property
465+
def x(self) -> str:
466+
return self._x
467+
468+
@x.setter
469+
def x(self, v: str) -> None:
470+
self._x = v
471+
[out]
472+
def Base.x(self):
473+
self :: __main__.Base
474+
r0 :: str
475+
L0:
476+
r0 = self._x
477+
return r0
478+
def Base.x__Base_glue(__mypyc_self__):
479+
__mypyc_self__ :: __main__.Base
480+
r0 :: str
481+
r1 :: object
482+
r2 :: str
483+
L0:
484+
r0 = 'x'
485+
r1 = CPyObject_GetAttr(__mypyc_self__, r0)
486+
r2 = cast(str, r1)
487+
return r2
488+
def Base.__mypyc_setter__x(self, v):
489+
self :: __main__.Base
490+
v :: str
491+
r0 :: bool
492+
L0:
493+
self._x = v; r0 = is_error
494+
return 1
495+
def Base.__mypyc_setter__x__Base_glue(self, v):
496+
self :: __main__.Base
497+
v, r0 :: str
498+
r1 :: i32
499+
r2 :: bit
500+
L0:
501+
r0 = 'x'
502+
r1 = PyObject_SetAttr(self, r0, v)
503+
r2 = r1 >= 0 :: signed
504+
return 1
505+
def Base.__mypyc_defaults_setup(__mypyc_self__):
506+
__mypyc_self__ :: __main__.Base
507+
r0 :: str
508+
L0:
509+
r0 = ''
510+
__mypyc_self__._x = r0
511+
return 1

mypyc/test-data/run-classes.test

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5774,3 +5774,43 @@ from native import Concrete
57745774
c = Concrete()
57755775
assert c.value() == 42
57765776
assert c.derived() == 42
5777+
5778+
[case testPropertyShadowVtableGlue]
5779+
from typing import List
5780+
import mypy_extensions
5781+
5782+
@mypy_extensions.mypyc_attr(allow_interpreted_subclasses=True)
5783+
class Base:
5784+
_x: str
5785+
_y: List[int]
5786+
def __init__(self) -> None:
5787+
self._x = ""
5788+
self._y = []
5789+
5790+
@property
5791+
def x(self) -> str:
5792+
return self._x
5793+
5794+
@x.setter
5795+
def x(self, v: str) -> None:
5796+
self._x = v
5797+
5798+
@property
5799+
def y(self) -> List[int]:
5800+
return self._y
5801+
5802+
@y.setter
5803+
def y(self, v: List[int]) -> None:
5804+
self._y = v
5805+
5806+
def method(self) -> str:
5807+
self.x = "a"
5808+
self.y = [1]
5809+
return self.x + str(len(self.y))
5810+
5811+
[file driver.py]
5812+
from native import Base
5813+
5814+
Sub = type("Sub", (Base,), {})
5815+
s = Sub()
5816+
assert s.method() == "a1"

0 commit comments

Comments
 (0)