From c48164f6faa47b7da8347e86423311a32d4a222b Mon Sep 17 00:00:00 2001 From: luisleo526 Date: Sat, 11 Jul 2026 09:48:01 +0800 Subject: [PATCH] Emit type-matched na sentinels on bare-na reassignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Pine `x := na` reassignment lowered as `x = na();` for every LHS type. Assigning NaN into an int/int64/bool member is UB: on ARM64 FCVTZS(NaN) yields 0, silently destroying the na sentinel (is_na stays false forever), while x86-64's CVTTSD2SI(NaN) coincidentally lands on INT64_MIN — the sentinel value — masking the bug on that platform. Exemplar: a daily session-open timer (int64, reset to na each new day) never re-armed on ARM64, permanently disabling an open-suppression gate and emitting phantom session-open entries. The constructor-initializer path already re-typed bare na to the member type; the two `:=` lowering sites and the function-var-init block did not. A new resolver returns the declared scalar type (int / int64_t with the epoch promotion / bool / string; None for double, collections, UDTs, and drawing types, which keep their current lowering), gated on the RHS being exactly a bare na. Provably neutral where the shape is absent: transpiling the entire corpus (254-265 sources) with and without the fix produces byte- identical output. The exemplar converges to an exact trade-for-trade TV match; the load-bearing int-na-reset sentinels are byte-identical. Co-Authored-By: Claude Fable 5 --- pineforge_codegen/codegen/emit_top.py | 5 + pineforge_codegen/codegen/types.py | 53 +++++++ pineforge_codegen/codegen/visit_stmt.py | 13 +- tests/test_na_reassign_retype.py | 183 ++++++++++++++++++++++++ 4 files changed, 252 insertions(+), 2 deletions(-) create mode 100644 tests/test_na_reassign_retype.py diff --git a/pineforge_codegen/codegen/emit_top.py b/pineforge_codegen/codegen/emit_top.py index 4f33b36..8fe498d 100644 --- a/pineforge_codegen/codegen/emit_top.py +++ b/pineforge_codegen/codegen/emit_top.py @@ -1216,6 +1216,11 @@ class member but its initializer was dropped, leaving the member ``na`` if (is_drawing or is_udt) and isinstance(init_ast, NaLiteral): continue init_cpp = self._visit_expr(init_ast) + # A bare-``na`` initializer for an int/int64_t/bool ``var`` member + # must be typed (``na()``), mirroring the class-scope ctor + # init (``_typed_na_init`` at _emit_constructor); a raw ``na()`` + # NaN stored into an int member is UB and defeats is_na(). + init_cpp = self._typed_na_init(init_cpp, name, ptype) init_lines.append(f" {target} = {init_cpp};") if not init_lines: return diff --git a/pineforge_codegen/codegen/types.py b/pineforge_codegen/codegen/types.py index 4ade0d4..678d35b 100644 --- a/pineforge_codegen/codegen/types.py +++ b/pineforge_codegen/codegen/types.py @@ -552,6 +552,59 @@ def _is_int64_builtin_init(self, name: str) -> bool: return True return name in self._int64_reassign_targets() + def _na_reassign_cpp_type(self, name: str) -> str | None: + """Declared scalar C++ type of a ``:=`` reassignment target ``name``, so a + bare-``na`` RHS (``x := na``) can be spelled ``na()`` matching the + member/local type instead of the default ``na()``. + + Assigning a double quiet-NaN into an ``int``/``int64_t``/``bool`` member is + undefined behaviour (NaN->int is unspecified; on ARM64 it saturates to 0, + which is not the ``na()`` sentinel) and defeats ``is_na()``. Mirrors + the member-declaration type logic (``base._emit_class_members`` / + ``_typed_na_init``): ``PINE_TYPE_TO_CPP`` plus the int->int64_t epoch + promotion. Returns ``None`` for collections / UDT / drawing handles and + for ``double`` (already the default lowering), so those paths are + unchanged. + """ + # Collections / UDT / drawing handles never take a scalar ``na()``: + # leave them to the drawing-na / default lowering in _visit_rhs_value. + if (name in self._array_vars + or name in self._map_vars + or name in getattr(self, "_matrix_specs", {}) + or name in self._udt_var_types): + return None + cpp_type: str | None = None + # 1. ``var`` member (class-scope OR function-local: both are recorded in + # ctx.var_members). This is the authoritative declaration source. + for vname, ptype, _init in self.ctx.var_members: + if vname == name: + cpp_type = PINE_TYPE_TO_CPP.get(ptype, "double") + break + # 2. Function-local plain (non-``var``) scalar: its declared type was + # remembered at the VarDecl (``_type_for_decl``). + if cpp_type is None: + cpp_type = getattr(self, "_current_func_local_types", {}).get(name) + # 3. Function parameter. + if cpp_type is None: + cpp_type = getattr(self, "_current_func_param_types", {}).get(name) + # 4. Global-scope non-``var`` class member. + if cpp_type is None: + for gname, gptype in self.ctx.global_var_decls: + if gname == name: + cpp_type = PINE_TYPE_TO_CPP.get(gptype, "double") + break + if cpp_type is None: + return None + # int -> int64_t promotion for epoch-ms builtins, mirroring the member + # declaration so the na sentinel width matches the storage width. + if cpp_type == "int" and self._is_int64_builtin_init(name): + cpp_type = "int64_t" + # Only the retypeable scalar types are meaningful; ``double`` already + # lowers to ``na()`` and everything else is left untouched. + if cpp_type in ("int", "int64_t", "bool", "std::string"): + return cpp_type + return None + # ------------------------------------------------------------------ # BUG C: user-defined-UDT lvalue aliasing # ------------------------------------------------------------------ diff --git a/pineforge_codegen/codegen/visit_stmt.py b/pineforge_codegen/codegen/visit_stmt.py index fb4c84f..7a1fccc 100644 --- a/pineforge_codegen/codegen/visit_stmt.py +++ b/pineforge_codegen/codegen/visit_stmt.py @@ -589,7 +589,14 @@ def _visit_assignment(self, node: Assignment, lines: list[str], pad: str) -> Non f"expected {self._type_spec_to_cpp(lhs_spec)}, " f"got {self._type_spec_to_cpp(rhs_spec)}", ) - val_cpp = self._visit_rhs_value(node.value, target_name) + # A bare-``na`` reassignment must adopt the target's declared scalar + # type (``x := na`` -> ``na()`` not ``na()``); otherwise + # a double NaN is stored into an int/int64_t/bool member (UB, defeats + # is_na()). Only computed for bare na — every other RHS is + # unaffected. + tct = (self._na_reassign_cpp_type(target_name) + if self._is_na_expr(node.value) else None) + val_cpp = self._visit_rhs_value(node.value, target_name, target_cpp_type=tct) if node.op == ":=": lines.append(f"{pad}{safe} = {val_cpp};") else: @@ -599,7 +606,9 @@ def _visit_assignment(self, node: Assignment, lines: list[str], pad: str) -> Non else: lines.append(f"{pad}{safe} {node.op} {val_cpp};") else: - val_cpp = self._visit_rhs_value(node.value, target_name) + tct = (self._na_reassign_cpp_type(target_name) + if self._is_na_expr(node.value) else None) + val_cpp = self._visit_rhs_value(node.value, target_name, target_cpp_type=tct) if node.op == ":=": lines.append(f"{pad}{safe} = {val_cpp};") else: diff --git a/tests/test_na_reassign_retype.py b/tests/test_na_reassign_retype.py new file mode 100644 index 0000000..375906b --- /dev/null +++ b/tests/test_na_reassign_retype.py @@ -0,0 +1,183 @@ +"""Regression: a bare-``na`` REASSIGNMENT (``x := na``) must lower to a typed +``na()`` matching the target's declared C++ type, not the unconditional +``na()``. + +Root cause: ``visit_expr._visit_ident`` renders a bare ``na`` as +``na()``. The *initializer* path retypes it (``_typed_na_init`` / +``_visit_rhs_value(target_cpp_type=...)``), but the ``:=`` reassignment path +called ``_visit_rhs_value`` WITHOUT a target type, so the retype branch never +fired and ``x := na`` emitted ``na()`` for every LHS type. + +Assigning a double quiet-NaN into an ``int``/``int64_t`` member is undefined +behaviour: on ARM64 ``FCVTZS(NaN) == 0`` (not the ``na()`` sentinel +``INT64_MIN``), so ``is_na()`` is false forever. Proven in +``gonzowiththewind-sisyphus-happiness``: an ``int64_t sessionOpenTime`` daily +open-suppress timer, reset with ``sessionOpenTime := na`` each new day, was +permanently disarmed because the reset stored ``na()`` instead of +``na()``. +""" + +from __future__ import annotations + +import re + +import pytest + +from pineforge_codegen import transpile + +from tests._compile import compile_cpp, have_compile_env + + +def _assign_rhs(cpp: str, lhs: str) -> list[str]: + """Return the RHS of every `` = ;`` statement (plain assignment, + not declaration / member-init-list) in the generated C++.""" + out = [] + pat = re.compile(rf"(?])\b{re.escape(lhs)}\s*=\s*(na<[^;]*>\(\))\s*;") + for m in pat.finditer(cpp): + out.append(m.group(1)) + return out + + +# -------------------------------------------------------------------------- +# (a) class-scope var int, never holds time -> declared ``int`` -> na() +# -------------------------------------------------------------------------- +def test_class_scope_int_var_na_reassign_retypes_to_int(): + src = """//@version=6 +strategy("t") +var int slot = na +if close > open + slot := na +plot(na(slot) ? na : 1.0) +""" + cpp = transpile(src) + assert "int slot;" in cpp + assert "slot = na();" in cpp, cpp + # The reassignment must NOT store a double NaN into an int member. + assert "slot = na();" not in cpp + + +# -------------------------------------------------------------------------- +# (d)/(gonzo) int var ALSO reassigned to ``time`` -> promoted to int64_t; +# BOTH the ``:= na`` reset AND the ``:= time`` set must use the int64 member. +# -------------------------------------------------------------------------- +def test_int64_promoted_var_na_reassign_retypes_to_int64(): + src = """//@version=6 +strategy("t") +var int sessionOpenTime = na +newDay = ta.change(dayofweek) != 0 +if newDay + sessionOpenTime := na +if not na(sessionOpenTime) + strategy.close("L") +if close > open + sessionOpenTime := time +plot(close) +""" + cpp = transpile(src) + assert "int64_t sessionOpenTime;" in cpp + # The na reset must match the promoted member type, not na(). + assert "sessionOpenTime = na();" in cpp, cpp + assert "sessionOpenTime = na();" not in cpp + + +# -------------------------------------------------------------------------- +# (b) function-local var int -> lifted to an int member; both the once-only +# init block AND the ``:= na`` reassignment must be typed. +# -------------------------------------------------------------------------- +def test_function_local_int_var_na_reassign_retypes_to_int(): + src = """//@version=6 +strategy("t") +f() => + var int et = na + if bar_index > 5 + et := na + na(et) ? 0.0 : 1.0 +plot(f()) +""" + cpp = transpile(src) + assert "int et;" in cpp + # No double-NaN slip into the int member (init block OR reassignment). + assert "et = na();" not in cpp, cpp + assert "et = na();" in cpp + + +# -------------------------------------------------------------------------- +# (c) float var -> declared double -> the na spelling stays na() +# (guard against over-reach). +# -------------------------------------------------------------------------- +def test_float_var_na_reassign_stays_double(): + src = """//@version=6 +strategy("t") +var float px = na +if close > open + px := na +plot(px) +""" + cpp = transpile(src) + assert "double px;" in cpp + assert "px = na();" in cpp + assert "px = na();" not in cpp + assert "px = na();" not in cpp + + +# -------------------------------------------------------------------------- +# (e) bool var -> na() +# -------------------------------------------------------------------------- +def test_bool_var_na_reassign_retypes_to_bool(): + src = """//@version=6 +strategy("t") +var bool flag = na +if close > open + flag := na +plot(na(flag) ? na : 1.0) +""" + cpp = transpile(src) + assert "bool flag;" in cpp + assert "flag = na();" in cpp, cpp + assert "flag = na();" not in cpp + + +# -------------------------------------------------------------------------- +# (f) faithful gonzo daily-open-timer reduction: the disarm bug in miniature. +# -------------------------------------------------------------------------- +def test_gonzo_daily_open_timer_reduction(): + src = """//@version=6 +strategy("gonzo-lite") +var int sessionOpenTime = na +newDay = ta.change(dayofweek) != 0 +inRTH = hour >= 9 and hour < 16 +if newDay + sessionOpenTime := na +if inRTH and na(sessionOpenTime) + sessionOpenTime := time +secSinceOpen = na(sessionOpenTime) ? na : (time - sessionOpenTime) / 1000 +if not na(secSinceOpen) and secSinceOpen > 3600 + strategy.entry("L", strategy.long) +plot(close) +""" + cpp = transpile(src) + assert "int64_t sessionOpenTime;" in cpp + rhs = _assign_rhs(cpp, "sessionOpenTime") + # Every na reassignment of the timer must be int64-typed. + assert rhs, cpp + assert all(r == "na()" for r in rhs), rhs + + +# -------------------------------------------------------------------------- +# Compile-level guard: the fixed gonzo-like emit is well-formed against the +# public engine headers (skips cleanly when the engine include is absent). +# -------------------------------------------------------------------------- +@pytest.mark.skipif(not have_compile_env(), reason="engine include / compiler unavailable") +def test_int64_na_reassign_compiles(): + src = """//@version=6 +strategy("t") +var int sessionOpenTime = na +newDay = ta.change(dayofweek) != 0 +if newDay + sessionOpenTime := na +if na(sessionOpenTime) + sessionOpenTime := time +plot(close) +""" + cpp = transpile(src) + compile_cpp(cpp, label="int64-na-reassign")