Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 44 additions & 4 deletions pineforge_codegen/codegen/drawing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"})

Expand Down Expand Up @@ -482,16 +488,50 @@ 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 <void-expression>;``.
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 <void-expression>;``.
"""
if self._drawing_call_is_void(node):
return True
if not isinstance(node, FuncCall) or not isinstance(node.callee, MemberAccess):
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.<method>(arr, ...) namespace form OR arr.<method>(...)
# method form on a std::vector receiver.
Expand Down
14 changes: 14 additions & 0 deletions pineforge_codegen/codegen/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
168 changes: 168 additions & 0 deletions tests/test_matrix_void_method_return.py
Original file line number Diff line number Diff line change
@@ -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<int> 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<int> values" if method in {"add_row", "add_col"} else ""
return f'''//@version=6
strategy("matrix {method} {form}")
method mutate_{method}_{form}(matrix<int> 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<int>"
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<int>.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<int>& 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<int> self) => 42
method delegate(matrix<int> 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<int> other) => 42
method delegate(int matrix, matrix<int> 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")
Loading