WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
160343
[ES6] Module binding can be exported by multiple names
https://bugs.webkit.org/show_bug.cgi?id=160343
Summary
[ES6] Module binding can be exported by multiple names
Yusuke Suzuki
Reported
2016-07-29 09:53:41 PDT
Module can export the same local binding by using multiple names. var value = 42; export { value }; export { value as value2 };
Attachments
Patch
(42.69 KB, patch)
2016-07-30 12:57 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(42.69 KB, patch)
2016-07-30 13:12 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(45.43 KB, patch)
2016-07-31 00:04 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1
2016-07-30 12:57:25 PDT
Created
attachment 284952
[details]
Patch
Yusuke Suzuki
Comment 2
2016-07-30 13:12:36 PDT
Created
attachment 284953
[details]
Patch
Saam Barati
Comment 3
2016-07-30 16:19:12 PDT
Comment on
attachment 284953
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=284953&action=review
r=me
> Source/JavaScriptCore/parser/Parser.cpp:2793 > +template <class TreeBuilder> typename TreeBuilder::ExportSpecifier Parser<LexerType>::parseExportSpecifier(TreeBuilder& context, Vector<std::pair<const Identifier*, const Identifier*>>& maybeExportedLocalNames, bool& hasKeywordForLocalBindings)
Why is this called "maybeExportedLocalNames" instead of just "exportedLocalNames". It looks like we iterate the list later and export all the names.
> Source/JavaScriptCore/tests/modules/aliased-names/main.js:10 > +export { change }
Maybe it's worth adding a test that should fail when the program is something like: export {foo as bar, baz as bar}
Yusuke Suzuki
Comment 4
2016-07-31 00:02:23 PDT
Comment on
attachment 284953
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=284953&action=review
Thanks!
>> Source/JavaScriptCore/parser/Parser.cpp:2793 >> +template <class TreeBuilder> typename TreeBuilder::ExportSpecifier Parser<LexerType>::parseExportSpecifier(TreeBuilder& context, Vector<std::pair<const Identifier*, const Identifier*>>& maybeExportedLocalNames, bool& hasKeywordForLocalBindings) > > Why is this called "maybeExportedLocalNames" instead of just "exportedLocalNames". > It looks like we iterate the list later and export all the names.
This is because maybeExportedLocalNames could be non-local names. Here, we need to handle two syntax. 1. export { a, b, c }; 2. export { a, b, c } from "other-module"; In the first case, a, b, c should be local bindings. So it is OK. However, in the second case, a, b, c is directly exported from the "other-module" and there are no local bindings in the current module (And we must not create such local binding in the current module). So in this case, these collected names are not exportedLocalNames. We cannot determine which the current parsing syntax is ((1) OR (2) ?) until we encounter the `from "other-module"` part. So when parsing export specifiers (here), these collected names are `maybe` exported local names.
>> Source/JavaScriptCore/tests/modules/aliased-names/main.js:10 >> +export { change } > > Maybe it's worth adding a test that should fail when the program is something like: > export {foo as bar, baz as bar}
Sounds nice!
Yusuke Suzuki
Comment 5
2016-07-31 00:04:49 PDT
Created
attachment 284962
[details]
Patch
Yusuke Suzuki
Comment 6
2016-07-31 00:05:18 PDT
Comment on
attachment 284962
[details]
Patch Oops, accidentally cleared r+. I'll land manually.
Yusuke Suzuki
Comment 7
2016-07-31 00:06:18 PDT
Committed
r203953
: <
http://trac.webkit.org/changeset/203953
>
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