diff --git a/pineforge_codegen/codegen/base.py b/pineforge_codegen/codegen/base.py index 3ec2245..c71120a 100644 --- a/pineforge_codegen/codegen/base.py +++ b/pineforge_codegen/codegen/base.py @@ -381,6 +381,10 @@ def __init__(self, ctx: AnalyzerContext) -> None: self._security_inline_counter = 0 self._random_call_counter = 0 self._for_counter = 0 + # Unique lambda-local names used when an array lowering references its + # receiver more than once. The binding keeps temporary-producing or + # side-effectful receivers single-evaluation (see TypeInferer). + self._array_receiver_counter = 0 # UDT / enum (needed before _collect_known_vars for input.enum) self._udt_defs: dict[str, list] = {} self._enum_defs: dict[str, list[str]] = {} diff --git a/pineforge_codegen/codegen/types.py b/pineforge_codegen/codegen/types.py index af26683..4ade0d4 100644 --- a/pineforge_codegen/codegen/types.py +++ b/pineforge_codegen/codegen/types.py @@ -329,6 +329,44 @@ def _type_spec_from_expr(self, node) -> TypeSpec | None: # Method lowering for collection types (used by visit_call paths) # ------------------------------------------------------------------ + def _array_receiver_once_expr( + self, array_expr: str, args: list[str], lower_receiver, + ) -> str: + """Lower an array method without duplicating its receiver evaluation. + + ``ARRAY_METHODS`` is intentionally a compact table of expression + templates. Many templates need the receiver more than once (for + example ``begin()`` + ``end()``). Substituting a temporary-producing + receiver directly into those slots creates distinct objects, so the + resulting iterator range is invalid. Render first with a fresh token; + when the template uses it repeatedly and the real receiver is not a + plain identifier lvalue, bind it to one lambda-local forwarding + reference and render every use through that binding. The lambda's + deduced ``auto`` return copies scalar results before a temporary + receiver dies; mutations still reach lvalue arrays through the + forwarding reference. Single-use and plain-identifier lowerings + remain byte-for-byte unchanged. + + This is receiver-only by design: Pine argument evaluation and the + separate empty-array semantics are outside this fix. + """ + counter = getattr(self, "_array_receiver_counter", 0) + occupied = "\n".join((array_expr, *args)) + while True: + receiver = f"__pf_array_receiver_{counter}" + counter += 1 + if receiver not in occupied: + break + self._array_receiver_counter = counter + + lowered = lower_receiver(receiver) + if lowered.count(receiver) <= 1 or array_expr.isidentifier(): + return lower_receiver(array_expr) + return ( + f"[&]() {{ auto&& {receiver} = ({array_expr}); " + f"return {lowered}; }}()" + ) + def _array_method_expr( self, array_expr: str, method: str, args: list[str], spec: TypeSpec | None = None, ) -> str: @@ -337,34 +375,36 @@ def _array_method_expr( arr_cpp_type = self._type_spec_to_cpp(spec) elem_cpp = self._type_spec_to_cpp(spec.element) if spec.element is not None else "double" if method == "copy": - return f"{arr_cpp_type}({array_expr})" - if method == "slice": - return f"{arr_cpp_type}({array_expr}.begin()+(int)({args[0]}),{array_expr}.begin()+(int)({args[1]}))" - if method == "join": + 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]}))" + elif method == "join" and elem_cpp == "std::string": sep = args[0] if args else 'std::string(",")' - if elem_cpp == "std::string": - return f"[&](){{ std::string r; for(size_t i=0;i<{array_expr}.size();i++){{ if(i>0)r+={sep}; r+={array_expr}[i]; }} return r; }}()" - numeric_only = { - "sum", "avg", "min", "max", "range", "stdev", "variance", "median", - "mode", "percentile_linear_interpolation", "percentile_nearest_rank", - "percentrank", "abs", "standardize", "covariance", "binary_search", - "binary_search_leftmost", "binary_search_rightmost", "sort_indices", - } - if method in numeric_only and elem_cpp not in ("double", "int"): - self._codegen_error( - None, - f"array.{method} requires a numeric array", - hint="Use numeric arrays for aggregate/statistical array functions.", - ) - if method in ARRAY_METHODS: - return ARRAY_METHODS[method](array_expr, args) - # Defensive: support_checker rejects any array.* method not in - # SUPPORTED_ARRAY (derived from ARRAY_METHODS). Reaching here means the - # checker was bypassed or the tables drifted. - raise ValueError( - f"codegen: unhandled array method '{method}' — analyzer should have " - f"rejected. Add it to ARRAY_METHODS." - ) + 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; }}()" + else: + numeric_only = { + "sum", "avg", "min", "max", "range", "stdev", "variance", "median", + "mode", "percentile_linear_interpolation", "percentile_nearest_rank", + "percentrank", "abs", "standardize", "covariance", "binary_search", + "binary_search_leftmost", "binary_search_rightmost", "sort_indices", + } + if method in numeric_only and elem_cpp not in ("double", "int"): + self._codegen_error( + None, + f"array.{method} requires a numeric array", + hint="Use numeric arrays for aggregate/statistical array functions.", + ) + if method not in ARRAY_METHODS: + # Defensive: support_checker rejects any array.* method not in + # SUPPORTED_ARRAY (derived from ARRAY_METHODS). Reaching here means the + # checker was bypassed or the tables drifted. + raise ValueError( + f"codegen: unhandled array method '{method}' — analyzer should have " + f"rejected. Add it to ARRAY_METHODS." + ) + lower_receiver = lambda recv: ARRAY_METHODS[method](recv, args) + + return self._array_receiver_once_expr(array_expr, args, lower_receiver) def _map_method_expr( self, map_expr: str, method: str, args: list[str], spec: TypeSpec | None = None, diff --git a/tests/test_codegen_validation_fixes.py b/tests/test_codegen_validation_fixes.py index cbffdea..7ab059d 100644 --- a/tests/test_codegen_validation_fixes.py +++ b/tests/test_codegen_validation_fixes.py @@ -1,6 +1,6 @@ """Regression tests for codegen bugs found by pinescript-scrapper validation. -Covers seven fix families: +Covers ten fix families: 1. drawing-handle ``na`` reset/assignment (Box{}/Line{}/... not na()), plus typed ``na`` for string/int/bool declaration init. 2. void drawing setter used as a UDF's last expression / if-branch value. @@ -17,8 +17,12 @@ step is supplied. 9. Numeric ternaries promote an ``int`` literal branch to ``double`` when the other branch is floating-point arithmetic. + 10. Array methods materialize a duplicated receiver expression once, so + nested temporary-producing calls cannot form cross-temporary iterators. """ +import re + from pineforge_codegen import transpile @@ -847,3 +851,33 @@ def test_numeric_ternary_with_int_literal_and_float_branch_declares_double(): ) assert "double pressure" in cpp assert "int pressure" not in cpp + + +# --------------------------------------------------------------------------- +# 10. duplicated array receivers are evaluated once +# --------------------------------------------------------------------------- +def test_nested_array_slice_aggregates_materialize_one_receiver(): + cpp = _cpp( + "a = array.from(1.0, 3.0, 2.0)\n" + "mx = array.max(array.slice(a, 0, 2))\n" + "mn = array.min(array.slice(a, 1, 3))\n" + "plot(mx + mn)" + ) + + mx_line = next(line for line in cpp.splitlines() if line.strip().startswith("mx = ")) + mn_line = next(line for line in cpp.splitlines() if line.strip().startswith("mn = ")) + + # Before the fix, each constructor appeared twice: max/min_element took + # begin() from one temporary vector and end() from another (undefined + # behaviour). Each source slice must now produce one vector constructor, + # and each iterator pair must use the same named receiver binding. + assert mx_line.count("std::vector(") == 1 + assert mn_line.count("std::vector(") == 1 + assert re.search( + r"std::max_element\((__pf_array_receiver_\d+)\.begin\(\),\1\.end\(\)\)", + mx_line, + ) + assert re.search( + r"std::min_element\((__pf_array_receiver_\d+)\.begin\(\),\1\.end\(\)\)", + mn_line, + ) diff --git a/tests/test_compile_smoke.py b/tests/test_compile_smoke.py index c69cffe..6ff6c79 100644 --- a/tests/test_compile_smoke.py +++ b/tests/test_compile_smoke.py @@ -294,6 +294,15 @@ def test_arrays_compile(): """) +def test_nested_array_slice_aggregates_compile(): + _check("nested_array_slice_aggregates", """ +a = array.from(1.0, 3.0, 2.0) +mx = array.max(array.slice(a, 0, 2)) +mn = array.min(array.slice(a, 1, 3)) +plot(mx + mn) +""") + + def test_descending_for_by_array_remove_compiles(): _check("descending_for_by_array_remove", """ var levels = array.new()