Skip to content

⚡ Bolt: Optimize list literal evaluation #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
logbie merged 1 commit into from
Feb 11, 2026
Merged

⚡ Bolt: Optimize list literal evaluation #337

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/interpreter/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5746,6 +5746,7 @@ impl Interpreter {
fn evaluate_literal_direct(
&self,
literal: &Literal,
env: &Rc<RefCell<Environment>>,
line: usize,
column: usize,
) -> Result<Option<Value>, RuntimeError> {
Expand All @@ -5761,8 +5762,20 @@ impl Interpreter {
line,
column,
)),
// List requires recursion, so fall through to boxed implementation
Literal::List(_) => Ok(None),
Literal::List(elements) => {
let mut list_values = Vec::with_capacity(elements.len());
for element in elements {
// Recursively try to evaluate elements synchronously.
// This works for nested lists as well.
if let Some(value) = self.try_evaluate_simple_expr_sync(element, env)? {
list_values.push(value);
} else {
// Element requires async evaluation, abort sync optimization for the whole list
return Ok(None);
Comment on lines +5770 to +5774

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Prevent double-evaluating list elements on async fallback

The new Literal::List fast path evaluates each element immediately and only returns None when it later encounters a non-sync expression, which causes evaluate_expression to re-run the entire list in _evaluate_expression. In mixed lists (for example [random, some_call(...)] where random is auto-called as a zero-arg native function), earlier elements are executed twice, changing observable behavior (extra RNG/time calls or other side effects) compared to the previous single evaluation path.

Useful? React with 👍 / 👎.

}
}
Comment on lines +5766 to +5776

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a later element requires async evaluation, this returns Ok(None) after already computing earlier elements, which will be recomputed again by the fallback (boxed/async) list path. Consider a two-pass approach (first pass checks whether all elements are sync-evaluable without producing Values; second pass evaluates), or refactor to preserve and reuse the already-evaluated prefix when falling back.

Copilot uses AI. Check for mistakes.
Ok(Some(Value::List(Rc::new(RefCell::new(list_values)))))
}
Comment on lines +5765 to +5778

@coderabbitai coderabbitai Bot Feb 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid double-evaluation when list sync path falls back to async.

If an element later in the list requires async evaluation, the fast-path returns Ok(None) after already evaluating earlier elements. The async fallback then re-evaluates the entire list, which can duplicate side effects (e.g., zero-arg native functions invoked via auto-call). Consider a two-phase approach (pre-scan for async-needed elements without executing), or preserve the already-evaluated prefix and continue asynchronously for the remainder instead of re-running from scratch.

🤖 Prompt for AI Agents 
In `@src/interpreter/mod.rs` around lines 5765 - 5778, The fast sync path for
Literal::List uses try_evaluate_simple_expr_sync to evaluate elements and
returns Ok(None if any element needs async), which causes already-evaluated
earlier elements to be re-run in the async path and can double-execute side
effects; modify the logic in the Literal::List handling so you first pre-scan
elements (calling a lightweight check like try_evaluate_simple_expr_sync in
"probe" mode or adding a new helper) to detect whether any element requires
async without executing side-effecting evaluations, and if a mix is detected
either (1) preserve the already-evaluated prefix (store values produced so far)
and continue evaluation asynchronously for the remaining elements, or (2)
perform a pure pre-scan pass that only determines async-necessity and then run a
single evaluation pass asynchronously; update try_evaluate_simple_expr_sync or
add a new try_probe_simple_expr_sync helper and ensure the final produced value
is wrapped as Value::List(Rc::new(RefCell::new(...))) as before.

}
}

Expand Down Expand Up @@ -5807,7 +5820,7 @@ impl Interpreter {
) -> Result<Option<Value>, RuntimeError> {
match expr {
Expression::Literal(literal, line, column) => {
self.evaluate_literal_direct(literal, *line, *column)
self.evaluate_literal_direct(literal, env, *line, *column)
}
Expression::Variable(name, line, column) => {
self.try_evaluate_variable_sync(name, env, *line, *column)
Expand Down