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
79 changes: 79 additions & 0 deletions pineforge_codegen/codegen/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,16 @@ def __init__(self, ctx: AnalyzerContext) -> None:
# needed: names are function-unique and the value-copy fallback ignores
# entries for inactive functions.
self._udt_ptr_alias_locals: set[str] = set()
# Names of hoisted GLOBAL-scope UDT loop-locals bound from a UDT array
# element (``z = arr.get(i)``) and later field-mutated. Pine array
# elements of a user-defined type are references, so such a local must
# ALIAS the element, not value-copy — the mutation has to write back
# into the array. These are de-hoisted from the class-member value-copy
# to a fresh per-iteration ``UDT& z = arr[i];`` local reference (the same
# form the non-hoisted function-local alias path already emits). Read-only
# get-locals are NOT recorded (no field mutation) and keep value-copy
# semantics. Populated by _register_udt_array_get_ref_locals.
self._udt_array_get_ref_locals: set[str] = set()
self._precalc_loop_active: bool = False
# Names of ``var`` members that live in a FUNCTION scope (not global).
# These are initialized once-per-function-variant on first call (a
Expand Down Expand Up @@ -688,6 +698,7 @@ def __init__(self, ctx: AnalyzerContext) -> None:
self._all_member_names.add(self._safe_name(name))

self._register_global_aggregate_member_types()
self._register_udt_array_get_ref_locals()
self._uses_matrix = self._detect_matrix_usage()
# Drawing-objects-as-data: gate all new emission (drawing.hpp include +
# the per-type arenas) on this flag so non-drawing strategies stay
Expand Down Expand Up @@ -1068,6 +1079,68 @@ def _register_global_aggregate_member_types(self) -> None:
self._matrix_specs[name] = spec2
self._collection_types[name] = spec2

def _walk_global_scope_with_loopflag(self, stmts, in_loop):
"""Yield ``(stmt, in_loop)`` for every statement in global scope,
recursing into control-flow bodies (if/for/while/switch) but NOT into
nested function definitions — a function-local of the same name lives in
a separate scope and must not be attributed to a global member. The
``in_loop`` flag is True once inside any for/while loop body."""
for s in stmts:
if isinstance(s, FuncDef):
continue
yield s, in_loop
child_in_loop = in_loop or isinstance(s, (ForStmt, ForInStmt, WhileStmt))
for attr in ("body", "else_body", "default_body"):
child = getattr(s, attr, None)
if isinstance(child, list):
yield from self._walk_global_scope_with_loopflag(child, child_in_loop)
cases = getattr(s, "cases", None)
if isinstance(cases, list):
for _case_expr, case_stmts in cases:
if isinstance(case_stmts, list):
yield from self._walk_global_scope_with_loopflag(case_stmts, child_in_loop)

def _register_udt_array_get_ref_locals(self) -> None:
"""Detect global-scope UDT loop-locals that alias a UDT array element and
are later field-mutated (Pine array elements of a user-defined type are
references — ``z = arr.get(i)`` then ``z.f := v`` MUST write back into
``arr``).

A non-``var`` global-scope ``UDT z = arr.get(i)`` (or ``.first`` /
``.last``) nested inside a for/while loop mis-lowers to a value copy: a
global ``while`` loop hoists ``z`` to a class member whose in-loop init
becomes a value-copy assignment, and a global ``for`` loop keeps ``z`` a
true local but the function-local alias path (``_udt_local_alias_kind``)
no-ops at global scope (``_current_func_body`` is None) — both silently
drop the field mutation. We record exactly this shape so the (possible)
class member is suppressed and the in-loop VarDecl is emitted as a fresh
per-iteration ``UDT& z = arr[i];`` reference instead (the same alias form
the non-hoisted function-local path already produces). Strictly gated:
the RHS must be a UDT-array-element lvalue AND the name must be field-
mutated at global scope AND the declaration must be loop-nested. A
read-only get-local is never recorded, so its value-copy output is
unchanged. Function-local get-locals are excluded (the walker skips
function bodies) — those keep using the existing alias path."""
pairs = list(self._walk_global_scope_with_loopflag(self.ctx.ast.body, False))
field_mutated: set[str] = set()
for s, _in_loop in pairs:
if (isinstance(s, Assignment)
and isinstance(s.target, MemberAccess)
and isinstance(s.target.object, Identifier)):
field_mutated.add(s.target.object.name)
for s, in_loop in pairs:
if not isinstance(s, VarDecl) or s.is_var or s.is_varip:
continue
if not in_loop:
continue
if not isinstance(s.value, FuncCall):
continue
if self._is_udt_lvalue(s.value) is None:
continue
if s.name not in field_mutated:
continue
self._udt_array_get_ref_locals.add(s.name)

def _extract_receiver_name(self, call_node) -> str | None:
"""Extract receiver Identifier name from m.method(...) or matrix.method(m, ...).

Expand Down Expand Up @@ -1980,6 +2053,12 @@ def generate(self) -> str:
for name, ptype in self.ctx.global_var_decls:
if name in seen_global or name in self.ctx.series_vars or name in self._var_names:
continue
# De-hoisted UDT array-element alias (Pine reference semantics): the
# in-loop VarDecl is emitted as a fresh ``UDT& z = arr[i];`` local
# reference each iteration, so there is no persistent class member.
if name in self._udt_array_get_ref_locals:
seen_global.add(name)
continue
seen_global.add(name)
safe = self._safe_name(name)

Expand Down
16 changes: 16 additions & 0 deletions pineforge_codegen/codegen/visit_stmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,22 @@ def remember_local_type(cpp_type: str | None) -> None:
lines.append(f"{pad}{cpp_type}& {safe} = {cpp_val};")
return

# Global-scope UDT array-element alias (BUG: Pine array elements of a
# user-defined type are references). A global loop-local bound from
# ``arr.get(i)`` and later field-mutated must ALIAS the element so the
# mutation writes back into the array. Emit a fresh per-iteration
# ``UDT& z = arr[i];`` reference (any hoisted class member was
# suppressed) — identical to the non-hoisted function-local alias path.
# Gated by _register_udt_array_get_ref_locals (which only records global-
# scope names); the ``_current_func_body is None`` guard confines this to
# global scope, so a function-local sharing the name keeps its own path.
if (node.name in self._udt_array_get_ref_locals
and getattr(self, "_current_func_body", None) is None):
udt_t = self._udt_var_types.get(node.name)
cpp_val = self._visit_rhs_value(node.value, node.name, target_cpp_type=udt_t)
lines.append(f"{pad}{udt_t}& {safe} = {cpp_val};")
return

# General declaration
cpp_type = self._type_for_decl(node) if not is_global_member else None
cpp_val = self._visit_rhs_value(node.value, node.name, target_cpp_type=cpp_type)
Expand Down
70 changes: 70 additions & 0 deletions tests/test_udt_lvalue_alias.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,76 @@ def test_array_get_udt_local_mutation_emits_reference_alias():
assert "p.crossed = true;" in body


_GLOBAL_ARRAY_PROLOGUE = PROLOGUE + """
var array<pivot> pivots = array.new<pivot>()
if array.size(pivots) == 0
array.push(pivots, pivot.new(na, false))
"""


def test_global_while_array_get_udt_mutation_emits_reference_alias():
# Regression: a GLOBAL-scope ``while`` loop-local bound from an array
# element and field-mutated is hoisted to a class member and its in-loop
# init lowered to a value-copy assignment, so the mutation was lost. It must
# instead ALIAS the element (Pine array elements of a UDT are references) so
# a later ``arr.get(i).f`` read sees the new value.
src = _GLOBAL_ARRAY_PROLOGUE + """
int wi = 0
while wi < array.size(pivots)
pivot p = array.get(pivots, wi)
p.currentLevel := close
p.crossed := true
wi += 1
plot(close)
"""
cpp = transpile(src)
# Fresh per-iteration reference to the array element (write-back), not copy.
assert re.search(r"pivot& p = pivots\[\(wi\)\];", cpp)
assert "pivot p = pivots[(wi)];" not in cpp
# The hoisted value member is suppressed (the alias replaces it).
assert "pivot p = pivot{};" not in cpp
# Mutations write through the reference with ``.`` member access.
assert "p.currentLevel = " in cpp
assert "p.crossed = true;" in cpp


def test_global_for_array_get_udt_mutation_emits_reference_alias():
# Same defect via a GLOBAL ``for i = ...`` loop. Here the get-local stays a
# true local (not hoisted), but the function-local alias path no-ops at
# global scope, so it also value-copied. Must alias for write-back.
src = _GLOBAL_ARRAY_PROLOGUE + """
for fi = 0 to array.size(pivots) - 1
pivot p = array.get(pivots, fi)
p.currentLevel := close
p.crossed := true
plot(close)
"""
cpp = transpile(src)
assert re.search(r"pivot& p = pivots\[\(fi\)\];", cpp)
assert "pivot p = pivots[(fi)];" not in cpp
assert "p.currentLevel = " in cpp
assert "p.crossed = true;" in cpp


def test_global_readonly_array_get_udt_stays_value_copy():
# CAUTION control locking the scope of the fix: a GLOBAL get-local that is
# only READ (never field-mutated) must keep value-copy semantics — no
# needless reference/pointer aliasing (this is what keeps the read-only
# tripwire strategies byte-identical).
src = _GLOBAL_ARRAY_PROLOGUE + """
float acc = 0.0
for ri = 0 to array.size(pivots) - 1
pivot p = array.get(pivots, ri)
acc := acc + p.currentLevel
plot(acc)
"""
cpp = transpile(src)
assert "pivot& p" not in cpp
assert "pivot* p" not in cpp
# Plain value copy of the element remains.
assert "pivot p = pivots[(ri)];" in cpp


def test_drawing_style_constant_into_string_param_coerced():
# The crash unmasked by the alias fix: label.style_* into a string param.
src = """//@version=6
Expand Down
Loading