AST/interpreter: stable lexical call-site ids for builtin calls#52
Open
FellowTraveler wants to merge 1 commit into
Open
AST/interpreter: stable lexical call-site ids for builtin calls#52FellowTraveler wants to merge 1 commit into
FellowTraveler wants to merge 1 commit into
Conversation
Built-in functions that keep per-call-site state (e.g. a crossover's previous-bar operands) need a stable identity for each call site. With lazy and/or and untaken if/ternary branches, a call site may not execute on every bar, so 'Nth call this bar = Nth call site' execution-order counting misidentifies sites whenever an earlier call is skipped. - pine-ast: Expr::Call gains id: u32 (serde default, skipped when 0, so serialized ASTs and the testdata snapshots are unchanged). Program::new assigns ids in AST-walk (lexical) order starting at 1, covering every statement/expression position including function bodies and parameter defaults. - pine-interpreter: FunctionCallArgs gains call_id (0 for hand-built args), threaded from the Call node at builtin dispatch via with_call_id(). Existing constructors keep their signatures. Tests: parser test pins parent-first lexical numbering (f(g(1)) or f(2) -> 1,2,3) and re-parse stability.
ferranbt
reviewed
Jul 9, 2026
ferranbt
left a comment
ferranbt
left a comment
Owner
There was a problem hiding this comment.
I am not sure I understand what the issue is here? Could you write a Pinescript testcase?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the #49/#50/#51 series (same origin: running real Pine strategies through pinecone bar-by-bar and diffing against TradingView's strategy tester). Like those three, this PR is fully independent: it compiles and passes the full workspace suite standalone on current
main(cargo test,just fmt-check,just clippyall clean), and cherry-picks conflict-free onto/under each of the other open PRs in any order.feat(ast,interpreter): stable lexical call-site ids for builtin callsBuilt-in functions that keep per-call-site state (e.g. a crossover's previous-bar operands) need a stable identity for each call site. Counting executions ("Nth call this bar = Nth call site") misidentifies sites whenever an earlier call is skipped — which happens today with calls inside untaken
if/ternary branches, and will also happen under Pine's lazyand/or(#53, a separate stacked PR which does not depend on this one).Expr::Callgainsid: u32(serde default, skipped when 0, so serialized ASTs and the testdata snapshots are unchanged).Program::newassigns ids in AST-walk (lexical) order starting at 1, covering every statement/expression position including function bodies and parameter defaults.FunctionCallArgsgainscall_id(0 for hand-built args), threaded from theCallnode at builtin dispatch viawith_call_id(). Existing constructors keep their signatures.Hosts implementing stateful builtins (
ta.crossover,ta.crossunder, etc.) can key their per-call-site state oncall_idinstead of execution order. We use exactly this downstream: keyingta.*state bycall_idwas validated against TradingView's strategy-tester trade list over a 15-year BTCUSD backtest (~1,900 trades reproduced, including exit-reason order comments).Tests: parser test pins parent-first lexical numbering (
f(g(1)) or f(2)→ 1,2,3) and re-parse stability.