fix(compiler): pop stale bool from stack in emitLoopBackwards#954
Merged
antonmedv merged 1 commit intoexpr-lang:masterfrom Apr 12, 2026
Merged
Conversation
emitLoopBackwards uses OpMoreOrEqual + OpJumpIfFalse to check the loop
condition, but never pops the comparison result from the stack. This
leaves a stale bool that corrupts the value stack for the parent context.
When findLast or findLastIndex is used inside a map literal like
{"r": findLast([1, 2, 3], # > 3)}, the OpMap opcode tries to pop a
string key but finds the leftover bool instead, causing:
interface conversion: interface {} is bool, not string
The forward-iterating emitLoop avoids this by using OpJumpIfEnd, which
checks scope.Index directly without touching the stack.
Fix: add OpPop after OpJumpIfFalse (continue path) and after patchJump
(exit path), matching the convention used by emitCond and every other
OpJumpIfFalse callsite in the compiler.
Fixes expr-lang#950
There was a problem hiding this comment.
Pull request overview
Fixes a VM stack corruption bug triggered by reverse-iterating builtins (findLast, findLastIndex) when used inside map literal expressions by ensuring the loop-condition boolean is popped in emitLoopBackwards.
Changes:
- Pop the
OpMoreOrEqualcondition result on both the continue and exit paths inemitLoopBackwards. - Add regression tests for
findLast/findLastIndexused within map literals (including multiple reverse builtins in the same map and a no-match case).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| expr_test.go | Adds regression coverage for reverse-iterating builtins inside map literals to prevent the reported runtime crash. |
| compiler/compiler.go | Fixes stack hygiene in emitLoopBackwards by popping the loop-condition boolean on both control-flow paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
antonmedv
approved these changes
Apr 12, 2026
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
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.
Fixes #950
findLastandfindLastIndexcrash withinterface conversion: interface {} is bool, not stringwhen used inside map literal expressions.Failing scenario:
The forward-iterating equivalents (
find,findIndex) work fine in the same position.Root cause:
emitLoopBackwardsusesOpMoreOrEqual+OpJumpIfFalseto check the loop condition but never pops the comparison result. The staleboolcorrupts the value stack — whenOpMaptries to pop a string key, it gets the leftover bool instead.The forward
emitLoopavoids this by usingOpJumpIfEnd, which checksscope.Indexdirectly without touching the stack.Fix:
Add
OpPopafterOpJumpIfFalse(continue path) and afterpatchJump(exit path). This matches the convention used byemitCondand every otherOpJumpIfFalsecallsite in the compiler.After fix:
Validation: