Bug 157806 – [ES6] Namespace object re-export should be handled as local export

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Keith Miller]

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.

[Radar WebKit Bug Importer]

<rdar://problem/26328773>

[Yusuke Suzuki]

It seems the bug of module loader. It seems that resolution.localName.impl() becomes nullptr in JSScope::abstractResolve.

[Yusuke Suzuki]

Created attachment 279231 [details]
Patch

[Mark Lam]

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.

[Yusuke Suzuki]

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 :)

[Yusuke Suzuki]

Committed r201085: <http://trac.webkit.org/changeset/201085>