diff --git a/pineforge_codegen/codegen/base.py b/pineforge_codegen/codegen/base.py index 6d46379..3ec2245 100644 --- a/pineforge_codegen/codegen/base.py +++ b/pineforge_codegen/codegen/base.py @@ -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 @@ -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 @@ -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, ...). @@ -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) diff --git a/pineforge_codegen/codegen/visit_stmt.py b/pineforge_codegen/codegen/visit_stmt.py index 8e30e92..3769bf9 100644 --- a/pineforge_codegen/codegen/visit_stmt.py +++ b/pineforge_codegen/codegen/visit_stmt.py @@ -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) diff --git a/tests/test_udt_lvalue_alias.py b/tests/test_udt_lvalue_alias.py index 543e5ae..50e1fd3 100644 --- a/tests/test_udt_lvalue_alias.py +++ b/tests/test_udt_lvalue_alias.py @@ -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 pivots = array.new() +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