Skip to content

Commit

Permalink
[mypyc] Fix glue methods with native int types (#13939)
Browse files Browse the repository at this point in the history
This fixes handling of bitmap arguments used to track default values of
arguments with native int types in method overrides.

The main use case is a method override that adds a native int argument
with a default value to the base class signature.

Also generate an error if we don't support generating a glue method for
an override. This can happen if the positions of bits in the bitmap
would change in the subclass. We also don't support switching between
error values and default value bitmaps in glue methods. These could be
supported, but it would be complicated and doesn't seem worth it.

Work on mypyc/mypyc#837.
  • Loading branch information
JukkaL authored Oct 25, 2022
1 parent cf705d7 commit ec149da
Show file tree
Hide file tree
Showing 7 changed files with 266 additions and 13 deletions.
8 changes: 8 additions & 0 deletions mypyc/ir/func_ir.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ class FuncSignature:
def __init__(self, args: Sequence[RuntimeArg], ret_type: RType) -> None:
self.args = tuple(args)
self.ret_type = ret_type
# Bitmap arguments are use to mark default values for arguments that
# have types with overlapping error values.
self.num_bitmap_args = num_bitmap_args(self.args)
if self.num_bitmap_args:
extra = [
Expand All @@ -78,6 +80,12 @@ def __init__(self, args: Sequence[RuntimeArg], ret_type: RType) -> None:
]
self.args = self.args + tuple(reversed(extra))

def real_args(self) -> tuple[RuntimeArg, ...]:
"""Return arguments without any synthetic bitmap arguments."""
if self.num_bitmap_args:
return self.args[: -self.num_bitmap_args]
return self.args

def bound_sig(self) -> "FuncSignature":
if self.num_bitmap_args:
return FuncSignature(self.args[1 : -self.num_bitmap_args], self.ret_type)
Expand Down
62 changes: 52 additions & 10 deletions mypyc/irbuild/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
from mypyc.primitives.generic_ops import py_setattr_op
from mypyc.primitives.misc_ops import register_function
from mypyc.primitives.registry import builtin_names
from mypyc.sametype import is_same_method_signature
from mypyc.sametype import is_same_method_signature, is_same_type

# Top-level transform functions

Expand Down Expand Up @@ -548,7 +548,7 @@ def is_decorated(builder: IRBuilder, fdef: FuncDef) -> bool:

def gen_glue(
builder: IRBuilder,
sig: FuncSignature,
base_sig: FuncSignature,
target: FuncIR,
cls: ClassIR,
base: ClassIR,
Expand All @@ -566,9 +566,9 @@ def gen_glue(
"shadow" glue methods that work with interpreted subclasses.
"""
if fdef.is_property:
return gen_glue_property(builder, sig, target, cls, base, fdef.line, do_py_ops)
return gen_glue_property(builder, base_sig, target, cls, base, fdef.line, do_py_ops)
else:
return gen_glue_method(builder, sig, target, cls, base, fdef.line, do_py_ops)
return gen_glue_method(builder, base_sig, target, cls, base, fdef.line, do_py_ops)


class ArgInfo(NamedTuple):
Expand All @@ -594,7 +594,7 @@ def get_args(builder: IRBuilder, rt_args: Sequence[RuntimeArg], line: int) -> Ar

def gen_glue_method(
builder: IRBuilder,
sig: FuncSignature,
base_sig: FuncSignature,
target: FuncIR,
cls: ClassIR,
base: ClassIR,
Expand Down Expand Up @@ -626,16 +626,25 @@ def f(builder: IRBuilder, x: object) -> int: ...
If do_pycall is True, then make the call using the C API
instead of a native call.
"""
check_native_override(builder, base_sig, target.decl.sig, line)

builder.enter()
builder.ret_types[-1] = sig.ret_type
builder.ret_types[-1] = base_sig.ret_type

rt_args = list(sig.args)
rt_args = list(base_sig.args)
if target.decl.kind == FUNC_NORMAL:
rt_args[0] = RuntimeArg(sig.args[0].name, RInstance(cls))
rt_args[0] = RuntimeArg(base_sig.args[0].name, RInstance(cls))

arg_info = get_args(builder, rt_args, line)
args, arg_kinds, arg_names = arg_info.args, arg_info.arg_kinds, arg_info.arg_names

bitmap_args = None
if base_sig.num_bitmap_args:
args = args[: -base_sig.num_bitmap_args]
arg_kinds = arg_kinds[: -base_sig.num_bitmap_args]
arg_names = arg_names[: -base_sig.num_bitmap_args]
bitmap_args = builder.builder.args[-base_sig.num_bitmap_args :]

# We can do a passthrough *args/**kwargs with a native call, but if the
# args need to get distributed out to arguments, we just let python handle it
if any(kind.is_star() for kind in arg_kinds) and any(
Expand All @@ -655,11 +664,15 @@ def f(builder: IRBuilder, x: object) -> int: ...
first, target.name, args[st:], line, arg_kinds[st:], arg_names[st:]
)
else:
retval = builder.builder.call(target.decl, args, arg_kinds, arg_names, line)
retval = builder.coerce(retval, sig.ret_type, line)
retval = builder.builder.call(
target.decl, args, arg_kinds, arg_names, line, bitmap_args=bitmap_args
)
retval = builder.coerce(retval, base_sig.ret_type, line)
builder.add(Return(retval))

arg_regs, _, blocks, ret_type, _ = builder.leave()
if base_sig.num_bitmap_args:
rt_args = rt_args[: -base_sig.num_bitmap_args]
return FuncIR(
FuncDecl(
target.name + "__" + base.name + "_glue",
Expand All @@ -673,6 +686,35 @@ def f(builder: IRBuilder, x: object) -> int: ...
)


def check_native_override(
builder: IRBuilder, base_sig: FuncSignature, sub_sig: FuncSignature, line: int
) -> None:
"""Report an error if an override changes signature in unsupported ways.
Glue methods can work around many signature changes but not all of them.
"""
for base_arg, sub_arg in zip(base_sig.real_args(), sub_sig.real_args()):
if base_arg.type.error_overlap:
if not base_arg.optional and sub_arg.optional and base_sig.num_bitmap_args:
# This would change the meanings of bits in the argument defaults
# bitmap, which we don't support. We'd need to do tricky bit
# manipulations to support this generally.
builder.error(
"An argument with type "
+ f'"{base_arg.type}" cannot be given a default value in a method override',
line,
)
if base_arg.type.error_overlap or sub_arg.type.error_overlap:
if not is_same_type(base_arg.type, sub_arg.type):
# This would change from signaling a default via an error value to
# signaling a default via bitmap, which we don't support.
builder.error(
"Incompatible argument type "
+ f'"{sub_arg.type}" (base class has type "{base_arg.type}")',
line,
)


def gen_glue_property(
builder: IRBuilder,
sig: FuncSignature,
Expand Down
20 changes: 18 additions & 2 deletions mypyc/irbuild/ll_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -935,10 +935,19 @@ def call(
arg_kinds: list[ArgKind],
arg_names: Sequence[str | None],
line: int,
*,
bitmap_args: list[Register] | None = None,
) -> Value:
"""Call a native function."""
"""Call a native function.
If bitmap_args is given, they override the values of (some) of the bitmap
arguments used to track the presence of values for certain arguments. By
default, the values of the bitmap arguments are inferred from args.
"""
# Normalize args to positionals.
args = self.native_args_to_positional(args, arg_kinds, arg_names, decl.sig, line)
args = self.native_args_to_positional(
args, arg_kinds, arg_names, decl.sig, line, bitmap_args=bitmap_args
)
return self.add(Call(decl, args, line))

def native_args_to_positional(
Expand All @@ -948,6 +957,8 @@ def native_args_to_positional(
arg_names: Sequence[str | None],
sig: FuncSignature,
line: int,
*,
bitmap_args: list[Register] | None = None,
) -> list[Value]:
"""Prepare arguments for a native call.
Expand Down Expand Up @@ -1015,6 +1026,11 @@ def native_args_to_positional(
output_args.append(output_arg)

for i in reversed(range(n)):
if bitmap_args and i < len(bitmap_args):
# Use override provided by caller
output_args.append(bitmap_args[i])
continue
# Infer values of bitmap args
bitmap = 0
c = 0
for lst, arg in zip(formal_to_actual, sig_args):
Expand Down
2 changes: 1 addition & 1 deletion mypyc/sametype.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def is_same_method_signature(a: FuncSignature, b: FuncSignature) -> bool:
len(a.args) == len(b.args)
and is_same_type(a.ret_type, b.ret_type)
and all(
is_same_type(t1.type, t2.type) and t1.name == t2.name
is_same_type(t1.type, t2.type) and t1.name == t2.name and t1.optional == t2.optional
for t1, t2 in zip(a.args[1:], b.args[1:])
)
)
Expand Down
108 changes: 108 additions & 0 deletions mypyc/test-data/irbuild-glue-methods.test
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,111 @@ L0:
r0 = __mypyc_self__.boxed
r1 = box(int, r0)
return r1

[case testI64GlueWithExtraDefaultArg]
from mypy_extensions import i64

class C:
def f(self) -> None: pass

class D(C):
def f(self, x: i64 = 44) -> None: pass
[out]
def C.f(self):
self :: __main__.C
L0:
return 1
def D.f(self, x, __bitmap):
self :: __main__.D
x :: int64
__bitmap, r0 :: uint32
r1 :: bit
L0:
r0 = __bitmap & 1
r1 = r0 == 0
if r1 goto L1 else goto L2 :: bool
L1:
x = 44
L2:
return 1
def D.f__C_glue(self):
self :: __main__.D
r0 :: None
L0:
r0 = D.f(self, 0, 0)
return r0

[case testI64GlueWithSecondDefaultArg]
from mypy_extensions import i64

class C:
def f(self, x: i64 = 11) -> None: pass
class D(C):
def f(self, x: i64 = 12, y: i64 = 13) -> None: pass
[out]
def C.f(self, x, __bitmap):
self :: __main__.C
x :: int64
__bitmap, r0 :: uint32
r1 :: bit
L0:
r0 = __bitmap & 1
r1 = r0 == 0
if r1 goto L1 else goto L2 :: bool
L1:
x = 11
L2:
return 1
def D.f(self, x, y, __bitmap):
self :: __main__.D
x, y :: int64
__bitmap, r0 :: uint32
r1 :: bit
r2 :: uint32
r3 :: bit
L0:
r0 = __bitmap & 1
r1 = r0 == 0
if r1 goto L1 else goto L2 :: bool
L1:
x = 12
L2:
r2 = __bitmap & 2
r3 = r2 == 0
if r3 goto L3 else goto L4 :: bool
L3:
y = 13
L4:
return 1
def D.f__C_glue(self, x, __bitmap):
self :: __main__.D
x :: int64
__bitmap :: uint32
r0 :: None
L0:
r0 = D.f(self, x, 0, __bitmap)
return r0

[case testI64GlueWithInvalidOverride]
from mypy_extensions import i64

class C:
def f(self, x: i64, y: i64 = 5) -> None: pass
def ff(self, x: int) -> None: pass
class CC(C):
def f(self, x: i64 = 12, y: i64 = 5) -> None: pass # Line 7
def ff(self, x: int = 12) -> None: pass

class D:
def f(self, x: int) -> None: pass
class DD(D):
def f(self, x: i64) -> None: pass # Line 13

class E:
def f(self, x: i64) -> None: pass
class EE(E):
def f(self, x: int) -> None: pass # Line 18
[out]
main:7: error: An argument with type "int64" cannot be given a default value in a method override
main:13: error: Incompatible argument type "int64" (base class has type "int")
main:18: error: Incompatible argument type "int" (base class has type "int64")
35 changes: 35 additions & 0 deletions mypyc/test-data/irbuild-i64.test
Original file line number Diff line number Diff line change
Expand Up @@ -1628,3 +1628,38 @@ L3:
goto L1
L4:
return 1

[case testI64MethodDefaultValueOverride]
from mypy_extensions import i64

class C:
def f(self, x: i64 = 11) -> None: pass
class D(C):
def f(self, x: i64 = 12) -> None: pass
[out]
def C.f(self, x, __bitmap):
self :: __main__.C
x :: int64
__bitmap, r0 :: uint32
r1 :: bit
L0:
r0 = __bitmap & 1
r1 = r0 == 0
if r1 goto L1 else goto L2 :: bool
L1:
x = 11
L2:
return 1
def D.f(self, x, __bitmap):
self :: __main__.D
x :: int64
__bitmap, r0 :: uint32
r1 :: bit
L0:
r0 = __bitmap & 1
r1 = r0 == 0
if r1 goto L1 else goto L2 :: bool
L1:
x = 12
L2:
return 1
44 changes: 44 additions & 0 deletions mypyc/test-data/run-i64.test
Original file line number Diff line number Diff line change
Expand Up @@ -1126,3 +1126,47 @@ def test_many_locals() -> None:
assert a31 == 10
assert a32 == 55
assert a33 == 20

[case testI64GlueMethods]
from typing_extensions import Final

MYPY = False
if MYPY:
from mypy_extensions import i64

MAGIC: Final = -113

class Base:
def foo(self) -> i64:
return 5

def bar(self, x: i64 = 2) -> i64:
return x + 1

def hoho(self, x: i64) -> i64:
return x - 1

class Derived(Base):
def foo(self, x: i64 = 5) -> i64:
return x + 10

def bar(self, x: i64 = 3, y: i64 = 20) -> i64:
return x + y + 2

def hoho(self, x: i64 = 7) -> i64:
return x - 2

def test_derived_adds_bitmap() -> None:
b: Base = Derived()
assert b.foo() == 15

def test_derived_adds_another_default_arg() -> None:
b: Base = Derived()
assert b.bar() == 25
assert b.bar(1) == 23
assert b.bar(MAGIC) == MAGIC + 22

def test_derived_switches_arg_to_have_default() -> None:
b: Base = Derived()
assert b.hoho(5) == 3
assert b.hoho(MAGIC) == MAGIC - 2

0 comments on commit ec149da

Please sign in to comment.