diff --git a/docs/codegen-coverage-gaps.md b/docs/codegen-coverage-gaps.md index 4317896..36b3a38 100644 --- a/docs/codegen-coverage-gaps.md +++ b/docs/codegen-coverage-gaps.md @@ -76,3 +76,55 @@ were added to convert the most fragile defensive paths from silent-on-bug to loud-on-bug. No new follow-up work identified by this audit. + +## Residuals left open by the array-bounds guard (2026-07-25) + +`array.insert`, the 3-argument `array.fill`, and `array.slice` now reject an +out-of-range index instead of emitting raw STL iterator arithmetic (undefined +behaviour). Three questions the bounds work deliberately does NOT settle: + +1. **`array.slice` shares storage with its source; codegen deep-copies.** + The Pine v6 reference says "Changing the elements in a slice directly + changes the elements in the original array, and vice versa", while both + lowerings construct a fresh `std::vector` from the iterator range. Any + write through a slice (or through the source while a slice is live) is + therefore lost. Closing this needs a view/alias type over the receiver's + storage, not an index check, so it is a separate and much larger change. + The extent of the aliasing is itself unsettled: the reference sentence + says "an object from the slice", which is unambiguous for arrays of + reference-like elements (UDT instances) but does NOT establish that a + primitive `array` slice aliases element-for-element. Pin that + against TradingView before designing the view type — the answer decides + whether the fix is "arrays of UDTs only" or "all arrays". + +2. **Inverted ranges (`index_from > index_to`) are rejected, not clamped.** + No evidence pins TradingView's behaviour for `array.fill(id, v, 2, 1)` / + `array.slice(id, 2, 1)`. Rejecting is the fail-closed choice and the + 3-argument `fill` range form has no current corpus exposure, but a + TradingView probe could show TV treats it as an empty range instead. + +3. **Negative endpoints for `fill`/`slice` are rejected.** The reference + lists only `array.get`/`array.set`/`array.insert`/`array.remove` as + negative-indexing functions, so `fill`/`slice` follow `array.percentrank` + and reject. If TradingView in fact normalises them, this is an + over-rejection (a valid script fails to transpile), not a wrong result. + +## Residual left open by the request.security rebind guard (2026-07-25) + +`_scalar_rebinds` is keyed by BARE NAME with no scope resolution, mirroring +the existing `_scalar_defs`. A divergent `:=` rebind of a *local* named `sym` +inside a user function therefore also disqualifies an unrelated *global* +`sym` used as a `request.security` symbol. That direction is fail-closed +(over-rejection, never a silent wrong-symbol run), but a scoped symbol table +would be the exact fix. A 788-source differential shows the over-rejection +costs nothing on the current corpus. + +**Still open — the ternary branch hole (KI-47(a)).** `_is_current_symbol_expr` +still accepts `cond ? "EXCH:OTHER" : syminfo.tickerid` because EITHER branch +resolving to the chart symbol is enough. Tightening it to require BOTH +branches is correct in principle but rejects +`data/standard/doriannnq-tjr-v4-strategy`, whose alternate branch is +unreachable at its declared input configuration (`smtSym` defaults to `""`) +and which grades Excellent at 44/44 / 100% match. Closing this needs the +checker to see the effective input configuration, which it currently cannot +(see the analysis in the R6 handoff). Held deliberately, not overlooked. diff --git a/pineforge_codegen/codegen/tables.py b/pineforge_codegen/codegen/tables.py index e057ec4..a146333 100644 --- a/pineforge_codegen/codegen/tables.py +++ b/pineforge_codegen/codegen/tables.py @@ -459,45 +459,143 @@ def tz_time_field_lambda(field_expr: str, ts_arg: str, tz_arg: str) -> str: # --------------------------------------------------------------------------- -def _checked_array_index_prelude(*, normalize_negative: bool = True) -> str: +def _checked_array_index_prelude( + *, + normalize_negative: bool = True, + allow_size: bool = False, + name: str = "index", +) -> str: """C++ body shared by checked single-index operations. - Pine v6 explicitly gives end-relative negative indices to the checked - ``get``, ``set``, and ``remove`` cluster. Other indexed functions such as - ``percentrank`` still need the same NA/non-finite/range checks, but must - reject negative indices instead of normalizing them. ``insert`` remains a - separately bounded residual. + Pine v6 explicitly gives end-relative negative indices to ``array.get``, + ``array.set``, ``array.insert``, and ``array.remove``. Other indexed + functions such as ``percentrank`` and the ``fill``/``slice`` range + endpoints still need the same NA/non-finite/range checks, but must reject + negative indices instead of normalizing them (``normalize_negative``). + + ``allow_size`` widens the upper bound from ``index < size`` to + ``index <= size``. An *element access* is invalid at ``size``, but an + *insertion point* is not: ``array.insert(id, size, v)`` appends, and the + half-open ``[index_from, index_to)`` ranges of ``array.fill`` / + ``array.slice`` legally end at ``size``. + + ``name`` renames every emitted local so two endpoints can be checked in + one C++ scope without colliding. ``name="index"`` reproduces the original + identifiers byte-for-byte. """ + raw_value = f"__pf_raw_{name}_value" + raw_type = f"__pf_raw_{name}_type" + raw_text = f"__pf_raw_{name}_text" + raw_wide = f"__pf_raw_{name}_wide" + raw = f"__pf_raw_{name}" + checked = f"__pf_array_{name}" + size = "__pf_array_size" if name == "index" else f"__pf_array_size_{name}" checked_index = ( - "__pf_raw_index<0?__pf_raw_index+__pf_array_size:__pf_raw_index" - if normalize_negative - else "__pf_raw_index" + f"{raw}<0?{raw}+{size}:{raw}" if normalize_negative else raw ) + upper = ">" if allow_size else ">=" return ( - "using __pf_raw_index_type=std::decay_t; " - "if constexpr(!std::is_same_v<__pf_raw_index_type,bool>) { " - "if(is_na(__pf_raw_index_value)) " + f"using {raw_type}=std::decay_t; " + f"if constexpr(!std::is_same_v<{raw_type},bool>) {{ " + f"if(is_na({raw_value})) " "pine_runtime_error(std::string(\"Index na is out of bounds. Array size is \")+" "std::to_string((int64_t)__pf_array.size())); } " - "if constexpr(std::is_floating_point_v<__pf_raw_index_type>) { " - "if(!std::isfinite(__pf_raw_index_value)) { " - "std::string __pf_raw_index_text=__pf_raw_index_value>0?\"inf\":\"-inf\"; " - "pine_runtime_error(std::string(\"Index \")+__pf_raw_index_text+" + f"if constexpr(std::is_floating_point_v<{raw_type}>) {{ " + f"if(!std::isfinite({raw_value})) {{ " + f"std::string {raw_text}={raw_value}>0?\"inf\":\"-inf\"; " + f"pine_runtime_error(std::string(\"Index \")+{raw_text}+" "\" is out of bounds. Array size is \"+" "std::to_string((int64_t)__pf_array.size())); } " - "long double __pf_raw_index_wide=(long double)__pf_raw_index_value; " - "if(__pf_raw_index_wide<(long double)std::numeric_limits::min()||" - "__pf_raw_index_wide>(long double)std::numeric_limits::max()) " + f"long double {raw_wide}=(long double){raw_value}; " + f"if({raw_wide}<(long double)std::numeric_limits::min()||" + f"{raw_wide}>(long double)std::numeric_limits::max()) " "pine_runtime_error(std::string(\"Index \")+" - "std::to_string((double)__pf_raw_index_value)+" + f"std::to_string((double){raw_value})+" "\" is out of bounds. Array size is \"+" "std::to_string((int64_t)__pf_array.size())); } " - "int64_t __pf_raw_index=(int64_t)__pf_raw_index_value; " - "int64_t __pf_array_size=(int64_t)__pf_array.size(); " - f"int64_t __pf_array_index={checked_index}; " - "if(__pf_array_index<0||__pf_array_index>=__pf_array_size) " - "pine_runtime_error(std::string(\"Index \")+std::to_string(__pf_raw_index)+" - "\" is out of bounds. Array size is \"+std::to_string(__pf_array_size)); " + f"int64_t {raw}=(int64_t){raw_value}; " + f"int64_t {size}=(int64_t)__pf_array.size(); " + f"int64_t {checked}={checked_index}; " + f"if({checked}<0||{checked}{upper}{size}) " + f"pine_runtime_error(std::string(\"Index \")+std::to_string({raw})+" + f"\" is out of bounds. Array size is \"+std::to_string({size})); " + ) + + +def _checked_array_range_prelude() -> str: + """Validate the half-open ``[index_from, index_to)`` range of fill/slice. + + Both endpoints are checked with ``allow_size`` (``index_to`` is exclusive, + so ``size`` is a legal endpoint) and without negative normalization + (neither function appears in the Pine v6 reference's negative-indexing + set). An inverted range is rejected rather than silently treated as empty: + ``begin()+from > begin()+to`` is undefined behaviour in the STL forms this + replaces, and no evidence pins TradingView's behaviour there. + """ + return ( + _checked_array_index_prelude( + normalize_negative=False, allow_size=True, name="index_from" + ) + + _checked_array_index_prelude( + normalize_negative=False, allow_size=True, name="index_to" + ) + + "if(__pf_array_index_from>__pf_array_index_to) " + "pine_runtime_error(std::string(\"Index range \")+" + "std::to_string(__pf_array_index_from)+\"..\"+" + "std::to_string(__pf_array_index_to)+" + "\" is invalid. Array size is \"+" + "std::to_string(__pf_array_size_index_from)); " + ) + + +def _checked_array_insert(a: str, args: list[str]) -> str: + """``array.insert(id, index, value)`` — index is an INSERTION point. + + ``index == size`` appends, so the bound is ``index <= size``; a negative + index is end-relative, exactly as for ``get``/``set``/``remove``. + """ + check = _checked_array_index_prelude(allow_size=True) + return ( + "[&](auto&& __pf_array){ " + "return [&](auto&& __pf_raw_index_value){ " + "return [&](auto&& __pf_array_value){ " + f"{check}" + "__pf_array.insert(__pf_array.begin()+(size_t)__pf_array_index, " + "__pf_array_value); " + f"}}(({args[1]})); }}(({args[0]})); }}(({a}))" + ) + + +def _checked_array_fill_range(a: str, args: list[str]) -> str: + """``array.fill(id, value, index_from, index_to)`` — bounded range fill.""" + check = _checked_array_range_prelude() + return ( + "[&](auto&& __pf_array){ " + "return [&](auto&& __pf_array_value){ " + "return [&](auto&& __pf_raw_index_from_value){ " + "return [&](auto&& __pf_raw_index_to_value){ " + f"{check}" + "std::fill(__pf_array.begin()+(size_t)__pf_array_index_from, " + "__pf_array.begin()+(size_t)__pf_array_index_to, __pf_array_value); " + f"}}(({args[2]})); }}(({args[1]})); }}(({args[0]})); }}(({a}))" + ) + + +def checked_array_slice(a: str, args: list[str], *, result_type: str) -> str: + """``array.slice(id, index_from, index_to)`` — bounded range extraction. + + ``result_type`` stays caller-supplied so the typed method lane keeps + emitting the receiver's own element type; only the bounds checks are new. + """ + check = _checked_array_range_prelude() + return ( + "[&](auto&& __pf_array){ " + "return [&](auto&& __pf_raw_index_from_value){ " + "return [&](auto&& __pf_raw_index_to_value){ " + f"{check}" + f"return {result_type}(__pf_array.begin()+(size_t)__pf_array_index_from, " + "__pf_array.begin()+(size_t)__pf_array_index_to); " + f"}}(({args[1]})); }}(({args[0]})); }}(({a}))" ) @@ -600,7 +698,7 @@ def _checked_array_percentrank(a: str, args: list[str]) -> str: "set": _checked_array_set, "push": lambda a, args: f"{a}.push_back({args[0]})", "unshift": lambda a, args: f"{a}.insert({a}.begin(), {args[0]})", - "insert": lambda a, args: f"{a}.insert({a}.begin() + (int)({args[0]}), {args[1]})", + "insert": _checked_array_insert, "pop": lambda a, args: _checked_array_end_remove(a, "pop"), "shift": lambda a, args: _checked_array_end_remove(a, "shift"), "remove": _checked_array_remove, @@ -609,7 +707,7 @@ def _checked_array_percentrank(a: str, args: list[str]) -> str: "size": lambda a, args: f"(double){a}.size()", "clear": lambda a, args: f"{a}.clear()", "fill": lambda a, args: f"std::fill({a}.begin(), {a}.end(), {args[0]})" if len(args) == 1 - else f"std::fill({a}.begin()+(int)({args[1]}), {a}.begin()+(int)({args[2]}), {args[0]})", + else _checked_array_fill_range(a, args), "includes": lambda a, args: f"(std::find({a}.begin(), {a}.end(), {args[0]}) != {a}.end())", "indexof": lambda a, args: f"[&](){{ auto it=std::find({a}.begin(),{a}.end(),{args[0]}); return it!={a}.end()?(double)(it-{a}.begin()):-1.0; }}()", "lastindexof": lambda a, args: f"[&](){{ for(int i=(int){a}.size()-1;i>=0;i--)if({a}[i]=={args[0]})return(double)i; return -1.0; }}()", @@ -619,7 +717,7 @@ def _checked_array_percentrank(a: str, args: list[str]) -> str: ), "reverse": lambda a, args: f"std::reverse({a}.begin(),{a}.end())", "copy": lambda a, args: f"std::vector({a})", - "slice": lambda a, args: f"std::vector({a}.begin()+(int)({args[0]}),{a}.begin()+(int)({args[1]}))", + "slice": lambda a, args: checked_array_slice(a, args, result_type="std::vector"), "concat": lambda a, args: f"{a}.insert({a}.end(),{args[0]}.begin(),{args[0]}.end())", "sum": lambda a, args: f"({a}.empty()?na():std::accumulate({a}.begin(),{a}.end(),0.0))", "avg": lambda a, args: f"({a}.empty()?na():std::accumulate({a}.begin(),{a}.end(),0.0)/{a}.size())", @@ -667,6 +765,9 @@ def _checked_array_percentrank(a: str, args: list[str]) -> str: "set": ["index", "value"], "remove": ["index"], "percentrank": ["index"], + "insert": ["index", "value"], + "fill": ["value", "index_from", "index_to"], + "slice": ["index_from", "index_to"], "first": [], "last": [], "pop": [], diff --git a/pineforge_codegen/codegen/types.py b/pineforge_codegen/codegen/types.py index 02b8b0b..ed42a52 100644 --- a/pineforge_codegen/codegen/types.py +++ b/pineforge_codegen/codegen/types.py @@ -50,6 +50,7 @@ MATRIX_RETURNING_METHODS, PINE_TYPE_TO_CPP, TA_RETURNS_BOOL, + checked_array_slice, ) # Collection (array / map / matrix) methods that MUTATE the receiver in place. @@ -960,7 +961,11 @@ def _array_method_expr( if method == "copy": lower_receiver = lambda recv: f"{arr_cpp_type}({recv})" elif method == "slice": - lower_receiver = lambda recv: f"{arr_cpp_type}({recv}.begin()+(int)({args[0]}),{recv}.begin()+(int)({args[1]}))" + # Bounds-checked in the shared helper; the element type stays + # caller-supplied so the typed lane keeps its own vector type. + lower_receiver = lambda recv: checked_array_slice( + recv, args, result_type=arr_cpp_type + ) elif method == "join" and elem_cpp == "std::string": sep = args[0] if args else 'std::string(",")' lower_receiver = lambda recv: f"[&](){{ std::string r; for(size_t i=0;i<{recv}.size();i++){{ if(i>0)r+={sep}; r+={recv}[i]; }} return r; }}()" diff --git a/pineforge_codegen/support_checker.py b/pineforge_codegen/support_checker.py index 71bc835..d79a5f1 100644 --- a/pineforge_codegen/support_checker.py +++ b/pineforge_codegen/support_checker.py @@ -516,11 +516,20 @@ def __init__(self, ast: Program, filename: str = "") -> None: # then ``request.security(haTicker, ...)``, or ``reqSym = cond ? other : # syminfo.tickerid``. First binding wins (closest to declaration). self._scalar_defs: dict[str, ASTNode] = {} + # EVERY reassignment value bound to a scalar name (``name := `` + # and the compound forms). A declaration alone does not pin the value a + # ``request.security`` symbol argument actually carries: the engine is + # registered with no symbol at all, so an accepted-but-divergent rebind + # silently backtests the CHART feed. Collected in a whole-AST pre-pass + # (below) as well as during the visit, so a rebind that lexically + # FOLLOWS the request.security call cannot slip past. + self._scalar_rebinds: dict[str, list[ASTNode]] = {} # -- Public API -- def check(self) -> list[Diagnostic]: self._collect_user_definitions(self._ast) + self._collect_scalar_rebinds(self._ast) for stmt in self._ast.body: self._visit(stmt) return self._diagnostics @@ -533,6 +542,42 @@ def check_or_raise(self) -> None: # -- Setup -- + def _record_scalar_rebind(self, node: Assignment) -> None: + """Remember ``name := `` (and the compound forms) by name. + + A compound op (``+=``, ``*=``, …) records its right-hand operand, which + is never a current-symbol expression — so a string-built symbol is + rejected by the same rule that rejects a plain divergent ``:=``. + """ + target = node.target + if not isinstance(target, Identifier) or node.value is None: + return + recorded = self._scalar_rebinds.setdefault(target.name, []) + if not any(value is node.value for value in recorded): + recorded.append(node.value) + + def _collect_scalar_rebinds(self, node: ASTNode) -> None: + """Pre-pass: collect every scalar rebind anywhere in the AST. + + The visit walks top-level statements in source order, so a rebind that + FOLLOWS a ``request.security`` call would otherwise never be seen by + that call's symbol check. Collecting up front is strictly fail-closed: + it can only add rejections, never remove one. + """ + if isinstance(node, Assignment): + self._record_scalar_rebind(node) + for value in vars(node).values(): + if isinstance(value, ASTNode): + self._collect_scalar_rebinds(value) + elif isinstance(value, list): + for item in value: + if isinstance(item, ASTNode): + self._collect_scalar_rebinds(item) + elif isinstance(value, dict): + for item in value.values(): + if isinstance(item, ASTNode): + self._collect_scalar_rebinds(item) + def _collect_user_definitions(self, ast: Program) -> None: for stmt in ast.body: if isinstance(stmt, TypeDecl): @@ -1001,6 +1046,14 @@ def _visit_VarDecl(self, node: VarDecl) -> None: ) self._visit_children(node) + def _visit_Assignment(self, node: Assignment) -> None: + # ``_scalar_defs`` records only the DECLARATION, so a later ``:=`` + # rebind used to be invisible to the request.security symbol check. + # (``check`` also pre-collects these; recording here keeps the visit + # self-consistent for any caller that drives visitors directly.) + self._record_scalar_rebind(node) + self._visit_children(node) + def _visit_TupleAssign(self, node: TupleAssign) -> None: drawing_mask = self._tuple_assign_drawing_mask(node.value) if drawing_mask: @@ -1723,10 +1776,21 @@ def _is_current_symbol_expr(self, node: ASTNode, _seen: set[str] | None = None) or self._is_current_symbol_expr(node.false_val, _seen)) # Def-use: resolve a bare identifier through its declaration value so an # aliased symbol is accepted (``haTicker = ticker.heikinashi(...)`` then - # ``request.security(haTicker, ...)``). Name-cycle-guarded. - if isinstance(node, Identifier) and node.name in self._scalar_defs and node.name not in _seen: - _seen.add(node.name) - return self._is_current_symbol_expr(self._scalar_defs[node.name], _seen) + # ``request.security(haTicker, ...)``). Every ``:=`` rebind of the name + # must resolve to the chart symbol too: the declaration alone does not + # pin the value the call actually receives. Name-cycle-guarded. + if isinstance(node, Identifier) and node.name not in _seen: + definition = self._scalar_defs.get(node.name) + rebinds = self._scalar_rebinds.get(node.name) + if definition is not None or rebinds: + _seen.add(node.name) + if rebinds and not all( + self._is_current_symbol_expr(value, _seen) for value in rebinds + ): + return False + if definition is None: + return False + return self._is_current_symbol_expr(definition, _seen) return False # -- Pine timeframe-literal validation -- diff --git a/tests/test_array_bounds_insert_fill_slice.py b/tests/test_array_bounds_insert_fill_slice.py new file mode 100644 index 0000000..b9fb585 --- /dev/null +++ b/tests/test_array_bounds_insert_fill_slice.py @@ -0,0 +1,277 @@ +"""Bounds coverage for ``array.insert``, 3-argument ``array.fill``, ``array.slice``. + +The checked-access family (``get``/``set``/``remove``/``first``/``last``/``pop``/ +``shift``/``percentrank``) already converts an out-of-range Pine index into a +deterministic ``pine_runtime_error``. These three lowerings were left emitting +raw STL iterator arithmetic, so an out-of-range index produced C++ undefined +behaviour (an iterator outside ``[begin, end]``) instead of Pine's runtime +error. + +Bound semantics pinned here, from the Pine v6 reference: + +* ``array.insert`` is an *insertion* point, so ``index == size`` is legal and + appends. The reference also lists ``insert`` in the negative-indexing set + (``array.get``/``array.set``/``array.insert``/``array.remove``), so a negative + index is end-relative exactly as it is for ``get``/``set``/``remove``. +* ``array.fill``/``array.slice`` take a half-open ``[index_from, index_to)`` + range — ``index_to`` is "one greater than the last index", so ``index_to == + size`` is legal. Neither is in the reference's negative-indexing set, so a + negative endpoint is rejected (the same stance ``array.percentrank`` already + takes). +""" + +from __future__ import annotations + +import pytest + +from pineforge_codegen import transpile +from tests import _compile as compile_env +from tests.test_array_checked_access import _compile_and_run + + +PRELUDE = '//@version=6\nstrategy("T")\n' +SETUP = "a = array.new_float(3, 0.0)\n" + + +def _generate(body: str) -> str: + return transpile(f"{PRELUDE}{body}\n") + + +def _statement(cpp: str, needle: str) -> str: + return next(line for line in cpp.splitlines() if needle in line) + + +# --------------------------------------------------------------------------- +# Out-of-range indices must reach the runtime error path, not raw STL +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize( + "body", + [ + SETUP + "array.insert(a, 9, 1.0)", + SETUP + "a.insert(9, 1.0)", + SETUP + "array.insert(id=a, index=9, value=1.0)", + SETUP + "a.insert(index=9, value=1.0)", + SETUP + "array.fill(a, 1.0, 0, 9)", + SETUP + "a.fill(1.0, 0, 9)", + SETUP + "array.fill(id=a, value=1.0, index_from=0, index_to=9)", + SETUP + "b = array.slice(a, 2, 9)\nplot(array.size(b))", + SETUP + "b = a.slice(2, 9)\nplot(array.size(b))", + SETUP + "b = array.slice(id=a, index_from=2, index_to=9)\nplot(array.size(b))", + ], +) +def test_bounded_methods_route_through_runtime_error_path(body: str): + assert "pine_runtime_error" in _generate(body) + + +@pytest.mark.parametrize( + "body", + [ + SETUP + "array.insert(a, 9, 1.0)", + SETUP + "array.fill(a, 1.0, 0, 9)", + SETUP + "b = array.slice(a, 2, 9)\nplot(array.size(b))", + ], +) +def test_bounded_methods_do_not_emit_raw_iterator_arithmetic(body: str): + cpp = _generate(body) + assert "a.begin() + (int)(" not in cpp + assert "a.begin()+(int)(" not in cpp + + +# --------------------------------------------------------------------------- +# Exact bound each lowering enforces +# --------------------------------------------------------------------------- + +def test_insert_accepts_index_equal_to_size(): + """``index == size`` appends; only ``index > size`` is out of bounds.""" + stmt = _statement(_generate(SETUP + "array.insert(a, 3, 1.0)"), "__pf_array.insert") + assert "__pf_array_index<0||__pf_array_index>__pf_array_size)" in stmt + assert "__pf_array_index>=__pf_array_size" not in stmt + + +def test_insert_normalizes_negative_indices_like_get_set_remove(): + stmt = _statement(_generate(SETUP + "array.insert(a, -1, 1.0)"), "__pf_array.insert") + assert "__pf_raw_index<0?__pf_raw_index+__pf_array_size:__pf_raw_index" in stmt + + +@pytest.mark.parametrize( + ("body", "needle"), + [ + (SETUP + "array.fill(a, 1.0, 0, 3)", "std::fill"), + (SETUP + "b = array.slice(a, 0, 3)\nplot(array.size(b))", "__pf_array_index_to"), + ], +) +def test_range_methods_accept_index_to_equal_to_size(body: str, needle: str): + """``index_to`` is exclusive, so ``index_to == size`` is a legal endpoint.""" + stmt = _statement(_generate(body), needle) + assert "__pf_array_index_from<0||__pf_array_index_from>__pf_array_size_index_from)" in stmt + assert "__pf_array_index_to<0||__pf_array_index_to>__pf_array_size_index_to)" in stmt + + +@pytest.mark.parametrize( + ("body", "needle"), + [ + (SETUP + "array.fill(a, 1.0, 0, 3)", "std::fill"), + (SETUP + "b = array.slice(a, 0, 3)\nplot(array.size(b))", "__pf_array_index_to"), + ], +) +def test_range_methods_do_not_normalize_negative_endpoints(body: str, needle: str): + """``fill``/``slice`` are absent from the reference's negative-index set.""" + stmt = _statement(_generate(body), needle) + assert "int64_t __pf_array_index_from=__pf_raw_index_from;" in stmt + assert "int64_t __pf_array_index_to=__pf_raw_index_to;" in stmt + assert "__pf_raw_index_from<0?" not in stmt + assert "__pf_raw_index_to<0?" not in stmt + + +def test_range_methods_reject_inverted_ranges(): + for body, needle in ( + (SETUP + "array.fill(a, 1.0, 2, 1)", "std::fill"), + (SETUP + "b = array.slice(a, 2, 1)\nplot(array.size(b))", "__pf_array_index_to"), + ): + stmt = _statement(_generate(body), needle) + assert "__pf_array_index_from>__pf_array_index_to" in stmt + + +def test_single_argument_fill_overload_is_unchanged(): + """The 1-argument ``array.fill(id, value)`` form has no index to check.""" + stmt = _statement(_generate(SETUP + "array.fill(a, 1.0)"), "std::fill") + assert stmt.strip() == "std::fill(a.begin(), a.end(), 1.0);" + + +# --------------------------------------------------------------------------- +# The emitted C++ still parses against the public engine headers +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize( + ("label", "body"), + [ + ("insert", SETUP + "array.insert(a, 9, 1.0)"), + ("insert_at_size", SETUP + "array.insert(a, 3, 1.0)"), + ("insert_negative", SETUP + "array.insert(a, -1, 1.0)"), + ("fill_range", SETUP + "array.fill(a, 1.0, 0, 9)"), + ("fill_single", SETUP + "array.fill(a, 1.0)"), + ("slice", SETUP + "b = array.slice(a, 2, 9)\nplot(array.size(b))"), + ( + "slice_typed_method", + "var array ints = array.from(1, 2, 3)\n" + "probe() =>\n" + " array s = ints.slice(0, 2)\n" + " array.size(s)\n" + "observed = probe()", + ), + ], +) +def test_bounded_lowerings_compile(label: str, body: str): + compile_env.skip_if_no_compile_env() + compile_env.compile_cpp(_generate(body), label=f"array-bounds-{label}") + + +# --------------------------------------------------------------------------- +# Runtime probes +# --------------------------------------------------------------------------- + +_VALID_SOURCE = """//@version=6 +strategy("Array bounds valid ranges") +values = array.new_float(3, 0.0) +array.insert(values, 3, 7.0) +array.insert(values, -1, 5.0) +values_size = array.size(values) +values_at_3 = array.get(values, 3) +values_at_4 = array.get(values, 4) +filled = array.new_float(4, 0.0) +array.fill(filled, 2.0, 1, 3) +filled_sum = array.sum(filled) +array.fill(filled, 9.0, 4, 4) +filled_sum_after_empty_fill = array.sum(filled) +sliced = array.slice(filled, 1, 4) +sliced_size = array.size(sliced) +sliced_sum = array.sum(sliced) +empty_slice = array.slice(filled, 4, 4) +empty_slice_size = array.size(empty_slice) +""" + + +_ERROR_SOURCE = """//@version=6 +strategy("Array bounds errors") +selector = close +values = array.new_float(3, 0.0) +sink = 0.0 + +if selector == 1 + array.insert(values, 4, 1.0) +else if selector == 2 + array.insert(values, -4, 1.0) +else if selector == 3 + array.fill(values, 1.0, 0, 4) +else if selector == 4 + array.fill(values, 1.0, -1, 3) +else if selector == 5 + array.fill(values, 1.0, 2, 1) +else if selector == 6 + sink := array.size(array.slice(values, 0, 4)) +else if selector == 7 + sink := array.size(array.slice(values, -1, 2)) +else if selector == 8 + sink := array.size(array.slice(values, 2, 1)) +""" + + +def test_valid_boundary_indices_run_and_produce_pine_results(): + driver = r""" +#include +int main() { + GeneratedStrategy strategy; + Bar bar{1.0, 1.0, 1.0, 1.0, 1.0, 0}; + strategy.run(&bar, 1); + if (!strategy.last_error().empty()) { + std::cerr << strategy.last_error() << "\n"; + return 2; + } + std::cout << strategy.values_size << " " + << strategy.values_at_3 << " " + << strategy.values_at_4 << " " + << strategy.filled_sum << " " + << strategy.filled_sum_after_empty_fill << " " + << strategy.sliced_size << " " + << strategy.sliced_sum << " " + << strategy.empty_slice_size << "\n"; +} +""" + output = _compile_and_run(transpile(_VALID_SOURCE) + driver) + assert tuple(float(value) for value in output.split()) == ( + 5.0, # [0, 0, 0, 5, 7] after append-at-size then negative insert + 5.0, + 7.0, + 4.0, # fill(2.0) over [1, 3) of a 4-element zero array + 4.0, # empty fill over [4, 4) changes nothing + 3.0, # slice [1, 4) of a 4-element array + 4.0, + 0.0, # slice [4, 4) is empty, not out of bounds + ) + + +def test_out_of_range_indices_surface_deterministic_last_error(): + driver = r""" +#include +int main() { + for (int selector = 1; selector <= 8; ++selector) { + GeneratedStrategy strategy; + double value = static_cast(selector); + Bar bar{value, value, value, value, 1.0, selector}; + strategy.run(&bar, 1); + std::cout << selector << "\t" << strategy.last_error() << "\n"; + } +} +""" + output = _compile_and_run(transpile(_ERROR_SOURCE) + driver) + assert output.splitlines() == [ + "1\tIndex 4 is out of bounds. Array size is 3", + "2\tIndex -4 is out of bounds. Array size is 3", + "3\tIndex 4 is out of bounds. Array size is 3", + "4\tIndex -1 is out of bounds. Array size is 3", + "5\tIndex range 2..1 is invalid. Array size is 3", + "6\tIndex 4 is out of bounds. Array size is 3", + "7\tIndex -1 is out of bounds. Array size is 3", + "8\tIndex range 2..1 is invalid. Array size is 3", + ] diff --git a/tests/test_array_checked_access.py b/tests/test_array_checked_access.py index b843ca3..8152b43 100644 --- a/tests/test_array_checked_access.py +++ b/tests/test_array_checked_access.py @@ -60,7 +60,10 @@ def test_checked_get_binds_temporary_receiver_and_index_once(): assignment = next( line for line in cpp.splitlines() if line.startswith(" x =") ) - assert assignment.count("std::vector(values.begin()") == 1 + # The temporary slice receiver is built exactly once (its own receiver + # ``values`` is substituted once), and the index is evaluated once. + assert assignment.count("std::vector(__pf_array.begin()") == 1 + assert assignment.count("values") == 1 assert assignment.count("idx()") == 1 assert "pine_runtime_error" in assignment diff --git a/tests/test_codegen_audit_fixes.py b/tests/test_codegen_audit_fixes.py index 70f3c49..f003187 100644 --- a/tests/test_codegen_audit_fixes.py +++ b/tests/test_codegen_audit_fixes.py @@ -266,7 +266,10 @@ def test_array_slice_string_elements(): 'sa = array.new(0)\narray.push(sa, "a")\narray.push(sa, "b")\n' "sb = array.slice(sa, 0, 1)\nplot(close)\n" ) - assert "std::vector(sa.begin()" in cpp + # The slice keeps the receiver's ELEMENT type (std::string, not double). + # Iterators come from the bounds-checked lambda's ``__pf_array`` binding. + assert "std::vector(__pf_array.begin()" in cpp + assert "}((sa))" in cpp # --------------------------------------------------------------------------- diff --git a/tests/test_support_checker.py b/tests/test_support_checker.py index 940d5a0..2381678 100644 --- a/tests/test_support_checker.py +++ b/tests/test_support_checker.py @@ -380,6 +380,77 @@ def test_request_security_syminfo_ticker_passes(): assert _errors(src) == [] +# --------------------------------------------------------------------------- +# request.security symbol holes: register_security_eval carries no symbol, so +# an accepted non-chart symbol silently backtests the CHART feed. A divergent +# ``:=`` rebind must be rejected at the exact symbol argument. +# --------------------------------------------------------------------------- + +def _expect_error_at(src: str, needle: str, line: int, col: int) -> None: + """Assert a matching error AND pin its precise file:line:col. + + ``file`` comes from the SourceLocation the PARSER stamped on the offending + node (```` for :func:`_check`'s sources), not from the checker's own + ``filename`` argument. + """ + _expect_error(src, needle) + matching = [ + d for d in _errors(src) + if needle in d.message or (d.hint and needle in d.hint) + ] + located = [ + f"{d.location.file}:{d.location.line}:{d.location.col}" for d in matching + ] + assert f":{line}:{col}" in located, located + + +def test_request_security_reassigned_symbol_identifier_rejected(): + """``var sym = syminfo.tickerid`` … ``sym := "EXCH:OTHER"`` must reject. + + ``_scalar_defs`` only ever saw the DECLARATION, so the divergent ``:=`` + rebind used to transpile clean and run on the chart feed. + """ + src = ( + PRELUDE + + 'var sym = syminfo.tickerid\n' + + 'sym := "EXCH:OTHER"\n' + + 'a = request.security(sym, "60", close)\n' + ) + _expect_error_at(src, "current chart symbol", line=5, col=22) + + +def test_request_security_reassigned_symbol_after_call_rejected(): + """A divergent rebind LATER in the source must reject too.""" + src = ( + PRELUDE + + 'var sym = syminfo.tickerid\n' + + 'a = request.security(sym, "60", close)\n' + + 'sym := "EXCH:OTHER"\n' + ) + _expect_error(src, "current chart symbol") + + +def test_request_security_symbol_rebound_to_current_symbol_passes(): + src = ( + PRELUDE + + 'var sym = syminfo.tickerid\n' + + 'sym := syminfo.ticker\n' + + 'a = request.security(sym, "60", close)\n' + ) + assert _errors(src) == [] + + +def test_request_security_symbol_compound_rebind_rejected(): + """A string-built symbol reaches the same rule through its RHS operand.""" + src = ( + PRELUDE + + 'var sym = syminfo.tickerid\n' + + 'sym += "X"\n' + + 'a = request.security(sym, "60", close)\n' + ) + _expect_error(src, "current chart symbol") + + # --------------------------------------------------------------------------- # Unknown built-ins (silent stub avoidance) # ---------------------------------------------------------------------------