await-using tests: add microtask-boundary ordering assertions#298
Merged
await-using tests: add microtask-boundary ordering assertions#298
Conversation
…ined AwaitValue now drains the microtask queue on all exit paths (non-promise values, non-thenable objects, and already-settled promises), matching the ES2026 §27.7.5.3 semantics where every await introduces a microtask boundary. This ensures DisposeTrackedResourcesAsync's AwaitValue call for skipped null/undefined disposables actually yields, preventing a silent regression where the block-exit await point is dropped. The two skipped-resource tests now schedule a microtask before the block and assert that it runs during disposal (before post-block code), verifying the async boundary is present even when no dispose method is called. Closes #294 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDrains the microtask queue from additional await/ disposal code paths and updates two tests to assert microtask-ordering at block exit for skipped Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
the
spec compliance
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@units/Goccia.Evaluator.pas`:
- Around line 1064-1078: The local AwaitValue function inside Array.fromAsync
currently returns non-promise values directly; update its non-promise branch to
call TGocciaMicrotaskQueue.Instance.DrainQueue (guarded by Assigned) before
returning so it matches the evaluator's AwaitValue behavior per ES2026
§27.7.5.3; locate the AwaitValue implementation in Array.fromAsync and insert
the same DrainQueue call used in the evaluator's non-promise branches
immediately before Result := AValue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d94ae7c9-d34f-440e-82a1-091b5626a911
📒 Files selected for processing (2)
tests/language/explicit-resource-management/await-using.jsunits/Goccia.Evaluator.pas
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
Contributor
Benchmark Results364 benchmarks Interpreted: 🟢 36 improved · 🔴 190 regressed · 138 unchanged · avg -1.2% arraybuffer.js — Interp: 🟢 1, 🔴 7, 6 unch. · avg -0.7% · Bytecode: 🟢 9, 🔴 2, 3 unch. · avg +6.4%
arrays.js — Interp: 🔴 16, 3 unch. · avg -1.7% · Bytecode: 🟢 18, 1 unch. · avg +9.5%
async-await.js — Interp: 🔴 5, 1 unch. · avg -1.5% · Bytecode: 🟢 6 · avg +7.6%
base64.js — Interp: 🟢 1, 🔴 3, 6 unch. · avg -0.8% · Bytecode: 🟢 9, 🔴 1 · avg +7.6%
classes.js — Interp: 🔴 25, 6 unch. · avg -1.7% · Bytecode: 🟢 7, 🔴 5, 19 unch. · avg -2.5%
closures.js — Interp: 🔴 9, 2 unch. · avg -2.3% · Bytecode: 🟢 4, 🔴 4, 3 unch. · avg -1.0%
collections.js — Interp: 🔴 9, 3 unch. · avg -2.6% · Bytecode: 🟢 10, 🔴 2 · avg +13.2%
destructuring.js — Interp: 🔴 20, 2 unch. · avg -3.4% · Bytecode: 🟢 4, 🔴 15, 3 unch. · avg -2.7%
fibonacci.js — Interp: 🔴 6, 2 unch. · avg -1.8% · Bytecode: 🟢 6, 2 unch. · avg +4.3%
float16array.js — Interp: 🟢 10, 🔴 9, 13 unch. · avg -0.0% · Bytecode: 🟢 8, 🔴 16, 8 unch. · avg -4.6%
for-of.js — Interp: 🔴 3, 4 unch. · avg -1.8% · Bytecode: 🟢 1, 🔴 2, 4 unch. · avg -2.7%
helpers/bench-module.js — Interp: 0 · Bytecode: 0
iterators.js — Interp: 🟢 2, 🔴 21, 19 unch. · avg -1.5% · Bytecode: 🟢 6, 🔴 23, 13 unch. · avg -4.9%
json.js — Interp: 🟢 4, 🔴 8, 8 unch. · avg -0.9% · Bytecode: 🟢 5, 🔴 4, 11 unch. · avg +1.2%
jsx.jsx — Interp: 🟢 1, 🔴 7, 13 unch. · avg -0.4% · Bytecode: 🔴 20, 1 unch. · avg -5.8%
modules.js — Interp: 🔴 8, 1 unch. · avg -1.9% · Bytecode: 🟢 7, 2 unch. · avg +7.7%
numbers.js — Interp: 🟢 2, 9 unch. · avg +1.3% · Bytecode: 🟢 5, 🔴 2, 4 unch. · avg -1.2%
objects.js — Interp: 🟢 1, 🔴 4, 2 unch. · avg -0.3% · Bytecode: 🟢 1, 🔴 5, 1 unch. · avg -3.8%
promises.js — Interp: 🟢 2, 🔴 1, 9 unch. · avg +0.6% · Bytecode: 🔴 5, 7 unch. · avg -1.9%
regexp.js — Interp: 🔴 9, 2 unch. · avg -2.6% · Bytecode: 🟢 2, 🔴 6, 3 unch. · avg -1.4%
strings.js — Interp: 🟢 6, 🔴 6, 7 unch. · avg -0.5% · Bytecode: 🟢 8, 🔴 3, 8 unch. · avg +1.8%
typed-arrays.js — Interp: 🟢 5, 🔴 6, 11 unch. · avg -0.5% · Bytecode: 🟢 8, 🔴 10, 4 unch. · avg +0.1%
uint8array-encoding.js — Interp: 🟢 1, 🔴 8, 9 unch. · avg -1.3% · Bytecode: 🟢 4, 🔴 10, 4 unch. · avg -2.6%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
The local AwaitValue in Array.fromAsync returned non-promise values directly and only drained for pending promises. Now drains the microtask queue on all paths (non-promise values and already-settled promises) to match the evaluator's AwaitValue per ES2026 §27.7.5.3. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@units/Goccia.Builtins.GlobalArray.pas`:
- Around line 367-373: Array.fromAsync's local AwaitValue currently only handles
TGocciaPromiseValue and returns other values directly, which misses thenable
assimilation; update the AwaitValue inside ArrayFromAsync to mirror the
evaluator's logic: after the existing TGocciaPromiseValue check add an else if
AValue is TGocciaObjectValue branch that calls GetProperty(PROP_THEN) on the
object, checks the property is callable and not undefined, and if so constructs
a new Promise that invokes the thenable with resolve/reject handlers (draining
TGocciaMicrotaskQueue.Instance after scheduling), waits for the promise to
settle and returns the resolved value or raises on rejection; ensure you reuse
the same property name PROP_THEN and behavior for microtask draining and error
propagation used elsewhere so thenables are properly assimilated rather than
returned raw.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7b0c8ee-1063-4594-95d9-ecbdb4397f3f
📒 Files selected for processing (1)
units/Goccia.Builtins.GlobalArray.pas
Remove the duplicated local AwaitValue from ArrayFromAsync and delegate to Goccia.Evaluator.AwaitValue instead. This picks up thenable assimilation (objects with callable .then) and all microtask-boundary semantics from the canonical implementation, eliminating the risk of the two copies drifting apart. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 24, 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
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
AwaitValueto drain the microtask queue on all exit paths (non-promise values, non-thenable objects, already-settled promises), matching ES2026 §27.7.5.3 semantics where everyawaitintroduces a microtask boundaryawait using nullandawait using undefinedtests to assert microtask-boundary ordering — a scheduled microtask runs during block-exit disposal, proving the async yield point is present even when no dispose method is calledTesting
./build.pas clean testrunner && ./build/TestRunner tests --no-progress— 5277 passed (same 7 pre-existing ASI failures)./build/TestRunner tests --no-progress --mode=bytecode— 5318 passed (same 7 ASI failures)./build/TestRunner tests/language/explicit-resource-management/await-using.js --no-progress— 6/6 passed./build/TestRunner tests/language/explicit-resource-management/await-using.js --no-progress --mode=bytecode— 6/6 passed./format.pas --check— all formatted correctlyCloses #294
🤖 Generated with Claude Code