Bug 157806 - [ES6] Namespace object re-export should be handled as local export
Summary: [ES6] Namespace object re-export should be handled as local export
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-05-17 13:00 PDT by Keith Miller
Modified: 2016-05-18 10:45 PDT (History)
7 users (show)

See Also:


Attachments
Patch (11.17 KB, patch)
2016-05-18 01:42 PDT, Yusuke Suzuki
mark.lam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2016-05-17 13:00:19 PDT
Running test262/test/language/module-code/instn-star-equality.js in the new test262 runner segmentation faults. I'm not sure exactly why yet, for now I'm omitting the test from the test suite. It's possible that this is just a bug in the test runner and not in the code, although unlikely.
Comment 1 Radar WebKit Bug Importer 2016-05-17 13:01:05 PDT
<rdar://problem/26328773>
Comment 2 Yusuke Suzuki 2016-05-17 19:35:43 PDT
It seems the bug of module loader. It seems that resolution.localName.impl() becomes nullptr in JSScope::abstractResolve.
Comment 3 Yusuke Suzuki 2016-05-18 01:42:58 PDT
Created attachment 279231 [details]
Patch
Comment 4 Mark Lam 2016-05-18 09:57:59 PDT
Comment on attachment 279231 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279231&action=review

LGTM with a comment.

> Source/JavaScriptCore/parser/ModuleAnalyzer.cpp:97
>          // Exported namespace binding.
>          // import * as namespace from "mod"
>          // export { namespace }
> -        moduleRecord()->addExportEntry(JSModuleRecord::ExportEntry::createNamespace(exportName, importEntry.moduleRequest));
> +        //
> +        // Sec 15.2.1.16.1 step 11-a-ii-2-b https://tc39.github.io/ecma262/#sec-parsemodule
> +        // Namespace export is handled as local export since a namespace object binding itself is implemented as a local binding.
> +        moduleRecord()->addExportEntry(JSModuleRecord::ExportEntry::createLocal(exportName, Identifier::fromUid(m_vm, localName.get())));
>          return;
>      }

The lines above this:
    Optional<JSModuleRecord::ImportEntry> optionalImportEntry = moduleRecord()->tryGetImportEntry(localName.get());
    ASSERT(optionalImportEntry);
    const JSModuleRecord::ImportEntry& importEntry = *optionalImportEntry;

... can be moved below this because they are only needed below.
Comment 5 Yusuke Suzuki 2016-05-18 10:35:23 PDT
Comment on attachment 279231 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279231&action=review

Thanks!

>> Source/JavaScriptCore/parser/ModuleAnalyzer.cpp:97
>>      }
> 
> The lines above this:
>     Optional<JSModuleRecord::ImportEntry> optionalImportEntry = moduleRecord()->tryGetImportEntry(localName.get());
>     ASSERT(optionalImportEntry);
>     const JSModuleRecord::ImportEntry& importEntry = *optionalImportEntry;
> 
> ... can be moved below this because they are only needed below.

Nice catch. Moved :)
Comment 6 Yusuke Suzuki 2016-05-18 10:45:15 PDT
Committed r201085: <http://trac.webkit.org/changeset/201085>