WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
168684
[JSC] It should be possible create a label named let when parsing Statement in non strict mode
https://bugs.webkit.org/show_bug.cgi?id=168684
Summary
[JSC] It should be possible create a label named let when parsing Statement i...
Caio Lima
Reported
2017-02-21 15:49:17 PST
i.e case like the following codes: ``` if (true) let: x() ``` or ``` with ({}) let: y() ```
Attachments
WIP - Patch
(1.20 KB, patch)
2017-02-21 15:51 PST
,
Caio Lima
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
(828.79 KB, application/zip)
2017-02-21 16:57 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews117 for mac-elcapitan
(1.64 MB, application/zip)
2017-02-21 17:08 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews126 for ios-simulator-wk2
(720.61 KB, application/zip)
2017-02-21 17:12 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews101 for mac-elcapitan
(1.27 MB, application/zip)
2017-02-22 03:38 PST
,
Build Bot
no flags
Details
Patch
(8.15 KB, patch)
2017-03-10 06:38 PST
,
Caio Lima
no flags
Details
Formatted Diff
Diff
Patch
(8.15 KB, patch)
2017-03-10 19:21 PST
,
Caio Lima
no flags
Details
Formatted Diff
Diff
Patch
(9.67 KB, patch)
2017-03-12 19:59 PDT
,
Caio Lima
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Caio Lima
Comment 1
2017-02-21 15:51:09 PST
Created
attachment 302324
[details]
WIP - Patch It starts. I still need to test it and create test cases.
Build Bot
Comment 2
2017-02-21 16:24:49 PST
Comment on
attachment 302324
[details]
WIP - Patch
Attachment 302324
[details]
did not pass jsc-ews (mac): Output:
http://webkit-queues.webkit.org/results/3168805
New failing tests: jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-no-llint ChakraCore.yaml/ChakraCore/test/LetConst/DeclOutofBlock.js.default jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-no-cjit jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-ftl-no-cjit ChakraCore.yaml/ChakraCore/test/strict/
bug212755
.js.default jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-dfg-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-no-ftl jsc-layout-tests.yaml/js/script-tests/let-syntax.js.layout-ftl-eager-no-cjit ChakraCore.yaml/ChakraCore/test/strict/comma_bug219390.js.default
Build Bot
Comment 3
2017-02-21 16:57:14 PST
Comment on
attachment 302324
[details]
WIP - Patch
Attachment 302324
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/3168979
New failing tests: js/let-syntax.html
Build Bot
Comment 4
2017-02-21 16:57:16 PST
Created
attachment 302340
[details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 5
2017-02-21 17:07:59 PST
Comment on
attachment 302324
[details]
WIP - Patch
Attachment 302324
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/3168997
New failing tests: js/let-syntax.html
Build Bot
Comment 6
2017-02-21 17:08:03 PST
Created
attachment 302344
[details]
Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 7
2017-02-21 17:12:52 PST
Comment on
attachment 302324
[details]
WIP - Patch
Attachment 302324
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
http://webkit-queues.webkit.org/results/3169004
New failing tests: js/let-syntax.html
Build Bot
Comment 8
2017-02-21 17:12:55 PST
Created
attachment 302345
[details]
Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 9
2017-02-22 03:38:55 PST
Comment on
attachment 302324
[details]
WIP - Patch
Attachment 302324
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/3171878
New failing tests: js/let-syntax.html
Build Bot
Comment 10
2017-02-22 03:38:58 PST
Created
attachment 302383
[details]
Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Caio Lima
Comment 11
2017-03-10 06:38:57 PST
Created
attachment 304040
[details]
Patch
Caio Lima
Comment 12
2017-03-10 19:21:03 PST
Created
attachment 304118
[details]
Patch
Saam Barati
Comment 13
2017-03-12 11:45:49 PDT
Comment on
attachment 304118
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=304118&action=review
r=me with comments
> Source/JavaScriptCore/parser/Parser.cpp:1729 > + case LET: { > + if (!strictMode()) { > + bool allowFunctionDeclarationAsStatement = false; > + result = parseExpressionOrLabelStatement(context, allowFunctionDeclarationAsStatement); > + shouldSetPauseLocation = !context.shouldSkipPauseLocation(result); > + break; > + } > + goto defaultCase; > + }
This is duplicate code with he IDENT/AWAIT/YIELD case below. Maybe add a goto to that case, or make a lambda that is called both here and there.
> LayoutTests/js/script-tests/let-syntax.js:56 > +function shouldNotHaveSyntaxErrorSloopyOnly(str)
Typo: "Sloopy" => "Sloppy"
> LayoutTests/js/script-tests/let-syntax.js:96 > +shouldNotHaveSyntaxErrorSloopyOnly("let x; with ({}) let: y = 3;");
Can you also add a test where you omit the ":" after the label. Something like: "let x; with ({}) let y = 3;" And it should be a syntax error in sloppy mode.
Caio Lima
Comment 14
2017-03-12 19:27:34 PDT
(In reply to
comment #13
)
> Comment on
attachment 304118
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=304118&action=review
> > r=me with comments > > > Source/JavaScriptCore/parser/Parser.cpp:1729 > > + case LET: { > > + if (!strictMode()) { > > + bool allowFunctionDeclarationAsStatement = false; > > + result = parseExpressionOrLabelStatement(context, allowFunctionDeclarationAsStatement); > > + shouldSetPauseLocation = !context.shouldSkipPauseLocation(result); > > + break; > > + } > > + goto defaultCase; > > + } > > This is duplicate code with he IDENT/AWAIT/YIELD case below. Maybe add a > goto to that case, or make a lambda that is called both here and there.
Done.
> > LayoutTests/js/script-tests/let-syntax.js:56 > > +function shouldNotHaveSyntaxErrorSloopyOnly(str) > > Typo: "Sloopy" => "Sloppy"
Oops. Thanks!
> > LayoutTests/js/script-tests/let-syntax.js:96 > > +shouldNotHaveSyntaxErrorSloopyOnly("let x; with ({}) let: y = 3;"); > > Can you also add a test where you omit the ":" after the label. Something > like: > > "let x; with ({}) let y = 3;" > > And it should be a syntax error in sloppy mode.
Done. Thank you very much for the review Saam!
Caio Lima
Comment 15
2017-03-12 19:59:11 PDT
Created
attachment 304217
[details]
Patch
WebKit Commit Bot
Comment 16
2017-03-13 10:03:23 PDT
Comment on
attachment 304217
[details]
Patch Clearing flags on attachment: 304217 Committed
r213850
: <
http://trac.webkit.org/changeset/213850
>
WebKit Commit Bot
Comment 17
2017-03-13 10:03:28 PDT
All reviewed patches have been landed. Closing bug.
Yusuke Suzuki
Comment 18
2017-06-03 04:55:33 PDT
***
Bug 148098
has been marked as a duplicate of this bug. ***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug