Repository navigation
Let a loop counter be read in its own end and step; refuse names read outside their scope, as TradingView does - #312
Conversation
TradingView accepts a for loop whose end or step expression reads its own counter, for example for i = 0 to (i == 0 ? 1 : 0) and for i = 0 to 1 by (i == 0 ? 1 : 2). We accepted such scripts and emitted C++ that g++ rejected ('i' was not declared in this scope): the counter was declared after the code that evaluates the end and step expressions.
Measured on TradingView (entry quantity of every trade; each loop accumulates the counter, so the quantity shows how the bounds are read):
- The end expression is read again after every step, with the new counter (for i = 0 to (i == 0 ? 1 : 0) gives 1; for i = 0 to (i == 0 ? 3 : 1) gives 3; an end bound that reads a variable the body changes gives 2, where one read before the loop would give 4). Inside request.security the end is also read again (3).
- The direction is fixed from the start against the first end read, taken with the counter at 0; then the counter is set to the start value and the end is read again at that counter before the first test: for i = 2 to (i == 2 ? 3 : 1) and for i = 2 to (i == 2 ? 1 : 3) both run zero iterations.
- A counter-dependent step is read once, with the counter at 0, whatever the start is (for i = 0 to 4 by (i == 0 ? 1 : 2) gives 15, where reading it again each time would give 7; for i = 2 to 6 by (i == 2 ? 1 : 2) also gives 15).
- The counter wins over a same-named outer variable, a parameter of the enclosing function and a global declared later (quantities 3, 15, 15 and 3 on the four shadowing probes).
- A step that is not positive (a negative or zero literal, or a variable that is negative) does not run the loop: TradingView stops the script with "'step' in loop must be greater than zero" on the first bar, so no trade is entered.
- A loop whose start is a float runs with a float counter and its float step as written (for x = 0.0 to 1.0 by 0.25 gives 5 iterations and a counter total of 2.5); a float end bound with an int counter and an int step runs as an int loop.
- A float step is also accepted when the start's type is not visible at the loop: a user function parameter called with 0.0 counts 5 iterations, a start declared as a float inside an enclosing loop's body counts 7 in total, and bounds taken from array.min and array.max of a float array count 3: in all three the counter is a float and the step is kept.
The loop lowering (codegen/visit_stmt.py, and the request.security helper in codegen/security.py) binds the counter before it evaluates the end and step expressions, so the counter shadows any outer binding of its name there; it refreshes the end bound after every step, re-reads a counter-dependent end at the start counter before the first test, and reads a counter-dependent step once. A non-positive v6 step stops the run with the message above instead of being converted into a positive step.
The counter's type is decided by one query used only for loop counters (pineforge_codegen/loop_types.py, called by the analyzer and by the emitter; the shared type-inference function is unchanged from main). The counter is a float when the start has positive float evidence (a float literal, arithmetic with one, a float-typed declaration, a tuple element or a call whose declared return is float, an array minimum, maximum or average of a float array), or when the start's type is unknown and the step has positive float evidence; positive int evidence for the start (for example array.size(a) - 1) or no float evidence at all keeps main's int counter and C++. A loop whose counter does not read itself in its bounds and has an int counter keeps main's C++ except for the added step check: tests/test_for_counter_headers.py and tests/test_for_counter_type_sources.py compare existing loop forms and the string, bool, tuple and chained-declaration shapes with C++ hashes captured from a clean main in a separate process, and tests/test_e2e_for_counter_headers.py runs the loops on the engine and asserts TradingView's quantities and the no-trade stops.
Known gaps, not changed here:
- TradingView refuses a float step against an int counter (error CE10106); we accept the shapes whose start type is unknown, and a step that narrows to zero or below for an int counter whose start is positively int stops the run.
- A loop whose end bound is exactly its own counter plus a constant (for i = 0 to i) never ends here; TradingView stops such a loop with a loop-time error when its result is used and drops it when it is not. Main failed to build these for a counter-dependent end; for an end that reads an outer variable of the same name main compiled and read the outer variable. A follow-up change closes this gap.
- A float counter passed to an untyped helper function is truncated to an int by the analyzer (TradingView passes the fractional value), array.from(x) with a float counter builds an int array, and a float step accumulates one iteration short where the last step lands on the end value by rounding (TradingView counts 0.0 to 0.3 by 0.1 as four iterations): measured, fixed by follow-up changes.
- Loops in request.security whose start calls a user function are refused (an existing limit of that route).
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… does (PF-E2075) TradingView refuses a read of a name that has no visible declaration at that point: a function or method that reads a local declared later in its body, a loop counter read after its loop has ended, and a block-local read after its block (error CE10272, undeclared identifier). Main accepted the first two forms. One scope walk (analyzer/value_bindings.py, run before request lowering) now resolves every authored value read through scopes that carry declaration positions. Every block and callable owns its declarations; an initializer precedes its own binding; a counted-loop binder follows its start expression but precedes its end and step. A read with no visible declaration is refused at the identifier: PF-E2075 for a declaration that is not yet available (a later local in a callable), PF-E2054 for a block binding or loop binder out of scope (existing), PF-E2053 for a later direct global in a callable (existing), PF-E2020 for a write with no declaration (existing). The walk also exposes ValueBindings.binding_of(node), a read-only lookup of the binding it recorded on an identifier (kind and declaring callable); it performs no resolution. Forms TradingView accepts stay accepted: a name that an enclosing declaration or a built-in supplies; a loop counter named like a built-in, which is restored after the loop; a qualifier with no visible value binding falls back to its namespace or type handler, including after the scope of a block-local with the same name has ended (an untyped block-local named map followed by a namespace map.put reads, writes and sizes the right map: quantity 2.43 on TradingView and on the engine, with a twin whose local has a neutral name). Measured against TradingView, the tests pin: the refused forms above with TradingView's position, and the accepted forms. Four disagreements with TradingView are known gaps and are not changed here: TradingView accepts a namespace call after an expired block-local of the same name (str.tostring, color.new, math.abs) while code generation still refuses it later, with a misleading message that main already gave; and PineForge accepts three forms that TradingView refuses: a default that names an earlier global (CE10132), a call before the function's definition (CE10271), and a local that shadows a built-in already used (CE10190). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; formal verification proved 0 of 12 changed function(s) (1 sampled, not proven, 11 not verified).
Formal verification. PR-changed functions: 1/12 verified (0 proven, 1 may-equivalent, 0 distinguished) · 11 not verified (1 vacuous, 10 unsupported).
Not verified on this run: \_generate (vacuous: never exercised), \_\_init\_\_ (unsupported), \_visit\_ForStmt (unsupported), \_visit\_Identifier (unsupported), analyze (unsupported), \_collect\_security\_ta\_binding\_stacks (unsupported), \_collect\_security\_ta\_hist\_indices (unsupported), \_emit\_security\_linear\_helper\_call\_scoped (unsupported), \_security\_check\_loop\_body (unsupported), \_emit\_counted\_loop\_body (unsupported), and 1 more.
Graphify review — findings
Adds a ValueBindings pass, the new fourth step of the six-pass transpile() pipeline, that resolves each authored value use through scopes with declaration positions. A qualifier with no visible binding falls back to its namespace/type handler, and unavailable, inaccessible or missing-target uses get PF-E2075, PF-E2054 and PF-E2020. Rebuilds counted for lowering so the end and step can read the loop's own counter, including over an outer same-named variable and in request.security helpers. The v6 step is read once at counter 0, a float counter is chosen only on positive float evidence, and a non-positive step now stops the run instead of being replaced with 1. Rewrites the compatibility rule in AGENTS.md: a lowering that compiles today but gives a different value from TradingView is fixed, never kept behind a warning.
Worth a look
- Public compile pipeline resolves ValueBindings twice —
pineforge_codegen/__init__.py:88· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 4334 functions depend on the 1570 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
Analyzer— 81 callers, 14 callees - worse:
_generate()— 5 callers, 23 callees - worse:
resolve()— 23 callers, 3 callees - worse:
emit_stmt()— 2 callers, 18 callees - new:
_CounterQuery— 1 callers, 18 callees - new:
LoopTypeSources— 4 callers, 4 callees - new:
_analyze()— 1 callers, 6 callees - new:
test_step_evidence_only_promotes_an_unknown_start()— 0 callers, 8 callees - …and 3 more — each is listed as a finding
Verification — 4334 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 4333 function(s) in the blast radius were not formally verified this run
Cross-repo impact
Cross-repo impact
No cross-repo impact detected among the indexed peer repo(s).
1 peer repo(s) in this group are not yet indexed — cross-repo impact may be incomplete.
Cross-repo linkage is derived (SCIP-moniker matches are sound; name/contract matches are heuristic) and ADVISORY — it never affects this check's conclusion.
Docs that may be stale (advisory)
README.md§ Handle unsupported features (lines 368-404): references changed symbolsrunsREADME.md§ How it works (lines 507-533): references changed symbols__init__tests/fixtures/array_history_tv/README.md§ CG-ARRAY-HIST TradingView evidence (lines 1-106): references changed symbolscalltests/fixtures/drawing_lifetime/README.md§ Drawing lifetime tapes (lanes W8B-SIGSTATE-2 and CG-W9-MISC) (lines 1-85): references changed symbolsrunstests/fixtures/krunerr_tv/README.md§ K-RUNERR TradingView trade evidence (lines 1-71): references changed symbolsrunstests/fixtures/security_sources_tv/README.md§ request.security payload source series: TradingView trade evidence (lines 1-45): references changed symbolsrunstests/fixtures/silent2_tv/README.md§ CG-SILENT-2 TradingView trade evidence (lines 1-132): references changed symbolscalltests/fixtures/udt_history_tv/README.md§ CG-UDT-HIST TradingView evidence (lines 1-143): references changed symbolscalltests/fixtures/w2_trio_tv/README.md§ A function's last statement (lines 51-91): references changed symbolsrunstests/fixtures/xsym_lib_tv/README.md§ XSYM-C TradingView trade evidence (lines 1-64): references changed symbolscall,declaration
Formal verification
Could not verify: Could not verify \_generate.
The verifier did not have enough to check \_generate, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 241 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly CompileError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_\_init\_\_.
The verifier did not have enough to check \_\_init\_\_, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_ForStmt.
The verifier did not have enough to check \_visit\_ForStmt, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_Identifier.
The verifier did not have enough to check \_visit\_Identifier, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify analyze.
The verifier did not have enough to check analyze, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
No difference found (not proven): No behavior difference found in \_collect\_security\_ohlc\_hist\_fields (not a proof).
The verifier ran both versions of \_collect\_security\_ohlc\_hist\_fields on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: the synth_receiver tier. A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_collect\_security\_ta\_binding\_stacks.
The verifier did not have enough to check \_collect\_security\_ta\_binding\_stacks, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_collect\_security\_ta\_hist\_indices.
The verifier did not have enough to check \_collect\_security\_ta\_hist\_indices, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_emit\_security\_linear\_helper\_call\_scoped.
The verifier did not have enough to check \_emit\_security\_linear\_helper\_call\_scoped, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_security\_check\_loop\_body.
The verifier did not have enough to check \_security\_check\_loop\_body, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_emit\_counted\_loop\_body.
The verifier did not have enough to check \_emit\_counted\_loop\_body, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
Could not verify: Could not verify \_visit\_for.
The verifier did not have enough to check \_visit\_for, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)
· 2 grounded finding(s) anchored inline below; 9 more finding(s) on lines outside this diff (see the check run).
| return None | ||
|
|
||
|
|
||
| class LoopTypeSources: |
There was a problem hiding this comment.
LoopTypeSources
high coupling complexity (Ca·Ce = 16).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ).append(_hint_spec(hint)) | ||
|
|
||
|
|
||
| class _CounterQuery: |
There was a problem hiding this comment.
_CounterQuery
fans out to 18 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
What changes
Counted loops (compile agreement). TradingView accepts a
forloop whose end or step expression reads its own counter. We accepted such scripts and emitted C++ that g++ rejected ('i' was not declared in this scope). The loop lowering now binds the counter before it evaluates the end and step, refreshes the end after every step, re-reads a counter-dependent end at the start counter before the first test, and reads a counter-dependent step once; a non-positive v6 step stops the run with TradingView's text. The counter wins over a same-named outer variable. The counter's type comes from one dedicated query (loop_types.py) used only for loop counters: a float counter for a start that is positively float, or an unknown start with a positively float step (three TradingView-measured loops); an int counter otherwise, so every script that did not read its counter in its bounds keeps its C++ except for the added step check. The shared type-inference function is unchanged. The values are checked against TradingView's own exports for 22 loop probes (entry quantities), including the shadowing, direction and first-end-read rows.Names outside their scope (PF-E2075, PF-E2054). A Pine v6 function or method that reads a later local without an earlier visible binding is refused during analysis at the read; a loop counter or
for ... inbinder read after its loop without another visible binding is refused at translate time. Both reproduce TradingView's own refusals (23 refused and 33 accepted probe rows are pinned). A namespace call after an expired block local of the same name is accepted by the analysis, as TradingView does; code generation still rejects some such calls (a known gap).Also in this change
array.from(x)with a float counter truncates.Verification
🤖 Generated with Claude Code