Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Bug: for loop becomes endless loop in 0.11.15 #9113

Closed
freiit opened this issue Jan 29, 2015 · 6 comments
Closed

Bug: for loop becomes endless loop in 0.11.15 #9113

freiit opened this issue Jan 29, 2015 · 6 comments

Comments

@freiit
Copy link

freiit commented Jan 29, 2015

The following one-liner runs in an endless loop on 0.11.15.
node --harmony --use-strict -e 'for (let i = 0; i < 3; ++i) { if (i == 1) { continue; } }'

Replace 'let' with 'var' to avoid the bug.

Bug does not show up in 0.11.14.

Tested with a clean docker image, created like this:
FROM node:0.11.15
CMD node --harmony --use-strict -e 'for (let i = 0; i < 3; ++i) { if (i == 1) { continue; } }'

@cjihrig
Copy link

cjihrig commented Jan 29, 2015

This is almost certainly related to the v8 3.28.73 update in 0.11.15.

@trevnorris
Copy link

I bisected as far back as 3.28.10 and the issue still occurred.

@freiit
Copy link
Author

freiit commented Feb 23, 2015

Can reproduce that on 0.12.

@Y--
Copy link

Y-- commented Feb 23, 2015

I do reproduce the bug on 0.12.0.
And it works properly with iojs 1.3.0.

@misterdjules
Copy link

Adding to milestone 0.12.3 as it seems it would be fixed by #14411.

misterdjules pushed a commit to misterdjules/node that referenced this issue Apr 28, 2015
Backport b17eaaa5755e625493c5fe537f42b58838923c52 from upstream v8.
Also add a regression test to make sure subsequent V8 upgrades include
this floating patch if needed.

Original commit message:
  Fix desugaring of let bindings in for loops to handle continue properly

  This requires putting the original loop's body inside an inner for loop (with
  the same labels as the original loop) and re-binding the temp variables in its
  "next" expression. A second flag is added to the desugared code to ensure the
  loop body executes at most once per loop.

  BUG=v8:3683
  LOG=y

  Review URL: https://codereview.chromium.org/720863002

  Cr-Commit-Position: refs/heads/master@{nodejs#25363}

Fixes nodejs#9113 and nodejs#14411.
misterdjules pushed a commit to misterdjules/node that referenced this issue Apr 28, 2015
Backport b17eaaa5755e625493c5fe537f42b58838923c52 from upstream v8.

Original commit message:
  Fix desugaring of let bindings in for loops to handle continue properly

  This requires putting the original loop's body inside an inner for loop (with
  the same labels as the original loop) and re-binding the temp variables in its
  "next" expression. A second flag is added to the desugared code to ensure the
  loop body executes at most once per loop.

  BUG=v8:3683
  LOG=y

  Review URL: https://codereview.chromium.org/720863002

  Cr-Commit-Position: refs/heads/master@{nodejs#25363}

Fixes nodejs#9113 and nodejs#14411.
misterdjules pushed a commit to misterdjules/node that referenced this issue Apr 28, 2015
0724602 floats a patch on V8 that fixes
issue nodejs#9113 that would cause let bindings and continue statements in for
loops to not work properly.

This change adds a regression test that fails if that patch is not
properly floated, thus preventing us from not floating that patch after
future V8 upgrades.
@misterdjules misterdjules self-assigned this Apr 28, 2015
misterdjules pushed a commit to misterdjules/node that referenced this issue Apr 30, 2015
Backport b17eaaa5755e625493c5fe537f42b58838923c52 from upstream v8.

Original commit message:
  Fix desugaring of let bindings in for loops to handle continue properly

  This requires putting the original loop's body inside an inner for loop (with
  the same labels as the original loop) and re-binding the temp variables in its
  "next" expression. A second flag is added to the desugared code to ensure the
  loop body executes at most once per loop.

  BUG=v8:3683
  LOG=y

  Review URL: https://codereview.chromium.org/720863002

  Cr-Commit-Position: refs/heads/master@{nodejs#25363}

Fixes nodejs#9113 and nodejs#14411.
misterdjules pushed a commit to misterdjules/node that referenced this issue Apr 30, 2015
0724602 floats a patch on V8 that fixes
issue nodejs#9113 that would cause let bindings and continue statements in for
loops to not work properly.

This change adds a regression test that fails if that patch is not
properly floated, thus preventing us from not floating that patch after
future V8 upgrades.
misterdjules pushed a commit that referenced this issue Apr 30, 2015
Backport b17eaaa5755e625493c5fe537f42b58838923c52 from upstream v8.

Original commit message:
  Fix desugaring of let bindings in for loops to handle continue properly

  This requires putting the original loop's body inside an inner for loop (with
  the same labels as the original loop) and re-binding the temp variables in its
  "next" expression. A second flag is added to the desugared code to ensure the
  loop body executes at most once per loop.

  BUG=v8:3683
  LOG=y

  Review URL: https://codereview.chromium.org/720863002

  Cr-Commit-Position: refs/heads/master@{#25363}

Fixes #9113 and #14411.

Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #23948
misterdjules pushed a commit that referenced this issue Apr 30, 2015
0724602 floats a patch on V8 that fixes
issue #9113 that would cause let bindings and continue statements in for
loops to not work properly.

This change adds a regression test that fails if that patch is not
properly floated, thus preventing us from not floating that patch after
future V8 upgrades.

Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #23948
@misterdjules
Copy link

Fixed in 2a5f4bd and cb55a05, thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants