[mypyc] Refactor: make LoadMem not borrow by default (#19445)

Borrowing is a dangerous default. It can only be used in very specific
circumstances, so it shouldn't be the default. This is also arguably
more consistent with other read ops which don't borrow by default.

Author: JukkaL

mypyc/codegen/emitfunc.py

@@ -793,6 +793,8 @@ def visit_load_mem(self, op: LoadMem) -> None:
         # TODO: we shouldn't dereference to type that are pointer type so far
         type = self.ctype(op.type)
         self.emit_line(f"{dest} = *({type} *){src};")
+        if not op.is_borrowed:
+            self.emit_inc_ref(dest, op.type)
 
     def visit_set_mem(self, op: SetMem) -> None:
         dest = self.reg(op.dest)

mypyc/ir/ops.py

@@ -1579,14 +1579,13 @@ class LoadMem(RegisterOp):
 
     error_kind = ERR_NEVER
 
-    def __init__(self, type: RType, src: Value, line: int = -1) -> None:
+    def __init__(self, type: RType, src: Value, line: int = -1, *, borrow: bool = False) -> None:
         super().__init__(line)
         self.type = type
-        # TODO: for now we enforce that the src memory address should be Py_ssize_t
-        #       later we should also support same width unsigned int
+        # TODO: Support other native integer types
         assert is_pointer_rprimitive(src.type)
         self.src = src
-        self.is_borrowed = True
+        self.is_borrowed = borrow and type.is_refcounted
 
     def sources(self) -> list[Value]:
         return [self.src]

mypyc/ir/pprint.py

@@ -267,7 +267,9 @@ def visit_float_comparison_op(self, op: FloatComparisonOp) -> str:
         return self.format("%r = %r %s %r", op, op.lhs, op.op_str[op.op], op.rhs)
 
     def visit_load_mem(self, op: LoadMem) -> str:
-        return self.format("%r = load_mem %r :: %t*", op, op.src, op.type)
+        return self.format(
+            "%r = %sload_mem %r :: %t*", op, self.borrow_prefix(op), op.src, op.type
+        )
 
     def visit_set_mem(self, op: SetMem) -> str:
         return self.format("set_mem %r, %r :: %t*", op.dest, op.src, op.dest_type)

mypyc/irbuild/for_helpers.py

@@ -733,7 +733,7 @@ def gen_condition(self) -> None:
 
         def except_match() -> Value:
             addr = builder.add(LoadAddress(pointer_rprimitive, stop_async_iteration_op.src, line))
-            return builder.add(LoadMem(stop_async_iteration_op.type, addr))
+            return builder.add(LoadMem(stop_async_iteration_op.type, addr, borrow=True))
 
         def try_body() -> None:
             awaitable = builder.call_c(anext_op, [builder.read(self.iter_target)], line)

mypyc/irbuild/ll_builder.py

@@ -286,6 +286,9 @@ def goto_and_activate(self, block: BasicBlock) -> None:
     def keep_alive(self, values: list[Value], *, steal: bool = False) -> None:
         self.add(KeepAlive(values, steal=steal))
 
+    def load_mem(self, ptr: Value, value_type: RType, *, borrow: bool = False) -> Value:
+        return self.add(LoadMem(value_type, ptr, borrow=borrow))
+
     def push_error_handler(self, handler: BasicBlock | None) -> None:
         self.error_handlers.append(handler)
 
@@ -660,7 +663,7 @@ def other() -> Value:
 
     def get_type_of_obj(self, obj: Value, line: int) -> Value:
         ob_type_address = self.add(GetElementPtr(obj, PyObject, "ob_type", line))
-        ob_type = self.add(LoadMem(object_rprimitive, ob_type_address))
+        ob_type = self.load_mem(ob_type_address, object_rprimitive, borrow=True)
         self.add(KeepAlive([obj]))
         return ob_type
 
@@ -2261,7 +2264,7 @@ def builtin_len(self, val: Value, line: int, use_pyssize_t: bool = False) -> Val
             size_value = self.primitive_op(var_object_size, [val], line)
         elif is_set_rprimitive(typ) or is_frozenset_rprimitive(typ):
             elem_address = self.add(GetElementPtr(val, PySetObject, "used"))
-            size_value = self.add(LoadMem(c_pyssize_t_rprimitive, elem_address))
+            size_value = self.load_mem(elem_address, c_pyssize_t_rprimitive)
             self.add(KeepAlive([val]))
         elif is_dict_rprimitive(typ):
             size_value = self.call_c(dict_ssize_t_size_op, [val], line)

mypyc/lower/list_ops.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from mypyc.common import PLATFORM_SIZE
-from mypyc.ir.ops import GetElementPtr, IncRef, Integer, IntOp, LoadMem, SetMem, Value
+from mypyc.ir.ops import GetElementPtr, Integer, IntOp, SetMem, Value
 from mypyc.ir.rtypes import (
     PyListObject,
     c_pyssize_t_rprimitive,
@@ -42,7 +42,7 @@ def buf_init_item(builder: LowLevelIRBuilder, args: list[Value], line: int) -> V
 @lower_primitive_op("list_items")
 def list_items(builder: LowLevelIRBuilder, args: list[Value], line: int) -> Value:
     ob_item_ptr = builder.add(GetElementPtr(args[0], PyListObject, "ob_item", line))
-    return builder.add(LoadMem(pointer_rprimitive, ob_item_ptr, line))
+    return builder.load_mem(ob_item_ptr, pointer_rprimitive)
 
 
 def list_item_ptr(builder: LowLevelIRBuilder, obj: Value, index: Value, line: int) -> Value:
@@ -68,6 +68,4 @@ def list_item_ptr(builder: LowLevelIRBuilder, obj: Value, index: Value, line: in
 def list_get_item_unsafe(builder: LowLevelIRBuilder, args: list[Value], line: int) -> Value:
     index = builder.coerce(args[1], c_pyssize_t_rprimitive, line)
     item_ptr = list_item_ptr(builder, args[0], index, line)
-    value = builder.add(LoadMem(object_rprimitive, item_ptr, line))
-    builder.add(IncRef(value))
-    return value
+    return builder.load_mem(item_ptr, object_rprimitive)

mypyc/test-data/irbuild-classes.test

@@ -363,7 +363,7 @@ def f(x):
 L0:
     r0 = __main__.B :: type
     r1 = get_element_ptr x ob_type :: PyObject
-    r2 = load_mem r1 :: builtins.object*
+    r2 = borrow load_mem r1 :: builtins.object*
     keep_alive x
     r3 = r2 == r0
     if r3 goto L1 else goto L2 :: bool
@@ -402,7 +402,7 @@ def f(x):
 L0:
     r0 = __main__.A :: type
     r1 = get_element_ptr x ob_type :: PyObject
-    r2 = load_mem r1 :: builtins.object*
+    r2 = borrow load_mem r1 :: builtins.object*
     keep_alive x
     r3 = r2 == r0
     if r3 goto L1 else goto L2 :: bool
@@ -412,7 +412,7 @@ L1:
 L2:
     r5 = __main__.B :: type
     r6 = get_element_ptr x ob_type :: PyObject
-    r7 = load_mem r6 :: builtins.object*
+    r7 = borrow load_mem r6 :: builtins.object*
     keep_alive x
     r8 = r7 == r5
     r4 = r8
@@ -449,7 +449,7 @@ def f(x):
 L0:
     r0 = __main__.A :: type
     r1 = get_element_ptr x ob_type :: PyObject
-    r2 = load_mem r1 :: builtins.object*
+    r2 = borrow load_mem r1 :: builtins.object*
     keep_alive x
     r3 = r2 == r0
     if r3 goto L1 else goto L2 :: bool
@@ -459,7 +459,7 @@ L1:
 L2:
     r5 = __main__.R :: type
     r6 = get_element_ptr x ob_type :: PyObject
-    r7 = load_mem r6 :: builtins.object*
+    r7 = borrow load_mem r6 :: builtins.object*
     keep_alive x
     r8 = r7 == r5
     r4 = r8
@@ -500,7 +500,7 @@ def f(x):
 L0:
     r0 = __main__.A :: type
     r1 = get_element_ptr x ob_type :: PyObject
-    r2 = load_mem r1 :: builtins.object*
+    r2 = borrow load_mem r1 :: builtins.object*
     keep_alive x
     r3 = r2 == r0
     if r3 goto L1 else goto L2 :: bool
@@ -510,7 +510,7 @@ L1:
 L2:
     r5 = __main__.C :: type
     r6 = get_element_ptr x ob_type :: PyObject
-    r7 = load_mem r6 :: builtins.object*
+    r7 = borrow load_mem r6 :: builtins.object*
     keep_alive x
     r8 = r7 == r5
     r4 = r8

mypyc/test-data/irbuild-isinstance.test

@@ -77,7 +77,7 @@ L0:
     x = r0
     r1 = __main__.C :: type
     r2 = get_element_ptr x ob_type :: PyObject
-    r3 = load_mem r2 :: builtins.object*
+    r3 = borrow load_mem r2 :: builtins.object*
     keep_alive x
     r4 = r3 == r1
     if r4 goto L1 else goto L2 :: bool

mypyc/test-data/irbuild-optional.test

@@ -311,7 +311,7 @@ def get(o):
 L0:
     r0 = __main__.A :: type
     r1 = get_element_ptr o ob_type :: PyObject
-    r2 = load_mem r1 :: builtins.object*
+    r2 = borrow load_mem r1 :: builtins.object*
     keep_alive o
     r3 = r2 == r0
     if r3 goto L1 else goto L2 :: bool
@@ -390,7 +390,7 @@ def g(o):
 L0:
     r0 = __main__.A :: type
     r1 = get_element_ptr o ob_type :: PyObject
-    r2 = load_mem r1 :: builtins.object*
+    r2 = borrow load_mem r1 :: builtins.object*
     keep_alive o
     r3 = r2 == r0
     if r3 goto L1 else goto L2 :: bool
@@ -403,7 +403,7 @@ L1:
 L2:
     r8 = __main__.B :: type
     r9 = get_element_ptr o ob_type :: PyObject
-    r10 = load_mem r9 :: builtins.object*
+    r10 = borrow load_mem r9 :: builtins.object*
     keep_alive o
     r11 = r10 == r8
     if r11 goto L3 else goto L4 :: bool
@@ -456,7 +456,7 @@ def f(o):
 L0:
     r0 = __main__.A :: type
     r1 = get_element_ptr o ob_type :: PyObject
-    r2 = load_mem r1 :: builtins.object*
+    r2 = borrow load_mem r1 :: builtins.object*
     keep_alive o
     r3 = r2 == r0
     if r3 goto L1 else goto L2 :: bool
@@ -488,7 +488,7 @@ def g(o):
 L0:
     r0 = __main__.A :: type
     r1 = get_element_ptr o ob_type :: PyObject
-    r2 = load_mem r1 :: builtins.object*
+    r2 = borrow load_mem r1 :: builtins.object*
     keep_alive o
     r3 = r2 == r0
     if r3 goto L1 else goto L2 :: bool

mypyc/test-data/irbuild-singledispatch.test

@@ -57,7 +57,7 @@ def f_obj.__call__(__mypyc_self__, arg):
     r27 :: bool
 L0:
     r0 = get_element_ptr arg ob_type :: PyObject
-    r1 = load_mem r0 :: builtins.object*
+    r1 = borrow load_mem r0 :: builtins.object*
     keep_alive arg
     r2 = __mypyc_self__.dispatch_cache
     r3 = CPyDict_GetWithNone(r2, r1)
@@ -82,7 +82,7 @@ L2:
 L3:
     r16 = load_address PyLong_Type
     r17 = get_element_ptr r6 ob_type :: PyObject
-    r18 = load_mem r17 :: builtins.object*
+    r18 = borrow load_mem r17 :: builtins.object*
     keep_alive r6
     r19 = r18 == r16
     if r19 goto L4 else goto L7 :: bool
@@ -195,7 +195,7 @@ def f_obj.__call__(__mypyc_self__, x):
     r24 :: None
 L0:
     r0 = get_element_ptr x ob_type :: PyObject
-    r1 = load_mem r0 :: builtins.object*
+    r1 = borrow load_mem r0 :: builtins.object*
     keep_alive x
     r2 = __mypyc_self__.dispatch_cache
     r3 = CPyDict_GetWithNone(r2, r1)
@@ -220,7 +220,7 @@ L2:
 L3:
     r16 = load_address PyLong_Type
     r17 = get_element_ptr r6 ob_type :: PyObject
-    r18 = load_mem r17 :: builtins.object*
+    r18 = borrow load_mem r17 :: builtins.object*
     keep_alive r6
     r19 = r18 == r16
     if r19 goto L4 else goto L5 :: bool