Bug 147063 - lexical scoping is broken with respect to "break" and "continue"
Summary: lexical scoping is broken with respect to "break" and "continue"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on: 147070
Blocks: 31813
  Show dependency treegraph
 
Reported: 2015-07-17 19:26 PDT by Saam Barati
Modified: 2015-07-18 17:38 PDT (History)
12 users (show)

See Also:


Attachments
patch (22.95 KB, patch)
2015-07-18 02:02 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2015-07-17 19:26:26 PDT
There is some silliness in how byte code generator handles push/pop lexical scope and prepareScopeForNextForLoopIteration.
This should be skipped. There is a silly mistake to always assume that scopeRegister() will be the scope in which
we want to grab the parent scope of. This is fundamentally wrong. We want to grab the parent of the corresponding
lexical scope. scopeRegister() just refers to scope stack top which is not always what we want to pop from.
Comment 1 Saam Barati 2015-07-18 02:02:11 PDT
Created attachment 257027 [details]
patch
Comment 2 Saam Barati 2015-07-18 02:04:53 PDT
(In reply to comment #0)
> There is some silliness in how byte code generator handles push/pop lexical
> scope and prepareScopeForNextForLoopIteration.
> This should be skipped. There is a silly mistake to always assume that
> scopeRegister() will be the scope in which
> we want to grab the parent scope of. This is fundamentally wrong. We want to
> grab the parent of the corresponding
> lexical scope. scopeRegister() just refers to scope stack top which is not
> always what we want to pop from.

This assessment is still correct. But it doesn't tell the whole story.
This was a symptom, rather than the leading cause, of the problem this patch fixes.
Comment 3 WebKit Commit Bot 2015-07-18 13:13:31 PDT
Comment on attachment 257027 [details]
patch

Clearing flags on attachment: 257027

Committed r186996: <http://trac.webkit.org/changeset/186996>
Comment 4 WebKit Commit Bot 2015-07-18 13:13:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 WebKit Commit Bot 2015-07-18 14:17:21 PDT
Re-opened since this is blocked by bug 147070
Comment 7 Saam Barati 2015-07-18 17:38:49 PDT
Fixed 32-bit tests. They were timing out. I made the test have fewer loop iterations.

landed in:
http://trac.webkit.org/changeset/187003