From 8f7dc924ed02696f2b799ec194c76187e3480bf6 Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Tue, 21 Jul 2026 21:02:21 +0800 Subject: [PATCH] Fix matrix void method return lowering --- pineforge_codegen/codegen/drawing.py | 48 ++++++- pineforge_codegen/codegen/tables.py | 14 ++ tests/test_matrix_void_method_return.py | 168 ++++++++++++++++++++++++ 3 files changed, 226 insertions(+), 4 deletions(-) create mode 100644 tests/test_matrix_void_method_return.py diff --git a/pineforge_codegen/codegen/drawing.py b/pineforge_codegen/codegen/drawing.py index 2cd1e49..7091f75 100644 --- a/pineforge_codegen/codegen/drawing.py +++ b/pineforge_codegen/codegen/drawing.py @@ -20,7 +20,13 @@ from __future__ import annotations from ..ast_nodes import FuncCall, Identifier, MemberAccess -from .tables import DRAWING_TYPE_TO_CPP, DRAWING_ARENA, ARRAY_VOID_METHODS as _ARRAY_VOID_METHODS +from ..symbols import method_receiver_type_name +from .tables import ( + ARRAY_VOID_METHODS as _ARRAY_VOID_METHODS, + DRAWING_ARENA, + DRAWING_TYPE_TO_CPP, + MATRIX_VOID_METHODS as _MATRIX_VOID_METHODS, +) _MAP_VOID_METHODS = frozenset({"clear", "put_all"}) @@ -482,9 +488,10 @@ def _call_is_void(self, node) -> bool: Covers drawing setters/delete/visual-noop (delegated to ``_drawing_call_is_void``), the Pine array MUTATOR methods whose C++ lowering is void / an iterator (``array.push/insert/clear/set/fill/ - sort/reverse/concat/unshift``), and terminal map ``clear``/``put_all`` - calls. A function ending in one of these calls must emit it as a - statement with a default return, never ``return ;``. + sort/reverse/concat/unshift``), matrix mutators whose runtime lowering + returns void, and terminal map ``clear``/``put_all`` calls. A function + ending in one of these calls must emit it as a statement with a default + return, never ``return ;``. """ if self._drawing_call_is_void(node): return True @@ -492,6 +499,39 @@ def _call_is_void(self, node) -> bool: return False method = node.callee.member _fn, ns = self._resolve_callee(node.callee) + + if method in _MATRIX_VOID_METHODS: + # Resolve the exact matrix receiver for both established spellings: + # ``matrix.set(id, ...)`` and ``id.set(...)``. A same-named typed + # user method has dispatch precedence over the builtin (KI-73), so + # never classify that call from the builtin method name alone. + direct_spec = self._type_spec_from_expr(node.callee.object) + direct_receiver_name = method_receiver_type_name(direct_spec) + direct_method_info = ( + getattr(self, "_func_info_map", {}).get( + f"{direct_receiver_name}.{method}" + ) + if direct_receiver_name is not None + else None + ) + if direct_method_info is not None and getattr( + direct_method_info, "is_udt_method", False + ): + return False + + functional_namespace = ns == "matrix" and ( + direct_spec is None or direct_spec.kind != "matrix" + ) + recv_spec = direct_spec + if functional_namespace: + recv_spec = ( + self._type_spec_from_expr(node.args[0]) + if node.args + else None + ) + if recv_spec is not None and recv_spec.kind == "matrix": + return True + if method in _ARRAY_VOID_METHODS: # array.(arr, ...) namespace form OR arr.(...) # method form on a std::vector receiver. diff --git a/pineforge_codegen/codegen/tables.py b/pineforge_codegen/codegen/tables.py index 3608365..20e73f1 100644 --- a/pineforge_codegen/codegen/tables.py +++ b/pineforge_codegen/codegen/tables.py @@ -731,6 +731,20 @@ def _matrix_add_col(m: str, args: list) -> str: "add_col": ["col_index", "array_id"], } +# Matrix mutators whose established C++ lowering returns ``void``. A Pine +# callable may use one as its terminal expression, but its generated numeric +# fallback wrapper must emit the mutation as a statement and then return the +# default value; ``return matrix.set(...)`` is invalid C++. +# +# ``remove_row`` / ``remove_col`` are intentionally absent: their lowering in +# ``MATRIX_METHODS`` already wraps the runtime mutation in a lambda that returns +# a numeric sentinel. Matrix-returning transforms belong exclusively in +# ``MATRIX_RETURNING_METHODS`` below. +MATRIX_VOID_METHODS: frozenset[str] = frozenset({ + "set", "fill", "add_row", "add_col", "swap_rows", "swap_columns", + "reshape", "reverse", "sort", +}) + # matrix.* method names whose RUNTIME return type is ``PineMatrix``. Used by # the codegen aggregate-type registration to declare the LHS variable as # ``PineMatrix`` instead of the analyzer's default ``double``. Without this diff --git a/tests/test_matrix_void_method_return.py b/tests/test_matrix_void_method_return.py new file mode 100644 index 0000000..895623b --- /dev/null +++ b/tests/test_matrix_void_method_return.py @@ -0,0 +1,168 @@ +"""Regression controls for a void matrix call as a method's sole expression. + +The parser and analyzer already preserve the authored method and identify its +terminal ``matrix.set`` call as void. Codegen must therefore emit the call as +a statement, not try to return the C++ ``void`` expression from its numeric +fallback method wrapper. +""" + +from __future__ import annotations + +import re + +import pytest + +from pineforge_codegen import transpile +from pineforge_codegen.analyzer import Analyzer +from pineforge_codegen.ast_nodes import ExprStmt, FuncCall, MemberAccess, MethodDef +from pineforge_codegen.lexer import Lexer +from pineforge_codegen.parser import Parser +from pineforge_codegen.symbols import PineType +from tests._compile import compile_cpp + + +SOURCE = '''//@version=6 +strategy("void matrix method") +method set(matrix self, int row, int col, int value) => matrix.set(self, row, col, value) +''' + + +MATRIX_VOID_CASES = ( + ("set", "matrix.set(self, 0, 0, 1)", "self.set(0, 0, 1)"), + ("fill", "matrix.fill(self, 1)", "self.fill(1)"), + ("add_row", "matrix.add_row(self, values)", "self.add_row(values)"), + ("add_col", "matrix.add_col(self, values)", "self.add_col(values)"), + ("swap_rows", "matrix.swap_rows(self, 0, 0)", "self.swap_rows(0, 0)"), + ( + "swap_columns", + "matrix.swap_columns(self, 0, 0)", + "self.swap_columns(0, 0)", + ), + ("reshape", "matrix.reshape(self, 1, 1)", "self.reshape(1, 1)"), + ("reverse", "matrix.reverse(self)", "self.reverse()"), + ("sort", "matrix.sort(self, 0)", "self.sort(0)"), +) + + +def _void_case_source(method: str, expression: str, form: str) -> str: + values_param = ", array values" if method in {"add_row", "add_col"} else "" + return f'''//@version=6 +strategy("matrix {method} {form}") +method mutate_{method}_{form}(matrix self{values_param}) => {expression} +''' + + +def _parse(): + return Parser(Lexer(SOURCE).tokenize(), source=SOURCE).parse() + + +def test_single_expression_matrix_void_method_parses_without_recovery() -> None: + program = _parse() + + assert (program.annotations or {}).get("parse_recovery_count", 0) == 0 + methods = [node for node in program.body if isinstance(node, MethodDef)] + assert len(methods) == 1 + method = methods[0] + assert method.type_name == "matrix" + assert method.params == ["self", "row", "col", "value"] + assert method.is_single_expr + assert len(method.body) == 1 + assert isinstance(method.body[0], ExprStmt) + assert isinstance(method.body[0].expr, FuncCall) + assert isinstance(method.body[0].expr.callee, MemberAccess) + assert method.body[0].expr.callee.member == "set" + + +def test_single_expression_matrix_void_method_analyzes_as_void() -> None: + ctx = Analyzer(_parse()).analyze() + + method_info = next( + info for info in ctx.func_infos if info.name == "matrix.set" + ) + assert method_info.return_type == PineType.VOID + assert method_info.return_type_spec is None + + +def test_single_expression_matrix_void_method_does_not_return_void_call() -> None: + cpp = transpile(SOURCE) + + assert re.search( + r"double _udt_matrix_int_set\(PineGenericMatrix& self, " + r"int row, int col, int value\)", + cpp, + ) + assert "return self.set((int)(row), (int)(col), value);" not in cpp + assert "self.set((int)(row), (int)(col), value);" in cpp + assert "return 0.0;" in cpp + + +def test_single_expression_matrix_void_method_native_compiles() -> None: + compile_cpp( + transpile(SOURCE), + label="matrix-single-expression-void-method", + ) + + +@pytest.mark.parametrize("method,namespace_expr,member_expr", MATRIX_VOID_CASES) +@pytest.mark.parametrize("form", ("namespace", "member")) +def test_every_void_matrix_mutator_is_a_statement_with_default_return( + method: str, + namespace_expr: str, + member_expr: str, + form: str, +) -> None: + expression = namespace_expr if form == "namespace" else member_expr + cpp = transpile(_void_case_source(method, expression, form)) + function_name = f"_udt_matrix_int_mutate_{method}_{form}" + body = cpp.split(f" {function_name}(", 1)[1].split("\n }", 1)[0] + + assert f"self.{method}(" in body + assert f"return self.{method}(" not in body + assert "return 0.0;" in body + + +def test_all_void_matrix_mutator_forms_native_compile() -> None: + sources = [ + _void_case_source( + method, + namespace_expr if form == "namespace" else member_expr, + form, + ) + for method, namespace_expr, member_expr in MATRIX_VOID_CASES + for form in ("namespace", "member") + ] + for index, source in enumerate(sources): + compile_cpp( + transpile(source), + label=f"matrix-void-mutator-{index}", + ) + + +def test_typed_user_matrix_method_still_precedes_same_named_builtin() -> None: + source = '''//@version=6 +strategy("typed user matrix method precedence") +method fill(matrix self) => 42 +method delegate(matrix self) => self.fill() +''' + + cpp = transpile(source) + body = cpp.split(" _udt_matrix_int_delegate(", 1)[1].split("\n }", 1)[0] + + assert "return _udt_matrix_int_fill(self);" in body + assert "return 0.0;" not in body + compile_cpp(cpp, label="matrix-void-mutator-user-method-precedence") + + +def test_typed_user_method_precedes_shadowed_matrix_namespace() -> None: + source = '''//@version=6 +strategy("typed user method shadows matrix namespace") +method fill(int self, matrix other) => 42 +method delegate(int matrix, matrix other) => matrix.fill(other) +''' + + cpp = transpile(source) + body = cpp.split(" _udt_int_delegate(", 1)[1].split("\n }", 1)[0] + + assert "return _udt_int_fill(matrix, other);" in body + assert "return 0;" not in body + compile_cpp(cpp, label="matrix-void-mutator-shadowed-namespace")