Merged
Conversation
Tests cover all code paths that use nested mainLoop calls (callR/Call) inside the bytecode loop, where yields from Go functions are not properly propagated: - OP_TFORLOOP: generic for-loop iterators (7 tests) - __index/__newindex metamethods (3 tests) - __add/__sub/__mul arithmetic metamethods (3 tests) - __concat metamethod (1 test) - __unm/__len metamethods (2 tests) - __eq/__lt/__le comparison metamethods (3 tests) - Mixed boundaries and nested scenarios (3 tests) All 22 tests fail, confirming the VM limitation. The bug manifests as either nil pointer dereference (when coroutine state is corrupted) or double-execution of the iterator (callR returns early on yield but the calling opcode continues processing with stale register state).
System yields (Go functions returning -1) now propagate correctly through all VM call paths: callR, Call, getField, setField, objectArith, stringConcat, equals, lessThan, and objectRational. Each opcode site that calls into user code checks yieldState after the call returns. On yield, it saves a continuation type (yieldCont) and target register (yieldContRA), then exits mainLoop. On resume, handleYieldContinuation finishes the opcode's post-call work (store result, update loop control, adjust Pc). Yield state merges the old yielded bool and yieldKind into a single yieldState field (0=none, 1=system, 2=user). coYield returns -2 to distinguish user yields from system yields without pre-setting state before switchToParentThread. coResumePropagate handles system yield propagation through coroutine boundaries, installing a continuation so the next resume re-enters the inner thread.
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
1 participant
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
callR,Call,getField/setField,objectArith,stringConcat,equals/lessThan/objectRational, andOP_TFORLOOPnow detect yields and save continuation state for proper resumeyieldStateafter return. On yield, saves continuation type + target register, exitsmainLoop. On resume,handleYieldContinuationfinishes the opcode's post-call logic (store result, update loop control, adjust Pc)yielded(bool) +yieldKind(uint8) into singleyieldStatefield.coYieldreturns-2to distinguish user yields from system yields without conflating thread-switch statecoResumePropagatehandles system yield propagation throughcoroutine.resumeboundaries, installing a continuation so the next resume re-enters the inner threadSupported yield-across patterns
Yields from Go functions (return -1) now work inside:
for...initerators (TFORCALL/TFORLOOP)__index,__newindex,__call,__add/__sub/etc.,__unm,__len,__concat,__eq/__lt/__le)pcall/xpcall(with proper error frame preservation)coroutine.resumechains (system yield propagates to host)coroutine.wrapused as iteratorTest plan