From 656dc21aa2d0c7186568e725b1e0374d56c994e6 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 2 Jul 2025 18:19:22 +0100 Subject: [PATCH 1/2] [mypyc] Document some of our inheritance conventions Our policy wasn't clear from just reading the code. --- mypyc/ir/ops.py | 20 +++++++++++++++++--- mypyc/ir/rtypes.py | 15 +++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/mypyc/ir/ops.py b/mypyc/ir/ops.py index eec9c34a965e..870497acc3a7 100644 --- a/mypyc/ir/ops.py +++ b/mypyc/ir/ops.py @@ -3,10 +3,24 @@ Opcodes operate on abstract values (Value) in a register machine. Each value has a type (RType). A value can hold various things, such as: -- local variables (Register) +- local variables or temporaries (Register) - intermediate values of expressions (RegisterOp subclasses) - condition flags (true/false) - literals (integer literals, True, False, etc.) + +NOTE: As a convention, we don't create subclasses of concrete Value/Op + subclasses (e.g. you shouldn't define a subclass of Integer, which + is a concrete class). + + If you want to introduce a variant of an existing class, you'd + typically add an attribute (e.g. a flag) to an existing concrete + class to enable the new behavior. Sometimes adding a new abstract + base class is also an option, or just creating a new subclass + without any inheritance relationship (some duplication of code + is preferred over introducing complex implementation inheritance). + + This makes it possible to use isinstance(x, ) checks without worrying about potential subclasses. """ from __future__ import annotations @@ -257,7 +271,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: class BaseAssign(Op): - """Base class for ops that assign to a register.""" + """Abstract base class for ops that assign to a register.""" def __init__(self, dest: Register, line: int = -1) -> None: super().__init__(line) @@ -320,7 +334,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: class ControlOp(Op): - """Control flow operation.""" + """Abstract base class for control flow operations.""" def targets(self) -> Sequence[BasicBlock]: """Get all basic block targets of the control operation.""" diff --git a/mypyc/ir/rtypes.py b/mypyc/ir/rtypes.py index 60a56065006f..b806479f14a1 100644 --- a/mypyc/ir/rtypes.py +++ b/mypyc/ir/rtypes.py @@ -18,6 +18,21 @@ mypyc.irbuild.mapper.Mapper.type_to_rtype converts mypy Types to mypyc RTypes. + +NOTE: As a convention, we don't create subclasses of concrete RType + subclasses (e.g. you shouldn't define a subclass of RTuple, which + is a concrete class). We prefer a flat class hierarchy. + + If you want to introduce a variant of an existing class, you'd + typically add an attribute (e.g. a flag) to an existing concrete + class to enable the new behavior. In rare cases, adding a new + abstract base class could also be an option. Adding a completely + separate class and sharing some functionality using module-level + helper functions may also be reasonable. + + This makes it possible to use isinstance(x, ) checks without worrying about potential subclasses + and avoids most trouble caused by implementation inheritance. """ from __future__ import annotations From a1cc9c3c38e24434b1def4b10df468f6541b66b5 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 3 Jul 2025 11:30:05 +0100 Subject: [PATCH 2/2] Use @final for Value and RType subclasses that shouldn't be subclassed --- mypyc/ir/ops.py | 45 ++++++++++++++++++++++++++++++++++++++++++++- mypyc/ir/rtypes.py | 9 ++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/mypyc/ir/ops.py b/mypyc/ir/ops.py index 870497acc3a7..355470955a5d 100644 --- a/mypyc/ir/ops.py +++ b/mypyc/ir/ops.py @@ -27,7 +27,7 @@ class to enable the new behavior. Sometimes adding a new abstract from abc import abstractmethod from collections.abc import Sequence -from typing import TYPE_CHECKING, Final, Generic, NamedTuple, TypeVar, Union +from typing import TYPE_CHECKING, Final, Generic, NamedTuple, TypeVar, Union, final from mypy_extensions import trait @@ -61,6 +61,7 @@ class to enable the new behavior. Sometimes adding a new abstract T = TypeVar("T") +@final class BasicBlock: """IR basic block. @@ -156,6 +157,7 @@ def is_void(self) -> bool: return isinstance(self.type, RVoid) +@final class Register(Value): """A Register holds a value of a specific type, and it can be read and mutated. @@ -182,6 +184,7 @@ def __repr__(self) -> str: return f"" +@final class Integer(Value): """Short integer literal. @@ -212,6 +215,7 @@ def numeric_value(self) -> int: return self.value +@final class Float(Value): """Float literal. @@ -278,6 +282,7 @@ def __init__(self, dest: Register, line: int = -1) -> None: self.dest = dest +@final class Assign(BaseAssign): """Assign a value to a Register (dest = src).""" @@ -300,6 +305,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_assign(self) +@final class AssignMulti(BaseAssign): """Assign multiple values to a Register (dest = src1, src2, ...). @@ -345,6 +351,7 @@ def set_target(self, i: int, new: BasicBlock) -> None: raise AssertionError(f"Invalid set_target({self}, {i})") +@final class Goto(ControlOp): """Unconditional jump.""" @@ -374,6 +381,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_goto(self) +@final class Branch(ControlOp): """Branch based on a value. @@ -440,6 +448,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_branch(self) +@final class Return(ControlOp): """Return a value from a function.""" @@ -469,6 +478,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_return(self) +@final class Unreachable(ControlOp): """Mark the end of basic block as unreachable. @@ -525,6 +535,7 @@ def can_raise(self) -> bool: return self.error_kind != ERR_NEVER +@final class IncRef(RegisterOp): """Increase reference count (inc_ref src).""" @@ -545,6 +556,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_inc_ref(self) +@final class DecRef(RegisterOp): """Decrease reference count and free object if zero (dec_ref src). @@ -573,6 +585,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_dec_ref(self) +@final class Call(RegisterOp): """Native call f(arg, ...). @@ -601,6 +614,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_call(self) +@final class MethodCall(RegisterOp): """Native method call obj.method(arg, ...)""" @@ -632,6 +646,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_method_call(self) +@final class PrimitiveDescription: """Description of a primitive op. @@ -684,6 +699,7 @@ def __repr__(self) -> str: return f"" +@final class PrimitiveOp(RegisterOp): """A higher-level primitive operation. @@ -721,6 +737,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_primitive_op(self) +@final class LoadErrorValue(RegisterOp): """Load an error value. @@ -751,6 +768,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_load_error_value(self) +@final class LoadLiteral(RegisterOp): """Load a Python literal object (dest = 'foo' / b'foo' / ...). @@ -786,6 +804,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_load_literal(self) +@final class GetAttr(RegisterOp): """obj.attr (for a native object)""" @@ -813,6 +832,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_get_attr(self) +@final class SetAttr(RegisterOp): """obj.attr = src (for a native object) @@ -864,6 +884,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: NAMESPACE_TYPE_VAR: Final = "typevar" +@final class LoadStatic(RegisterOp): """Load a static name (name :: static). @@ -904,6 +925,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_load_static(self) +@final class InitStatic(RegisterOp): """static = value :: static @@ -936,6 +958,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_init_static(self) +@final class TupleSet(RegisterOp): """dest = (reg, ...) (for fixed-length tuple)""" @@ -968,6 +991,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_tuple_set(self) +@final class TupleGet(RegisterOp): """Get item of a fixed-length tuple (src[index]).""" @@ -992,6 +1016,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_tuple_get(self) +@final class Cast(RegisterOp): """cast(type, src) @@ -1023,6 +1048,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_cast(self) +@final class Box(RegisterOp): """box(type, src) @@ -1057,6 +1083,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_box(self) +@final class Unbox(RegisterOp): """unbox(type, src) @@ -1083,6 +1110,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_unbox(self) +@final class RaiseStandardError(RegisterOp): """Raise built-in exception with an optional error string. @@ -1122,6 +1150,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: StealsDescription = Union[bool, list[bool]] +@final class CallC(RegisterOp): """result = function(arg0, arg1, ...) @@ -1176,6 +1205,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_call_c(self) +@final class Truncate(RegisterOp): """result = truncate src from src_type to dst_type @@ -1206,6 +1236,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_truncate(self) +@final class Extend(RegisterOp): """result = extend src from src_type to dst_type @@ -1240,6 +1271,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_extend(self) +@final class LoadGlobal(RegisterOp): """Load a low-level global variable/pointer. @@ -1267,6 +1299,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_load_global(self) +@final class IntOp(RegisterOp): """Binary arithmetic or bitwise op on integer operands (e.g., r1 = r2 + r3). @@ -1331,6 +1364,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: int_op_to_id: Final = {op: op_id for op_id, op in IntOp.op_str.items()} +@final class ComparisonOp(RegisterOp): """Low-level comparison op for integers and pointers. @@ -1392,6 +1426,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_comparison_op(self) +@final class FloatOp(RegisterOp): """Binary float arithmetic op (e.g., r1 = r2 + r3). @@ -1433,6 +1468,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: float_op_to_id: Final = {op: op_id for op_id, op in FloatOp.op_str.items()} +@final class FloatNeg(RegisterOp): """Float negation op (r1 = -r2).""" @@ -1453,6 +1489,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_float_neg(self) +@final class FloatComparisonOp(RegisterOp): """Low-level comparison op for floats.""" @@ -1489,6 +1526,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: float_comparison_op_to_id: Final = {op: op_id for op_id, op in FloatComparisonOp.op_str.items()} +@final class LoadMem(RegisterOp): """Read a memory location: result = *(type *)src. @@ -1518,6 +1556,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_load_mem(self) +@final class SetMem(Op): """Write to a memory location: *(type *)dest = src @@ -1549,6 +1588,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_set_mem(self) +@final class GetElementPtr(RegisterOp): """Get the address of a struct element. @@ -1575,6 +1615,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_get_element_ptr(self) +@final class LoadAddress(RegisterOp): """Get the address of a value: result = (type)&src @@ -1609,6 +1650,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_load_address(self) +@final class KeepAlive(RegisterOp): """A no-op operation that ensures source values aren't freed. @@ -1656,6 +1698,7 @@ def accept(self, visitor: OpVisitor[T]) -> T: return visitor.visit_keep_alive(self) +@final class Unborrow(RegisterOp): """A no-op op to create a regular reference from a borrowed one. diff --git a/mypyc/ir/rtypes.py b/mypyc/ir/rtypes.py index b806479f14a1..61aadce9b9d4 100644 --- a/mypyc/ir/rtypes.py +++ b/mypyc/ir/rtypes.py @@ -38,7 +38,7 @@ class to enable the new behavior. In rare cases, adding a new from __future__ import annotations from abc import abstractmethod -from typing import TYPE_CHECKING, ClassVar, Final, Generic, TypeVar +from typing import TYPE_CHECKING, ClassVar, Final, Generic, TypeVar, final from typing_extensions import TypeGuard from mypyc.common import HAVE_IMMORTAL, IS_32_BIT_PLATFORM, PLATFORM_SIZE, JsonDict, short_name @@ -170,6 +170,7 @@ def visit_rvoid(self, typ: RVoid, /) -> T: raise NotImplementedError +@final class RVoid(RType): """The void type (no value). @@ -202,6 +203,7 @@ def __hash__(self) -> int: void_rtype: Final = RVoid() +@final class RPrimitive(RType): """Primitive type such as 'object' or 'int'. @@ -665,6 +667,7 @@ def visit_rvoid(self, t: RVoid) -> str: assert False, "rvoid in tuple?" +@final class RTuple(RType): """Fixed-length unboxed tuple (represented as a C struct). @@ -806,6 +809,7 @@ def compute_aligned_offsets_and_size(types: list[RType]) -> tuple[list[int], int return offsets, final_size +@final class RStruct(RType): """C struct type""" @@ -859,6 +863,7 @@ def deserialize(cls, data: JsonDict, ctx: DeserMaps) -> RStruct: assert False +@final class RInstance(RType): """Instance of user-defined class (compiled to C extension class). @@ -919,6 +924,7 @@ def serialize(self) -> str: return self.name +@final class RUnion(RType): """union[x, ..., y]""" @@ -1009,6 +1015,7 @@ def is_optional_type(rtype: RType) -> bool: return optional_value_type(rtype) is not None +@final class RArray(RType): """Fixed-length C array type (for example, int[5]).