Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2343823002:
[parser] Fix tail calls in for in/of loops (Closed)
Description[parser] Fix tail calls in for in/of loops
According to the ES6 specification, in "for in/of" loops like:
for (var v of [1,2,3]) return f(...);
the call to f() should not be considered a tail call. This was
not working properly, i.e., the case without declarations:
var v;
for (v of [1,2,3]) return f(...);
R=adamk@chromium.org, ishell@chromium.org
BUG=
LOG=N
Committed: https://crrev.com/d5dcce33bf2140f97055c91aa2dc4e87c7a9d668
Cr-Commit-Position: refs/heads/master@{#39497}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rebase #Patch Set 3 : Better rebase #
Messages
Total messages: 33 (18 generated)
lgtm, thanks!
The CQ bit was checked by nikolaos@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm % question https://codereview.chromium.org/2343823002/diff/1/test/mjsunit/es6/tail-call.js File test/mjsunit/es6/tail-call.js (right): https://codereview.chromium.org/2343823002/diff/1/test/mjsunit/es6/tail-call.... test/mjsunit/es6/tail-call.js:298: //%DebugPrint(o); Why was this here/why is it commented?
https://codereview.chromium.org/2343823002/diff/1/test/mjsunit/es6/tail-call.js File test/mjsunit/es6/tail-call.js (right): https://codereview.chromium.org/2343823002/diff/1/test/mjsunit/es6/tail-call.... test/mjsunit/es6/tail-call.js:298: //%DebugPrint(o); On 2016/09/15 15:48:53, adamk wrote: > Why was this here/why is it commented? Igor asked me to comment this out, while at this CL. AFAIK, this just used to print more debug information when the test failed.
The CQ bit was checked by nikolaos@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/24241)
The CQ bit was checked by nikolaos@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org, adamk@chromium.org Link to the patchset: https://codereview.chromium.org/2343823002/#ps20001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...) v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)
The CQ bit was checked by nikolaos@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
This became a bit more complicated after https://codereview.chromium.org/2347633002/ The new test there was the one that failed in my first rebase. PTAL.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm, though now that I re-read this I'm fuzzy on what the ReturnExprScope has to do with block_state.
The CQ bit was checked by nikolaos@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org Link to the patchset: https://codereview.chromium.org/2343823002/#ps40001 (title: "Better rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux_chromium_gn_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_chromium_gn_rel/bu...)
The CQ bit was checked by nikolaos@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== [parser] Fix tail calls in for in/of loops According to the ES6 specification, in "for in/of" loops like: for (var v of [1,2,3]) return f(...); the call to f() should not be considered a tail call. This was not working properly, i.e., the case without declarations: var v; for (v of [1,2,3]) return f(...); R=adamk@chromium.org, ishell@chromium.org BUG= LOG=N ========== to ========== [parser] Fix tail calls in for in/of loops According to the ES6 specification, in "for in/of" loops like: for (var v of [1,2,3]) return f(...); the call to f() should not be considered a tail call. This was not working properly, i.e., the case without declarations: var v; for (v of [1,2,3]) return f(...); R=adamk@chromium.org, ishell@chromium.org BUG= LOG=N Committed: https://crrev.com/d5dcce33bf2140f97055c91aa2dc4e87c7a9d668 Cr-Commit-Position: refs/heads/master@{#39497} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/d5dcce33bf2140f97055c91aa2dc4e87c7a9d668 Cr-Commit-Position: refs/heads/master@{#39497} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
