diff --git a/pineforge_codegen/analyzer/diagnostics.py b/pineforge_codegen/analyzer/diagnostics.py index 7d199b8..2838373 100644 --- a/pineforge_codegen/analyzer/diagnostics.py +++ b/pineforge_codegen/analyzer/diagnostics.py @@ -89,8 +89,22 @@ def _warn_if_unknown_source_id( loc, ) + # Non-atomic node kinds whose serialized infix form must be parenthesized + # when used as an operand, so the string re-parses to the SAME tree. A TA + # ctor-arg string produced here is later re-parsed and lowered by the + # codegen (``_runtime_ctor_arg_for_reset``); a flattened ``(a - b) / c`` + # would otherwise silently reassociate under C++ precedence. + _NONATOMIC_EXPR_NODES = (BinOp, UnaryOp, Ternary) + + def _operand_to_str(self, node: ASTNode) -> str: + s = self._expr_to_str(node) + if isinstance(node, self._NONATOMIC_EXPR_NODES): + return f"({s})" + return s + def _expr_to_str(self, node: ASTNode) -> str: - """Convert an expression node to a rough string representation.""" + """Convert an expression node to a string that re-parses to the same + tree (grouping preserved for non-atomic operands).""" if isinstance(node, NumberLiteral): return str(node.value) if isinstance(node, StringLiteral): @@ -104,9 +118,9 @@ def _expr_to_str(self, node: ASTNode) -> str: if isinstance(node, MemberAccess): return f"{self._expr_to_str(node.object)}.{node.member}" if isinstance(node, BinOp): - return f"{self._expr_to_str(node.left)} {node.op} {self._expr_to_str(node.right)}" + return f"{self._operand_to_str(node.left)} {node.op} {self._operand_to_str(node.right)}" if isinstance(node, UnaryOp): - return f"{node.op}{self._expr_to_str(node.operand)}" + return f"{node.op}{self._operand_to_str(node.operand)}" if isinstance(node, FuncCall): args = ", ".join(self._expr_to_str(a) for a in node.args) callee_str = self._expr_to_str(node.callee) @@ -114,5 +128,5 @@ def _expr_to_str(self, node: ASTNode) -> str: if isinstance(node, Subscript): return f"{self._expr_to_str(node.object)}[{self._expr_to_str(node.index)}]" if isinstance(node, Ternary): - return f"{self._expr_to_str(node.condition)} ? {self._expr_to_str(node.true_val)} : {self._expr_to_str(node.false_val)}" + return f"{self._operand_to_str(node.condition)} ? {self._operand_to_str(node.true_val)} : {self._operand_to_str(node.false_val)}" return "" diff --git a/pineforge_codegen/codegen/base.py b/pineforge_codegen/codegen/base.py index 3b0ff36..af04ea5 100644 --- a/pineforge_codegen/codegen/base.py +++ b/pineforge_codegen/codegen/base.py @@ -191,6 +191,13 @@ def __init__(self, ctx: AnalyzerContext) -> None: self._func_cs_var_remap: dict[tuple[str, int], dict[str, str]] = {} # Active var name remap (set during per-call-site function emission) self._active_var_remap: dict[str, str] = {} + # When True (only while lowering a TA runtime-reset length expression + # through the expression visitor), an input-backed variable identifier + # renders as an override-aware ``get_input_*()`` read instead of its + # class member name. The reset can run in ``evaluate_security`` BEFORE + # the input members are initialised, so it must not depend on their + # init order. See ``_lower_reset_expr_via_visitor``. + self._reset_input_getter_mode: bool = False # Set of var/series member names that belong to user functions (need cloning) self._func_var_members_set: set[str] = set() # BUG C: function-local names emitted as ``UDT*`` pointer aliases (a UDT @@ -1434,11 +1441,31 @@ def _expr_is_stable(self, node) -> bool: return False return False + # AST node kinds whose serialized form is self-delimiting (a literal, a + # name, a member read, or a ``name(...)`` call). Non-atomic kinds (BinOp / + # UnaryOp / Ternary) MUST be parenthesized when they appear as an operand, + # otherwise re-parsing the flattened infix string silently reassociates the + # tree: Pine grouping ``(a - b) / (c - d)`` degrades to ``a - b / c - d`` + # under C++ precedence. See ``_runtime_ctor_arg_for_reset`` (the string is + # re-parsed and lowered through the expression visitor). + _ATOMIC_ARITH_NODES = (NumberLiteral, Identifier, MemberAccess, FuncCall) + + def _arith_operand_to_str(self, node) -> str | None: + """Serialize ``node`` for use as an operand: parenthesize it unless its + serialized form is already self-delimiting, so grouping survives a + round-trip through the parser.""" + s = self._arith_expr_to_str(node) + if s is None: + return None + if isinstance(node, self._ATOMIC_ARITH_NODES): + return s + return f"({s})" + def _arith_expr_to_str(self, node) -> str | None: """Render a numeric arithmetic-over-identifiers expression to a string - whose token spelling matches ``_resolve_known`` / ``_runtime_ctor_arg_for_reset`` - (e.g. ``rsiLen * 2 - 1``). Returns None for any node shape we don't fold - (series subscripts, ternaries, etc.) so the caller leaves the var untracked. + that re-parses to the SAME tree (grouping preserved via + ``_arith_operand_to_str``). Returns None for any node shape we don't + fold (series subscripts, etc.) so the caller leaves the var untracked. """ if isinstance(node, NumberLiteral): v = node.value @@ -1450,20 +1477,20 @@ def _arith_expr_to_str(self, node) -> str | None: if isinstance(node, MemberAccess) and isinstance(node.object, Identifier): return f"{node.object.name}.{node.member}" if isinstance(node, BinOp): - l = self._arith_expr_to_str(node.left) - r = self._arith_expr_to_str(node.right) + l = self._arith_operand_to_str(node.left) + r = self._arith_operand_to_str(node.right) if l is None or r is None: return None return f"{l} {node.op} {r}" if isinstance(node, UnaryOp): - o = self._arith_expr_to_str(node.operand) + o = self._arith_operand_to_str(node.operand) if o is None: return None return f"{node.op}{o}" if isinstance(node, Ternary): - c = self._arith_expr_to_str(node.condition) - t = self._arith_expr_to_str(node.true_val) - f = self._arith_expr_to_str(node.false_val) + c = self._arith_operand_to_str(node.condition) + t = self._arith_operand_to_str(node.true_val) + f = self._arith_operand_to_str(node.false_val) if c is None or t is None or f is None: return None return f"{c} ? {t} : {f}" @@ -2479,6 +2506,18 @@ def _rep(p: re.Match) -> str: if not (has_input or has_timeframe or has_inline_input): return None + # Preferred path: re-parse the (gate-approved, grouping-faithful) + # expression and lower it through the SAME expression visitor the + # statement path uses. Operator grouping and Pine numeric typing + # (``/`` always yields float; ``(double)`` coercion) are then correct + # by construction — reused from ``_visit_binop``, not re-derived here. + # Falls back to the legacy token-substitution renderer below only if + # re-parse / lowering unexpectedly fails, so a working site is never + # worse off than before this change. + lowered = self._lower_reset_expr_via_visitor(expanded) + if lowered is not None: + return lowered + # Render inline input() calls to override-aware get_input_*() reads # now that the gate has accepted the expression. Done after the gate # so the rendered getter tokens (get_input_int, ...) do not have to @@ -2546,6 +2585,50 @@ def _sub(p: re.Match) -> str: return f"(int)({rewritten})" return rewritten + def _lower_reset_expr_via_visitor(self, expanded: str) -> str | None: + """Re-parse a gate-approved, fully-expanded TA-length expression and + lower it through the SAME expression visitor the statement path uses, + so operator grouping and Pine numeric typing are preserved identically + (Pine ``/`` always yields float; ``math.*`` returns double; branches are + parenthesized). Reuses ``_visit_binop`` etc.; nothing is re-typed here. + + Input-backed variables render as override-aware ``get_input_*()`` reads + (not member refs) via ``_reset_input_getter_mode`` — the reset can run + in ``evaluate_security`` before the input members are initialised, so it + must not depend on their init order. Inline ``input(...)`` calls and + ``math.*`` / ``timeframe.*`` members lower through the visitor's own + handlers (same C++ the statement path would emit). + + Returns None if re-parse / lowering fails, so the caller falls back to + the legacy token-substitution renderer (never worse than before).""" + try: + from ..lexer import Lexer + from ..parser import Parser + tokens = Lexer(expanded).tokenize() + node = Parser(tokens, source=expanded)._parse_expression() + except Exception: + return None + prev = self._reset_input_getter_mode + self._reset_input_getter_mode = True + try: + rendered = self._visit_expr(node) + except Exception: + return None + finally: + self._reset_input_getter_mode = prev + if not rendered or "/* " in rendered: + # Unknown/unhandled node leaked a placeholder — defer to legacy. + return None + # A TA length must be int. Truncate when the lowered form is + # float-typed (a ``std::*`` call, a ``(double)`` division coercion, or a + # ``timeframe.*`` helper — all timeframe helpers reference script_tf_). + # A bare int / identifier length carries none of these and is left + # unwrapped, so simple sites stay byte-identical to the legacy output. + if ("std::" in rendered or "(double)" in rendered + or "script_tf_" in rendered): + return f"(int)({rendered})" + return rendered + def _collect_ta_runtime_resets(self) -> list[str]: """Collect reassignment statements for every TA object whose ctor args diff --git a/pineforge_codegen/codegen/visit_expr.py b/pineforge_codegen/codegen/visit_expr.py index 8f6434c..a4972b3 100644 --- a/pineforge_codegen/codegen/visit_expr.py +++ b/pineforge_codegen/codegen/visit_expr.py @@ -275,6 +275,16 @@ def _visit_ident(self, node: Identifier) -> str: return str(val) if isinstance(val, str): return f'std::string("{val}")' + # TA runtime-reset lowering: an input-backed var renders as its + # override-aware getter (not the member name), because the reset may + # run before the input members are initialised (evaluate_security path). + if (self._reset_input_getter_mode + and name in self._input_backed_vars + and name in self._input_var_to_call): + call_node = self._input_var_to_call[name] + func_name_i, namespace_i = self._resolve_callee(call_node.callee) + title = self._get_input_title(call_node, var_name=name) + return self._render_input_value(call_node, func_name_i, namespace_i, title) # Pine type name `color` used as a value (no variable) → int64 color constant. # Params handled above; symbol table does not retain function locals after analysis. if name == "color": diff --git a/tests/test_codegen_ta_derived_length.py b/tests/test_codegen_ta_derived_length.py index 6035a64..871a851 100644 --- a/tests/test_codegen_ta_derived_length.py +++ b/tests/test_codegen_ta_derived_length.py @@ -66,7 +66,9 @@ def test_class_scope_arithmetic_over_input_length(): # And the runtime reset re-derives it from the (possibly overridden) input. reset = _reset_line(cpp, member) assert 'get_input_int("RSI Length", 14)' in reset - assert "* 2 - 1" in reset + # Reset lowers through the expression visitor, which parenthesizes operands + # (grouping preserved: ``(rsiLen * 2) - 1``, not a flattened infix string). + assert '(get_input_int("RSI Length", 14) * 2) - 1' in reset assert "ta::EMA(1)" not in reset # must NOT be the silent no-op @@ -95,7 +97,7 @@ def test_function_local_derived_length(): for m in sized: reset = _reset_line(cpp, m) assert 'get_input_int("Smooth EMA Length", 5)' in reset - assert "* 2 - 1" in reset + assert '(get_input_int("Smooth EMA Length", 5) * 2) - 1' in reset assert "ta::EMA(1)" not in cpp @@ -529,3 +531,68 @@ def test_cloned_no_ctor_ta_sites_declared_through_dead_caller(): "dead adx_dead body should not be emitted" ) + +# --------------------------------------------------------------------------- +# Regression: the stable-runtime-length reset re-materializes the length by +# re-parsing the expanded expression and lowering it through the SAME +# expression visitor the statement path uses. Two Pine semantics MUST survive: +# (1) operator grouping (a parenthesized +/- subexpression under a division), +# (2) float division of two int operands (Pine `/` never truncates). +# Both used to be destroyed by the old lossy AST->infix serializer + +# token-substitution renderer (grouping flattened, int/int stayed integer +# division), collapsing a derived TA length to 1 and trading zero times. +# --------------------------------------------------------------------------- + +def test_reset_preserves_grouping_across_division(): + # length = int(round((a + b) / (a - b))). Pine groups the sum over the + # difference: (10+3)/(10-3) = 13/7 ≈ 1.857 → 2. If the serializer drops the + # parentheses the string reassociates under C++ precedence to + # `a + b / a - b` = 10 + 0.3 - 3 ≈ 7.3 → 7 — a different length. The reset + # must emit the difference as one grouped divisor, coerced to double. + src = """//@version=6 +strategy("grp") +a = input.int(10, "A") +b = input.int(3, "B") +lenv = int(math.round((a + b) / (a - b))) +plot(ta.ema(close, lenv)) +""" + cpp = transpile(src) + members = re.findall(r"(_ta_ema_\d+)\(", cpp) + assert members, "no EMA member emitted" + reset = _reset_line(cpp, members[0]) + assert reset, "no runtime reset emitted for grouped-division length" + # Pine `/` is float division → both operands coerced to double. + assert "(double)(" in reset + # The difference (a - b) survives as one grouped divisor unit — NOT + # reassociated so that `+ b` and `- b` become sibling additive terms. + assert '(get_input_int("A", 10) - get_input_int("B", 3))' in reset + assert '(get_input_int("A", 10) + get_input_int("B", 3))' in reset + # The pre-fix flattened form would place the sum's `+ ... /` and a trailing + # `- get_input_int("B", 3)` as siblings of the top-level division. + assert '+ get_input_int("B", 3) /' not in reset + assert "ta::EMA(1)" not in reset # must NOT collapse to the silent no-op + + +def test_reset_uses_float_division_for_two_int_operands(): + # 2 / poles must be FLOAT division (Pine semantics): 2/4 = 0.5, so + # pow(2, 0.5) = 1.414…, *10 → 14.1 → round 14. Integer division would give + # 2/4 = 0 → pow(2, 0) = 1 → *10 → round 10 — the discriminant/length + # collapse behind the adxae zero-trades bug. + src = """//@version=6 +strategy("fdiv") +poles = input.int(4, "Poles") +lenv = int(math.round(10 * math.pow(2.0, 2 / poles))) +plot(ta.ema(close, lenv)) +""" + cpp = transpile(src) + members = re.findall(r"(_ta_ema_\d+)\(", cpp) + assert members, "no EMA member emitted" + reset = _reset_line(cpp, members[0]) + assert reset, "no runtime reset emitted for float-division length" + # The int input operand is coerced to double for the division; the bare + # `2 / get_input_int(...)` integer-division form (the bug) must be gone. + assert '(double)(get_input_int("Poles", 4))' in reset + assert '2 / get_input_int("Poles", 4)' not in reset + assert "std::pow" in reset + assert "ta::EMA(1)" not in reset +