pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/WebKit/WebKit/pull/60818

/assets/global-52276e82f63bb403.css" /> [JSC] for-using statement should not dispose on continue by sosukesuzuki · Pull Request #60818 · WebKit/WebKit · GitHub
Skip to content

[JSC] for-using statement should not dispose on continue#60818

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
sosukesuzuki:eng/for-using-continue-dispose-once
Mar 20, 2026
Merged

[JSC] for-using statement should not dispose on continue#60818
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
sosukesuzuki:eng/for-using-continue-dispose-once

Conversation

@sosukesuzuki
Copy link
Contributor

@sosukesuzuki sosukesuzuki commented Mar 17, 2026

6c959cf

[JSC] for-using statement should not dispose on continue
https://bugs.webkit.org/show_bug.cgi?id=310144

Reviewed by Yusuke Suzuki.

ForNode created its LabelScope before emitBodyWithUsingIfNeeded pushes
the finally control-flow scope, so the recorded scope depth predates the
finally. ContinueNode compared against that pre-finally depth, concluded
that a finally sits between it and the target, and routed the jump
through the finally. This disposed the using resource on every continue
in addition to the normal end-of-loop disposal; per ForLoopEvaluation [1]
step 13, DisposeResources runs once after ForBodyEvaluation returns.

Move newLabelScope into the emitBodyWithUsingIfNeeded lambda so it runs
after the finally scope is pushed. The recorded depth then matches the
depth seen by ContinueNode, so continue jumps directly. Break also jumps
directly: breakTarget is now emitted at the end of the lambda, and since
emitUsingBodyScope falls through from the try body into the finally on
normal completion, break still disposes. Labeled break still routes
through the finally because LabelNode's NamedLabel scope sits outside the
try. This is the same arrangement emitGenericEnumeration already uses.

The loopEnd label and conditionFalseTarget branching are no longer
needed: breakTarget serves both the non-using and using cases now that
it lives inside the lambda.

[1]: https://tc39.es/proposal-explicit-resource-management/#sec-runtime-semantics-forloopevaluation

* JSTests/stress/for-using-initializer-continue-dispose-once.js: Added.
(shouldBe):
(async testAwaitUsing):
* Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:
(JSC::ForNode::emitBytecode):

Canonical link: https://commits.webkit.org/309590@main

6c68a61

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-debug-arm64 ✅ 🛠 ios-safer-cpp ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@sosukesuzuki sosukesuzuki self-assigned this Mar 17, 2026
@sosukesuzuki sosukesuzuki added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Mar 17, 2026
@sosukesuzuki sosukesuzuki marked this pull request as ready for review March 18, 2026 00:17
@sosukesuzuki sosukesuzuki requested a review from a team as a code owner March 18, 2026 00:17
generator.emitBodyWithUsingIfNeeded(usingDeclarationCount(), hasAwaitUsingDeclaration(),
scopedLambda<void(BytecodeGenerator&)>([&](BytecodeGenerator& generator) {
if (usingDeclarationCount())
scope->setContinueScopeDepth(generator.labelScopeDepth());
Copy link
Member

Choose a reason for hiding this comment

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

continueScopeDepth is set only when usingDeclarationCount is true, but isn't continueScopeDepth read regardless?

Copy link
Member

@Constellation Constellation Mar 18, 2026

Choose a reason for hiding this comment

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

I'm a bit concerning that we need to add a special field additionally. Isn't it possible to have a new LabelScope instead for example? Is this "specially adjusting continue target" specified in the spec?

Copy link
Member

@Constellation Constellation left a comment

Choose a reason for hiding this comment

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

Commented

@sosukesuzuki sosukesuzuki force-pushed the eng/for-using-continue-dispose-once branch from 1dc4fa0 to 6c68a61 Compare March 19, 2026 01:38
@sosukesuzuki sosukesuzuki added the merge-queue Applied to send a pull request to merge-queue label Mar 20, 2026
https://bugs.webkit.org/show_bug.cgi?id=310144

Reviewed by Yusuke Suzuki.

ForNode created its LabelScope before emitBodyWithUsingIfNeeded pushes
the finally control-flow scope, so the recorded scope depth predates the
finally. ContinueNode compared against that pre-finally depth, concluded
that a finally sits between it and the target, and routed the jump
through the finally. This disposed the using resource on every continue
in addition to the normal end-of-loop disposal; per ForLoopEvaluation [1]
step 13, DisposeResources runs once after ForBodyEvaluation returns.

Move newLabelScope into the emitBodyWithUsingIfNeeded lambda so it runs
after the finally scope is pushed. The recorded depth then matches the
depth seen by ContinueNode, so continue jumps directly. Break also jumps
directly: breakTarget is now emitted at the end of the lambda, and since
emitUsingBodyScope falls through from the try body into the finally on
normal completion, break still disposes. Labeled break still routes
through the finally because LabelNode's NamedLabel scope sits outside the
try. This is the same arrangement emitGenericEnumeration already uses.

The loopEnd label and conditionFalseTarget branching are no longer
needed: breakTarget serves both the non-using and using cases now that
it lives inside the lambda.

[1]: https://tc39.es/proposal-explicit-resource-management/#sec-runtime-semantics-forloopevaluation

* JSTests/stress/for-using-initializer-continue-dispose-once.js: Added.
(shouldBe):
(async testAwaitUsing):
* Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:
(JSC::ForNode::emitBytecode):

Canonical link: https://commits.webkit.org/309590@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/for-using-continue-dispose-once branch from 6c68a61 to 6c959cf Compare March 20, 2026 01:34
@webkit-commit-queue
Copy link
Collaborator

Committed 309590@main (6c959cf): https://commits.webkit.org/309590@main

Reviewed commits have been landed. Closing PR #60818 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 6c959cf into WebKit:main Mar 20, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.





Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy