Modified: trunk/JSTests/test262.yaml (223893 => 223894)
--- trunk/JSTests/test262.yaml 2017-10-24 17:05:23 UTC (rev 223893)
+++ trunk/JSTests/test262.yaml 2017-10-24 17:11:39 UTC (rev 223894)
@@ -85790,7 +85790,7 @@
- path: test262/test/language/module-code/instn-iee-star-cycle-indirect-x_FIXTURE.js
cmd: prepareTest262Fixture
- path: test262/test/language/module-code/instn-iee-star-cycle.js
- cmd: runTest262 :fail, "NoException", ["../../../harness/assert.js", "../../../harness/sta.js"], [:module]
+ cmd: runTest262 :normal, "NoException", ["../../../harness/assert.js", "../../../harness/sta.js"], [:module]
- path: test262/test/language/module-code/instn-iee-trlng-comma.js
cmd: runTest262 :normal, "NoException", ["../../../harness/assert.js", "../../../harness/sta.js"], [:module]
- path: test262/test/language/module-code/instn-iee-trlng-comma_FIXTURE.js
@@ -85892,7 +85892,7 @@
- path: test262/test/language/module-code/instn-named-star-cycle-indirect-x_FIXTURE.js
cmd: prepareTest262Fixture
- path: test262/test/language/module-code/instn-named-star-cycle.js
- cmd: runTest262 :fail, "NoException", ["../../../harness/assert.js", "../../../harness/sta.js"], [:module]
+ cmd: runTest262 :normal, "NoException", ["../../../harness/assert.js", "../../../harness/sta.js"], [:module]
- path: test262/test/language/module-code/instn-once.js
cmd: runTest262 :normal, "NoException", ["../../../harness/assert.js", "../../../harness/sta.js"], [:module]
- path: test262/test/language/module-code/instn-resolve-empty-export.js
@@ -85998,7 +85998,7 @@
- path: test262/test/language/module-code/instn-star-star-cycle-indirect-x_FIXTURE.js
cmd: prepareTest262Fixture
- path: test262/test/language/module-code/instn-star-star-cycle.js
- cmd: runTest262 :fail, "NoException", ["../../../harness/assert.js", "../../../harness/sta.js"], [:module]
+ cmd: runTest262 :normal, "NoException", ["../../../harness/assert.js", "../../../harness/sta.js"], [:module]
- path: test262/test/language/module-code/instn-uniq-env-rec-other_FIXTURE.js
cmd: prepareTest262Fixture
- path: test262/test/language/module-code/instn-uniq-env-rec.js
Modified: trunk/Source/_javascript_Core/ChangeLog (223893 => 223894)
--- trunk/Source/_javascript_Core/ChangeLog 2017-10-24 17:05:23 UTC (rev 223893)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-10-24 17:11:39 UTC (rev 223894)
@@ -1,3 +1,22 @@
+2017-10-15 Yusuke Suzuki <[email protected]>
+
+ [JSC] modules can be visited more than once when resolving bindings through "star" exports as long as the exportName is different each time
+ https://bugs.webkit.org/show_bug.cgi?id=178308
+
+ Reviewed by Mark Lam.
+
+ With the change of the spec[1], we now do not need to remember star resolution modules.
+ We reflect this change to our implementation. Since this change is covered by test262,
+ this patch improves the score of test262.
+
+ We also add logging to ResolveExport to debug it easily.
+
+ [1]: https://github.com/tc39/ecma262/commit/a865e778ff0fc60e26e3e1c589635103710766a1
+
+ * runtime/AbstractModuleRecord.cpp:
+ (JSC::AbstractModuleRecord::ResolveQuery::dump const):
+ (JSC::AbstractModuleRecord::resolveExportImpl):
+
2017-10-24 Yusuke Suzuki <[email protected]>
[JSC] Use emitDumbVirtualCall in 32bit JIT
Modified: trunk/Source/_javascript_Core/runtime/AbstractModuleRecord.cpp (223893 => 223894)
--- trunk/Source/_javascript_Core/runtime/AbstractModuleRecord.cpp 2017-10-24 17:05:23 UTC (rev 223893)
+++ trunk/Source/_javascript_Core/runtime/AbstractModuleRecord.cpp 2017-10-24 17:11:39 UTC (rev 223894)
@@ -35,6 +35,9 @@
#include "UnlinkedModuleProgramCodeBlock.h"
namespace JSC {
+namespace AbstractModuleRecordInternal {
+static const bool verbose = false;
+} // namespace AbstractModuleRecordInternal
const ClassInfo AbstractModuleRecord::s_info = { "AbstractModuleRecord", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(AbstractModuleRecord) };
@@ -177,6 +180,7 @@
static bool equal(const ResolveQuery&, const ResolveQuery&);
static const bool safeToCompareToEmptyOrDeleted = true;
};
+ using HashTraits = WTF::CustomHashTraits<ResolveQuery>;
ResolveQuery(AbstractModuleRecord* moduleRecord, UniquedStringImpl* exportName)
: moduleRecord(moduleRecord)
@@ -211,6 +215,15 @@
return exportName.isHashTableDeletedValue();
}
+ void dump(PrintStream& out) const
+ {
+ if (!moduleRecord) {
+ out.print("<empty>");
+ return;
+ }
+ out.print(moduleRecord->moduleKey(), " \"", exportName.get(), "\"");
+ }
+
// The module record is not marked from the GC. But these records are reachable from the JSGlobalObject.
// So we don't care the reachability to this record.
AbstractModuleRecord* moduleRecord;
@@ -245,8 +258,11 @@
VM& vm = exec->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
- // http://www.ecma-international.org/ecma-262/6.0/#sec-resolveexport
+ if (AbstractModuleRecordInternal::verbose)
+ dataLog("Resolving ", root, "\n");
+ // https://tc39.github.io/ecma262/#sec-resolveexport
+
// How to avoid C++ recursion in this function:
// This function avoids C++ recursion of the naive ResolveExport implementation.
// Flatten the recursion to the loop with the task queue and frames.
@@ -480,7 +496,7 @@
// 4. Once we follow star links, we should not retrieve the result from the cache and should not cache the result.
// 5. Once we see star links, even if we have not yet traversed that star link path, we should disable caching.
- typedef WTF::HashSet<ResolveQuery, ResolveQuery::Hash, WTF::CustomHashTraits<ResolveQuery>> ResolveSet;
+ using ResolveSet = WTF::HashSet<ResolveQuery, ResolveQuery::Hash, ResolveQuery::HashTraits>;
enum class Type { Query, IndirectFallback, GatherStars };
struct Task {
ResolveQuery query;
@@ -487,9 +503,21 @@
Type type;
};
+ auto typeString = [] (Type type) -> const char* {
+ switch (type) {
+ case Type::Query:
+ return "Query";
+ case Type::IndirectFallback:
+ return "IndirectFallback";
+ case Type::GatherStars:
+ return "GatherStars";
+ }
+ RELEASE_ASSERT_NOT_REACHED();
+ return nullptr;
+ };
+
Vector<Task, 8> pendingTasks;
ResolveSet resolveSet;
- HashSet<AbstractModuleRecord*> starSet;
Vector<Resolution, 8> frames;
@@ -500,7 +528,7 @@
// Call when the query is not resolved in the current module.
// It will enqueue the star resolution requests. Return "false" if the error occurs.
auto resolveNonLocal = [&](const ResolveQuery& query) -> bool {
- // http://www.ecma-international.org/ecma-262/6.0/#sec-resolveexport
+ // https://tc39.github.io/ecma262/#sec-resolveexport
// section 15.2.1.16.3, step 6
// If the "default" name is not resolved in the current module, we need to throw an error and stop resolution immediately,
// Rationale to this error: A default export cannot be provided by an export *.
@@ -509,10 +537,6 @@
if (query.exportName == vm.propertyNames->defaultKeyword.impl())
return false;
- // step 7, If exportStarSet contains module, then return null.
- if (!starSet.add(query.moduleRecord).isNewEntry)
- return true;
-
// Enqueue the task to gather the results of the stars.
// And append the new Resolution frame to gather the local result of the stars.
pendingTasks.append(Task { query, Type::GatherStars });
@@ -519,7 +543,6 @@
foundStarLinks = true;
frames.append(Resolution::notFound());
-
// Enqueue the tasks in reverse order.
for (auto iterator = query.moduleRecord->starExportEntries().rbegin(), end = query.moduleRecord->starExportEntries().rend(); iterator != end; ++iterator) {
const RefPtr<UniquedStringImpl>& starModuleName = *iterator;
@@ -563,6 +586,9 @@
const Task task = pendingTasks.takeLast();
const ResolveQuery& query = task.query;
+ if (AbstractModuleRecordInternal::verbose)
+ dataLog(" ", typeString(task.type), " ", task.query, "\n");
+
switch (task.type) {
case Type::Query: {
AbstractModuleRecord* moduleRecord = query.moduleRecord;
