diff --git a/docs/SPEC.md b/docs/SPEC.md index e550345c..4bde73d1 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -577,6 +577,7 @@ All in `PM/commonTest`, so they run wherever the interpreter loads. | No deopting node in an impure function | not compiled; diagnostic | | Array access in bounds, or a runtime bounds check kept | the check stays (never an unchecked access) | | Array parameters only indexed, stored, `len`-ed; no aliasing (entry guard) | not compiled; diagnostic | + | A function holding an array parameter runs no user code: closed, and no slot read or `C(args)` whose slow path runs user code, in its whole call tree (#203) | not compiled; diagnostic | | OBJ references follow one ownership rule; no IR form for free, borrowed alias or NULL read | not compiled; diagnostic | | Integer overflow checked and promoted (deopt); an op marked `proven` must re-prove its interval fits i64 | not compiled; diagnostic `verify/proven-overflow` | | `Call.redo` only from an impure caller to a pure, may-deopt callee returning F64/BOOL/NONE | not compiled; diagnostic `verify/redo` | diff --git a/python-multiplatform-ksp/src/main/python/typedpython/ir.py b/python-multiplatform-ksp/src/main/python/typedpython/ir.py index b97fe6bf..4660df44 100644 --- a/python-multiplatform-ksp/src/main/python/typedpython/ir.py +++ b/python-multiplatform-ksp/src/main/python/typedpython/ir.py @@ -536,9 +536,9 @@ class ArrayParam: At entry (guards, before any effect): the argument is exactly a `list`, every element is exactly `float` (F64_ARRAY) or exactly `int` within i64 (I64_ARRAY), and no two array parameters are the same list object (aliasing would make separate copies diverge). Then the elements are copied - into a C array. The function body only indexes, stores into and takes `len` of it (no calls that - could observe the list, no appends: the front end checks), so the list and the array cannot - diverge observably. On **every** exit, return, exception, or deopt, dirty elements are + into a C array. The function body only indexes, stores into and takes `len` of it, no appends, and + runs no user code at all: the verifier rejects a function holding an array that is not closed + (`verify/array-open`, #203), so the list and the array cannot diverge observably. On **every** exit, return, exception, or deopt, dirty elements are written back as new float/int objects before control leaves the function, so CPython's partial-update-then-raise behaviour is preserved, and a deopt redo starts from the same list state the interpreted function would see... which is only correct because a function with array diff --git a/python-multiplatform-ksp/src/main/python/typedpython/verify.py b/python-multiplatform-ksp/src/main/python/typedpython/verify.py index 9ad89afc..5bbc7be8 100644 --- a/python-multiplatform-ksp/src/main/python/typedpython/verify.py +++ b/python-multiplatform-ksp/src/main/python/typedpython/verify.py @@ -42,6 +42,7 @@ on a node `proven` does not apply to, or it is not a bool) verify/redo `Call.redo` where it is not legal (see below) verify/entry-global-open a function with entry_globals is not closed + verify/array-open a function with an ArrayParam runs user code (#203) verify/class a class named by FieldGet/FieldSet/New/IsExact/CheckExact/Param.cls is not a (valid) ClassDecl of the module, a field is not one of its fields, a New names a class without trivial_init or has the wrong arity, or a @@ -196,6 +197,7 @@ PROVEN = "verify/proven-overflow" REDO = "verify/redo" ENTRY_OPEN = "verify/entry-global-open" +ARRAY_OPEN = "verify/array-open" CLASS = "verify/class" FIELD = "verify/field-unproved" @@ -278,6 +280,7 @@ def __init__(self, function: Function, signatures: dict[str, Function], self.lens: dict[str, tuple[int, int]] = {} # single-assignment local array -> length self.entry: dict[str, Type] = {} # name -> type of every entry global self.opens: list[str] = [] # reasons the function is not closed + self.slow_user: list[str] = [] # slow paths that can run user code (FieldGet, New) def error(self, rule: str, message: str) -> None: d = Diagnostic(self.fn.name, rule, message, self.fn.source_line) @@ -919,9 +922,13 @@ def e_FieldGet(self, e: FieldGet, state: _State) -> ir.Expr: self.result(e, Type.OBJ) if self.known_field(e.cls, e.field, f"FieldGet .{e.field}"): self.proved(obj, e.cls, f"FieldGet {e.cls}.{e.field}") - return dataclasses.replace(e, obj=obj) # a slot read runs no user code: not an effect + # a slot read runs no user code: not an effect. Its slow path after a class change (getattr) + # can, which matters to array functions (#203). + self.slow_user.append(f"FieldGet .{e.field} (generic getattr after a class change)") + return dataclasses.replace(e, obj=obj) def e_New(self, e: New, state: _State) -> ir.Expr: + self.slow_user.append(f"New {e.cls} (calls the class after a class change)") args = self.args(e.args, state, f"New {e.cls}") for k, a in enumerate(args): self.slot(a, Type.OBJ, f"argument {k + 1} of New {e.cls}") @@ -1349,6 +1356,7 @@ def reject(fn_name: str, line: int, rule: str, message: str) -> None: checked: dict[str, Function] = {} callees: dict[str, set[str]] = {} open_why: dict[str, str] = {} # a function that runs user code -> why + slow_why: dict[str, str] = {} # a function whose slow paths can run user code -> why for name, g in sigs.items(): c = _Checker(g, sigs, classes) rebuilt = c.run() @@ -1358,6 +1366,8 @@ def reject(fn_name: str, line: int, rule: str, message: str) -> None: callees[name] = c.callees if c.opens: open_why[name] = c.opens[0] + if c.slow_user: + slow_why[name] = c.slow_user[0] # Closedness (entry_globals): a fixpoint over the call graph. changed = True @@ -1374,6 +1384,26 @@ def reject(fn_name: str, line: int, rule: str, message: str) -> None: reject(name, g.source_line, ENTRY_OPEN, f"has entry globals but is not closed: {open_why[name]}") + # ArrayParam (#203): the list is copied in and written back, so nothing may run user code while + # the copy is live: user code could read or change the list. Closed as above, and also no + # FieldGet/New, whose slow paths run user code after a class change, anywhere in the call tree. + array_why = dict(open_why) + for name, why in slow_why.items(): + array_why.setdefault(name, why) + changed = True + while changed: + changed = False + for name, cs in callees.items(): + if name not in array_why: + bad = sorted(c for c in cs if c in array_why) + if bad: + array_why[name] = f"calls '{bad[0]}', which can run user code ({array_why[bad[0]]})" + changed = True + for name, g in sigs.items(): + if name in array_why and any(isinstance(p, ArrayParam) for p in g.params): + reject(name, g.source_line, ARRAY_OPEN, + f"holds a list copy-in but can run user code: {array_why[name]}") + # A call cycle (direct or mutual recursion) is legal: every compiled impl function counts its # frame at entry (tp_enter_call), so recursion ends in RecursionError where CPython raises it # and the C stack cannot overflow (issue #57). Purity and may_deopt flow through the cycle as diff --git a/python-multiplatform-ksp/src/test/python/test_cgen_class_changes.py b/python-multiplatform-ksp/src/test/python/test_cgen_class_changes.py index 7e1c76d9..80e50c42 100644 --- a/python-multiplatform-ksp/src/test/python/test_cgen_class_changes.py +++ b/python-multiplatform-ksp/src/test/python/test_cgen_class_changes.py @@ -112,9 +112,9 @@ def is_node(cb, x): cb() return type(x) is Node - def fill(a, n): + def fill(n): x = n.left - a[0] = SCALE + return SCALE ''') NODE = ClassDecl("Node", ("left", "right"), True) @@ -199,12 +199,13 @@ def f_is_node(): def f_fill(): - # closed (no user code), impure (an array store), with an entry global - return fn("fill", [ArrayParam("a", Type.F64_ARRAY, True), Param("n", OBJ, cls="Node")], NONE, + # closed (no user code) with an entry global. Not an array function any more: a slot read's + # slow path can run a property, which must not run while a list copy-in is live (#203) + return fn("fill", [Param("n", OBJ, cls="Node")], F64, {"x": OBJ}, [Assign("x", FieldGet(OBJ, _o("n"), "Node", "left")), - StoreIndex("a", c(0), L("SCALE", F64)), Return()], - pure=False, may_deopt=False, entry_globals=(Param("SCALE", F64),)) + Return(L("SCALE", F64))], + pure=True, may_deopt=False, entry_globals=(Param("SCALE", F64),)) def functions(): @@ -270,8 +271,7 @@ def sc_unchanged(M, call, P): call("pair", 1, 2), call("mid_get", NOOP, n), call("mid_set", NOOP, n, "y"), call("mid_new", NOOP, 3, 4), call("arg_new", lambda: "a", 5), call("via_pair", NOOP, 6, 7), call("is_node", NOOP, n), call("is_node", NOOP, M.Twin(1, 2))] - a = [0.0] - out += [call("fill", a, n), a] + out += [call("fill", n)] P("after") return out @@ -456,16 +456,14 @@ def sc_exhaust(M, call, P): def sc_entry_global(M, call, P): n = M.Node(1, 2) - a = [0.0] - out = [call("fill", a, n), list(a)] + out = [call("fill", n)] P("before") def getter(s): M.SCALE = 5.0 return "p" M.Node.left = property(getter) - b = [0.0] - out += [call("fill", b, n), list(b), M.SCALE] + out += [call("fill", n), M.SCALE] P("after") return out @@ -601,12 +599,12 @@ def round_(): M.via_pair(cb, shared, shared) M.arg_new(cb, shared) M.is_node(cb, n) - M.fill([0.0], n) + M.fill(n) A2.get_left(n2) A2.mid_get(cb, n2) A2.setl(n2, shared) A2.mid_set(cb, n2, shared) - A2.fill([0.0], n2) + A2.fill(n2) try: A2.check(n2) except AttributeError: @@ -780,7 +778,8 @@ def test_exhausted_version_tags_take_the_slow_path(report): def test_closed_function_with_an_entry_global_sees_the_rebinding(report): r = same(report, "entry_global") - assert r["compiled"][1] == [2.0] and r["compiled"][3] == [5.0] + # fill returns SCALE: the old value before, the value the property getter set after + assert r["compiled"][0] == 2.0 and r["compiled"][1] == 5.0 # --- behaviour: no dict watcher available ------------------------------------------------------------ diff --git a/python-multiplatform-ksp/src/test/python/test_pipeline.py b/python-multiplatform-ksp/src/test/python/test_pipeline.py index a835153c..039b39c5 100644 --- a/python-multiplatform-ksp/src/test/python/test_pipeline.py +++ b/python-multiplatform-ksp/src/test/python/test_pipeline.py @@ -100,3 +100,38 @@ def test_demo_reports_identical_output(write, tmp_path): assert result.returncode == 0, result.stdout + result.stderr assert "identical output" in result.stdout and "249750.000" in result.stdout assert "compiled functions: total" in result.stdout + + +ARRAY_CALLBACK = """\ +# typedpython: compiled +from typing import Callable + + +def first_after(xs: list[float], cb: Callable[[], None]) -> float: + cb() + return xs[0] + + +def fill_then(xs: list[float], cb: Callable[[], None]) -> None: + xs[1] = 5.0 + cb() +""" + + +def test_array_function_with_a_callback_matches_cpython(write, tmp_path): + # #203: the callback changes the list; a copy-in would read or write back stale values + path = write("arrcb.py", ARRAY_CALLBACK) + result = pipeline.compile_module(path, tmp_path / "out") + assert "first_after" in result.skipped and "fill_then" in result.skipped, result.skipped + compiled, ref = pipeline.load(result), interpreted(path) + for mod in (compiled, ref): + xs = [1.0, 2.0, 3.0] + def cb(): + xs[0] = 100.0 + xs[1] = 7.0 + assert mod.first_after(xs, cb) == 100.0 + ys = [1.0, 2.0, 3.0] + def cb2(): + ys[1] = 9.0 + mod.fill_then(ys, cb2) + assert ys == [1.0, 9.0, 3.0] diff --git a/python-multiplatform-ksp/src/test/python/test_verify.py b/python-multiplatform-ksp/src/test/python/test_verify.py index d98046d8..ec31224d 100644 --- a/python-multiplatform-ksp/src/test/python/test_verify.py +++ b/python-multiplatform-ksp/src/test/python/test_verify.py @@ -843,14 +843,16 @@ def test_range_up_to_a_param_bounds_the_var_by_the_param_range(): def test_len_bound_is_zero_to_two_to_the_62(): - a = [ArrayParam("a", F64A, stored=False)] + # impure through a store into the array itself (an array function may not call user code, #203) + a = [ArrayParam("a", F64A, stored=True)] L = Len(I64, "a") - assert_accepted(fn(params=a, returns=I64, body=[IMPURE, Return(padd(L, i(2 ** 62 - 1)))])) + W = If(Compare(BOOL, CompareKind.GT, L, i(0)), (StoreIndex("a", i(0), f(0.0)),)) + assert_accepted(fn(params=a, returns=I64, body=[W, Return(padd(L, i(2 ** 62 - 1)))])) assert_rejected(PROVEN, fn(params=a, returns=I64, - body=[IMPURE, Return(padd(L, i(2 ** 62)))])) - assert_rejected(PROVEN, fn(params=a, returns=I64, body=[IMPURE, Return(pmul(L, i(2)))])) - assert_accepted(fn(params=a, returns=I64, body=[IMPURE, Return(pmul(L, i(-2)))])) # -2**63 - assert_rejected(PROVEN, fn(params=a, returns=I64, body=[IMPURE, Return(pmul(L, i(-3)))])) + body=[W, Return(padd(L, i(2 ** 62)))])) + assert_rejected(PROVEN, fn(params=a, returns=I64, body=[W, Return(pmul(L, i(2)))])) + assert_accepted(fn(params=a, returns=I64, body=[W, Return(pmul(L, i(-2)))])) # -2**63 + assert_rejected(PROVEN, fn(params=a, returns=I64, body=[W, Return(pmul(L, i(-3)))])) def test_mod_and_floordiv_are_full_range_so_nothing_is_provable_from_them(): @@ -978,10 +980,10 @@ def test_entry_global_in_a_function_running_user_code_is_rejected(node): def test_the_same_open_node_without_entry_globals_is_accepted(): - # The open node alone is fine: only entry globals need the function closed. - a = ArrayParam("a", F64A, stored=False) - assert_accepted(fn(params=[a], returns=F64, body=[ - ExprStmt(CallObject(OBJ, Global(OBJ, "print"), (), ())), Return(Index(F64, "a", i(0)))])) + # The open node alone is fine: only entry globals (and array parameters, #203) need the + # function closed. + assert_accepted(fn(params=[Param("x", F64)], returns=F64, body=[ + ExprStmt(CallObject(OBJ, Global(OBJ, "print"), (), ())), Return(lf("x"))])) def test_global_read_and_box_keep_a_function_closed(): @@ -1074,9 +1076,10 @@ def _base_functions(): Return(add(lf("s"), MathCall(F64, MathFunc.SQRT, (ToFloat(F64, li("c")),)), F64)), ]) - impure = fn("scale", params=[ArrayParam("a", F64A, stored=True), Param("x", F64), - Param("o", OBJ), Param("c", BOOL)], - locals_={"k": I64, "r": OBJ, "y": F64}, returns=F64, line=20, body=[ + # a local array, not an ArrayParam: this function calls user code (#203) + impure = fn("scale", params=[Param("x", F64), Param("o", OBJ), Param("c", BOOL)], + locals_={"a": F64A, "k": I64, "r": OBJ, "y": F64}, returns=F64, line=20, body=[ + Assign("a", na(F64A, i(4), f(1.0))), Assign("r", call_obj(GetAttr(OBJ, g_("Modifier"), "padding"), Box(OBJ, lf("x")), lo("o"), kw=("dp",))), Assign("y", Unbox(F64, lo("r"))), @@ -1147,6 +1150,10 @@ def _breaking_mutations(rng, g): if kind == 4: # flip may_deopt return "flip may_deopt", dataclasses.replace(g, may_deopt=not g.may_deopt), { FLAG_DEOPT, DEOPT} + if kind == 5 and not any(isinstance(p, ArrayParam) for p in g.params): + kind = 3 # no ArrayParam (scale has a local array since #203): flip pure instead + if kind == 3: + return "flip pure", dataclasses.replace(g, pure=not g.pure), {FLAG_PURE, DEOPT, FLAG_DEOPT} if kind == 5: # flip stored on the first array k = next(j for j, p in enumerate(g.params) if isinstance(p, ArrayParam)) p = g.params[k] @@ -1915,7 +1922,7 @@ def _array_functions(): StoreIndex("a", li("k"), BinOp(F64, BinOpKind.ADD, Index(F64, "z", li("k")), f(1.0))), )), - Assign("r", call_obj(g_("print"), Box(OBJ, Index(F64, "z", i(0))))), + Assign("r", Box(OBJ, Index(F64, "z", i(0)))), # no user call: an ArrayParam (#203) Return(), ]) sink = fn("sink", params=[Param("x", OBJ)], returns=NONE, line=12, body=[Return()]) @@ -2021,3 +2028,40 @@ def test_each_array_mutation_kind_was_exercised(): base = _array_functions() kinds = {_array_mutations(rng, base)[0] for _ in range(400)} assert len(kinds) == 18, sorted(kinds) # 16 kinds; kinds 7 and 8 have two variants + + +# --- #203: an ArrayParam function must be closed --------------------------------------------- + +ARRAY_OPEN = "verify/array-open" + + +def _arr_fn(name, body, returns=F64, locals_=None, params_extra=()): + return fn(name, params=[ArrayParam("a", F64A, stored=False), *params_extra], returns=returns, + locals_=locals_, body=body) + + +def test_array_function_calling_an_object_is_rejected(): + # the callback can change the list the function copied in (#203) + assert_rejected(ARRAY_OPEN, _arr_fn("f", [ExprStmt(pycall("cb")), Return(Index(F64, "a", i(0)))])) + + +def test_array_function_reading_an_attribute_is_rejected(): + body = [ExprStmt(GetAttr(OBJ, lo("o"), "x")), Return(Index(F64, "a", i(0)))] + assert_rejected(ARRAY_OPEN, _arr_fn("f", body, params_extra=[Param("o", OBJ)])) + + +def test_array_function_testing_an_object_is_rejected(): + body = [If(Truth(BOOL, lo("o")), (Return(f(0.0)),)), Return(Index(F64, "a", i(0)))] + assert_rejected(ARRAY_OPEN, _arr_fn("f", body, params_extra=[Param("o", OBJ)])) + + +def test_array_function_calling_an_open_function_is_rejected(): + g = fn("g", returns=NONE, body=[ExprStmt(pycall("cb")), Return()]) + caller = _arr_fn("f", [ExprStmt(Call(NONE, "g", ())), Return(Index(F64, "a", i(0)))]) + assert_rejected(ARRAY_OPEN, caller, g) + + +def test_closed_array_function_is_still_accepted(): + g = fn("g", params=[Param("x", F64)], returns=F64, body=[Return(lf("x"))]) + caller = _arr_fn("f", [Return(Call(F64, "g", (Index(F64, "a", i(0)),)))]) + assert_accepted(caller, g)