Changeset 222421 in webkit for trunk/Source/JavaScriptCore


Ignore:
Timestamp:
Sep 22, 2017, 7:08:04 PM (8 years ago)
Author:
[email protected]
Message:

Speculatively change iteration protocall to use the same next function
​https://bugs.webkit.org/show_bug.cgi?id=175653

Reviewed by Saam Barati.

JSTests:

Change test to match the new iteration behavior.

  • stress/spread-optimized-properly.js:

Source/JavaScriptCore:

This patch speculatively makes a change to the iteration protocall to fetch the next
property immediately after calling the Symbol.iterator function. This is, in theory,
a breaking change, so we will see if this breaks things (most likely it won't as this
is a relatively subtle point).

See: ​https://github.com/tc39/ecma262/issues/976

  • builtins/IteratorHelpers.js:

(performIteration):

  • bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::emitEnumeration):
(JSC::BytecodeGenerator::emitIteratorNext):
(JSC::BytecodeGenerator::emitIteratorNextWithValue):
(JSC::BytecodeGenerator::emitDelegateYield):

  • bytecompiler/BytecodeGenerator.h:
  • bytecompiler/NodesCodegen.cpp:

(JSC::ArrayPatternNode::bindValue const):

  • inspector/JSInjectedScriptHost.cpp:

(Inspector::JSInjectedScriptHost::iteratorEntries):

  • runtime/IteratorOperations.cpp:

(JSC::iteratorNext):
(JSC::iteratorStep):
(JSC::iteratorClose):
(JSC::iteratorForIterable):

  • runtime/IteratorOperations.h:

(JSC::forEachInIterable):

  • runtime/JSGenericTypedArrayViewConstructorInlines.h:

(JSC::constructGenericTypedArrayViewFromIterator):
(JSC::constructGenericTypedArrayViewWithArguments):

LayoutTests:

Change test to match the new iteration behavior.

  • js/sequence-iterator-protocol-2-expected.txt:
  • js/sequence-iterator-protocol-2.html:
Location:
trunk/Source/JavaScriptCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r222417 r222421  
     12017-09-22  Keith Miller  <[email protected]>
     2
     3        Speculatively change iteration protocall to use the same next function
     4        https://bugs.webkit.org/show_bug.cgi?id=175653
     5
     6        Reviewed by Saam Barati.
     7
     8        This patch speculatively makes a change to the iteration protocall to fetch the next
     9        property immediately after calling the Symbol.iterator function. This is, in theory,
     10        a breaking change, so we will see if this breaks things (most likely it won't as this
     11        is a relatively subtle point).
     12
     13        See: https://github.com/tc39/ecma262/issues/976
     14
     15        * builtins/IteratorHelpers.js:
     16        (performIteration):
     17        * bytecompiler/BytecodeGenerator.cpp:
     18        (JSC::BytecodeGenerator::emitEnumeration):
     19        (JSC::BytecodeGenerator::emitIteratorNext):
     20        (JSC::BytecodeGenerator::emitIteratorNextWithValue):
     21        (JSC::BytecodeGenerator::emitDelegateYield):
     22        * bytecompiler/BytecodeGenerator.h:
     23        * bytecompiler/NodesCodegen.cpp:
     24        (JSC::ArrayPatternNode::bindValue const):
     25        * inspector/JSInjectedScriptHost.cpp:
     26        (Inspector::JSInjectedScriptHost::iteratorEntries):
     27        * runtime/IteratorOperations.cpp:
     28        (JSC::iteratorNext):
     29        (JSC::iteratorStep):
     30        (JSC::iteratorClose):
     31        (JSC::iteratorForIterable):
     32        * runtime/IteratorOperations.h:
     33        (JSC::forEachInIterable):
     34        * runtime/JSGenericTypedArrayViewConstructorInlines.h:
     35        (JSC::constructGenericTypedArrayViewFromIterator):
     36        (JSC::constructGenericTypedArrayViewWithArguments):
     37
    1382017-09-22  Fujii Hironori  <[email protected]>
    239

  • trunk/Source/JavaScriptCore/builtins/IteratorHelpers.js

    r208637 r222421  
    3434
    3535    let iterator = iterable.@iteratorSymbol();
     36    let next = iterator.next;
    3637    let item;
    3738    let index = 0;
    3839    while (true) {
    39         item = iterator.next();
     40        item = next.@call(iterator);
    4041        if (!@isObject(item))
    4142            @throwTypeError("Iterator result interface is not an object");

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r221358 r222421  
    43164316    RefPtr<RegisterID> subject = newTemporary();
    43174317    emitNode(subject.get(), subjectNode);
    4318 
    43194318    RefPtr<RegisterID> iterator = isForAwait ? emitGetAsyncIterator(subject.get(), node) : emitGetIterator(subject.get(), node);
     4319    RefPtr<RegisterID> nextMethod = emitGetById(newTemporary(), iterator.get(), propertyNames().next);
    43204320
    43214321    Ref<Label> loopDone = newLabel();
    … …  
    44234423
    44244424        {
    4425             emitIteratorNext(value.get(), iterator.get(), node, isForAwait ? EmitAwait::Yes : EmitAwait::No);
     4425            emitIteratorNext(value.get(), nextMethod.get(), iterator.get(), node, isForAwait ? EmitAwait::Yes : EmitAwait::No);
    44264426
    44274427            emitJumpIfTrue(emitGetById(newTemporary(), value.get(), propertyNames().done), loopDone.get());
    … …  
    45884588}
    45894589   
    4590 RegisterID* BytecodeGenerator::emitIteratorNext(RegisterID* dst, RegisterID* iterator, const ThrowableExpressionData* node, EmitAwait doEmitAwait)
     4590RegisterID* BytecodeGenerator::emitIteratorNext(RegisterID* dst, RegisterID* nextMethod, RegisterID* iterator, const ThrowableExpressionData* node, EmitAwait doEmitAwait)
    45914591{
    45924592    {
    4593         RefPtr<RegisterID> next = emitGetById(newTemporary(), iterator, propertyNames().next);
    45944593        CallArguments nextArguments(*this, nullptr);
    45954594        emitMove(nextArguments.thisRegister(), iterator);
    4596         emitCall(dst, next.get(), NoExpectedFunction, nextArguments, node->divot(), node->divotStart(), node->divotEnd(), DebuggableCall::No);
     4595        emitCall(dst, nextMethod, NoExpectedFunction, nextArguments, node->divot(), node->divotStart(), node->divotEnd(), DebuggableCall::No);
    45974596
    45984597        if (doEmitAwait == EmitAwait::Yes)
    … …  
    46084607}
    46094608
    4610 RegisterID* BytecodeGenerator::emitIteratorNextWithValue(RegisterID* dst, RegisterID* iterator, RegisterID* value, const ThrowableExpressionData* node)
     4609RegisterID* BytecodeGenerator::emitIteratorNextWithValue(RegisterID* dst, RegisterID* nextMethod, RegisterID* iterator, RegisterID* value, const ThrowableExpressionData* node)
    46114610{
    46124611    {
    4613         RefPtr<RegisterID> next = emitGetById(newTemporary(), iterator, propertyNames().next);
    46144612        CallArguments nextArguments(*this, nullptr, 1);
    46154613        emitMove(nextArguments.thisRegister(), iterator);
    46164614        emitMove(nextArguments.argumentRegister(0), value);
    4617         emitCall(dst, next.get(), NoExpectedFunction, nextArguments, node->divot(), node->divotStart(), node->divotEnd(), DebuggableCall::No);
     4615        emitCall(dst, nextMethod, NoExpectedFunction, nextArguments, node->divot(), node->divotStart(), node->divotEnd(), DebuggableCall::No);
    46184616    }
    46194617
    … …  
    49174915    emitLabel(asyncIteratorNotFound.get());
    49184916
    4919     RefPtr<RegisterID> commonIterator = emitGetById(newTemporary(), argument, propertyNames().iteratorSymbol);
    4920     emitCallIterator(commonIterator.get(), argument, node);
     4917    RefPtr<RegisterID> commonIterator = emitGetIterator(argument, node);
    49214918    emitMove(iterator.get(), commonIterator.get());
    49224919
    … …  
    49484945    {
    49494946        RefPtr<RegisterID> iterator = parseMode() == SourceParseMode::AsyncGeneratorBodyMode ? emitGetAsyncIterator(argument, node) : emitGetIterator(argument, node);
     4947        RefPtr<RegisterID> nextMethod = emitGetById(newTemporary(), iterator.get(), propertyNames().next);
    49504948
    49514949        Ref<Label> loopDone = newLabel();
    … …  
    50455043
    50465044            emitLabel(nextElement.get());
    5047             emitIteratorNextWithValue(value.get(), iterator.get(), value.get(), node);
     5045            emitIteratorNextWithValue(value.get(), nextMethod.get(), iterator.get(), value.get(), node);
    50485046
    50495047            emitLabel(branchOnResult.get());

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r221622 r222421  
    781781        void emitRequireObjectCoercible(RegisterID* value, const String& error);
    782782
    783         RegisterID* emitIteratorNext(RegisterID* dst, RegisterID* iterator, const ThrowableExpressionData* node, JSC::EmitAwait = JSC::EmitAwait::No);
    784         RegisterID* emitIteratorNextWithValue(RegisterID* dst, RegisterID* iterator, RegisterID* value, const ThrowableExpressionData* node);
     783        RegisterID* emitIteratorNext(RegisterID* dst, RegisterID* nextMethod, RegisterID* iterator, const ThrowableExpressionData* node, JSC::EmitAwait = JSC::EmitAwait::No);
     784        RegisterID* emitIteratorNextWithValue(RegisterID* dst, RegisterID* nextMethod, RegisterID* iterator, RegisterID* value, const ThrowableExpressionData* node);
    785785        void emitIteratorClose(RegisterID* iterator, const ThrowableExpressionData* node, EmitAwait = EmitAwait::No);
    786786

  • trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

    r222115 r222421  
    39453945        generator.emitCall(iterator.get(), iterator.get(), NoExpectedFunction, args, divot(), divotStart(), divotEnd(), DebuggableCall::No);
    39463946    }
     3947    RefPtr<RegisterID> nextMethod = generator.emitGetById(generator.newTemporary(), iterator.get(), generator.propertyNames().next);
    39473948
    39483949    if (m_targetPatterns.isEmpty()) {
    … …  
    39633964
    39643965            RefPtr<RegisterID> value = generator.newTemporary();
    3965             generator.emitIteratorNext(value.get(), iterator.get(), this);
     3966            generator.emitIteratorNext(value.get(), nextMethod.get(), iterator.get(), this);
    39663967            generator.emitGetById(done.get(), value.get(), generator.propertyNames().done);
    39673968            generator.emitJumpIfTrue(done.get(), iterationSkipped.get());
    … …  
    39994000
    40004001            RefPtr<RegisterID> value = generator.newTemporary();
    4001             generator.emitIteratorNext(value.get(), iterator.get(), this);
     4002            generator.emitIteratorNext(value.get(), nextMethod.get(), iterator.get(), this);
    40024003            generator.emitGetById(done.get(), value.get(), generator.propertyNames().done);
    40034004            generator.emitJumpIfTrue(done.get(), iterationDone.get());

  • trunk/Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp

    r221822 r222421  
    587587        return jsUndefined();
    588588
     589    IterationRecord iterationRecord = { iterator, iterator.get(exec, vm.propertyNames->next) };
     590
    589591    unsigned numberToFetch = 5;
    590592    JSValue numberToFetchArg = exec->argument(1);
    … …  
    598600
    599601    for (unsigned i = 0; i < numberToFetch; ++i) {
    600         JSValue next = iteratorStep(exec, iterator);
     602        JSValue next = iteratorStep(exec, iterationRecord);
    601603        if (UNLIKELY(scope.exception()) || next.isFalse())
    602604            break;
    … …  
    610612        if (UNLIKELY(scope.exception())) {
    611613            scope.release();
    612             iteratorClose(exec, iterator);
     614            iteratorClose(exec, iterationRecord);
    613615            break;
    614616        }

  • trunk/Source/JavaScriptCore/runtime/IteratorOperations.cpp

    r218794 r222421  
    3737namespace JSC {
    3838
    39 JSValue iteratorNext(ExecState* exec, JSValue iterator, JSValue value)
    40 {
    41     VM& vm = exec->vm();
    42     auto scope = DECLARE_THROW_SCOPE(vm);
    43 
    44     JSValue nextFunction = iterator.get(exec, vm.propertyNames->next);
    45     RETURN_IF_EXCEPTION(scope, JSValue());
     39JSValue iteratorNext(ExecState* exec, IterationRecord iterationRecord, JSValue argument)
     40{
     41    VM& vm = exec->vm();
     42    auto scope = DECLARE_THROW_SCOPE(vm);
     43
     44    JSValue iterator = iterationRecord.iterator;
     45    JSValue nextFunction = iterationRecord.nextMethod;
    4646
    4747    CallData nextFunctionCallData;
    … …  
    5151
    5252    MarkedArgumentBuffer nextFunctionArguments;
    53     if (!value.isEmpty())
    54         nextFunctionArguments.append(value);
     53    if (!argument.isEmpty())
     54        nextFunctionArguments.append(argument);
    5555    JSValue result = call(exec, nextFunction, nextFunctionCallType, nextFunctionCallData, iterator, nextFunctionArguments);
    5656    RETURN_IF_EXCEPTION(scope, JSValue());
    … …  
    6262}
    6363
    64 JSValue iteratorNext(ExecState* exec, JSValue iterator)
    65 {
    66     return iteratorNext(exec, iterator, JSValue());
    67 }
    68 
    6964JSValue iteratorValue(ExecState* exec, JSValue iterResult)
    7065{
    … …  
    7873}
    7974
    80 JSValue iteratorStep(ExecState* exec, JSValue iterator)
    81 {
    82     VM& vm = exec->vm();
    83     auto scope = DECLARE_THROW_SCOPE(vm);
    84 
    85     JSValue result = iteratorNext(exec, iterator);
     75JSValue iteratorStep(ExecState* exec, IterationRecord iterationRecord)
     76{
     77    VM& vm = exec->vm();
     78    auto scope = DECLARE_THROW_SCOPE(vm);
     79
     80    JSValue result = iteratorNext(exec, iterationRecord);
    8681    RETURN_IF_EXCEPTION(scope, JSValue());
    8782    bool done = iteratorComplete(exec, result);
    … …  
    9287}
    9388
    94 void iteratorClose(ExecState* exec, JSValue iterator)
     89void iteratorClose(ExecState* exec, IterationRecord iterationRecord)
    9590{
    9691    VM& vm = exec->vm();
    … …  
    10398        catchScope.clearException();
    10499    }
    105     JSValue returnFunction = iterator.get(exec, vm.propertyNames->returnKeyword);
     100    JSValue returnFunction = iterationRecord.iterator.get(exec, vm.propertyNames->returnKeyword);
    106101    RETURN_IF_EXCEPTION(throwScope, void());
    107102
    … …  
    123118
    124119    MarkedArgumentBuffer returnFunctionArguments;
    125     JSValue innerResult = call(exec, returnFunction, returnFunctionCallType, returnFunctionCallData, iterator, returnFunctionArguments);
     120    JSValue innerResult = call(exec, returnFunction, returnFunctionCallType, returnFunctionCallData, iterationRecord.iterator, returnFunctionArguments);
    126121
    127122    if (exception) {
    … …  
    191186}
    192187
    193 JSValue iteratorForIterable(ExecState& state, JSObject* object, JSValue iteratorMethod)
     188IterationRecord iteratorForIterable(ExecState& state, JSObject* object, JSValue iteratorMethod)
    194189{
    195190    VM& vm = state.vm();
    … …  
    212207    }
    213208
    214     return iterator;
    215 }
    216 
    217 JSValue iteratorForIterable(ExecState* state, JSValue iterable)
     209    JSValue nextMethod = iterator.getObject()->get(&state, vm.propertyNames->next);
     210    RETURN_IF_EXCEPTION(scope, { });
     211
     212    return { iterator, nextMethod };
     213}
     214
     215IterationRecord iteratorForIterable(ExecState* state, JSValue iterable)
    218216{
    219217    VM& vm = state->vm();
    … …  
    221219   
    222220    JSValue iteratorFunction = iterable.get(state, vm.propertyNames->iteratorSymbol);
    223     RETURN_IF_EXCEPTION(scope, JSValue());
     221    RETURN_IF_EXCEPTION(scope, { });
    224222   
    225223    CallData iteratorFunctionCallData;
    … …  
    227225    if (iteratorFunctionCallType == CallType::None) {
    228226        throwTypeError(state, scope);
    229         return JSValue();
     227        return { };
    230228    }
    231229
    232230    ArgList iteratorFunctionArguments;
    233231    JSValue iterator = call(state, iteratorFunction, iteratorFunctionCallType, iteratorFunctionCallData, iterable, iteratorFunctionArguments);
    234     RETURN_IF_EXCEPTION(scope, JSValue());
     232    RETURN_IF_EXCEPTION(scope, { });
    235233
    236234    if (!iterator.isObject()) {
    237235        throwTypeError(state, scope);
    238         return JSValue();
    239     }
    240 
    241     return iterator;
     236        return { };
     237    }
     238
     239    JSValue nextMethod = iterator.getObject()->get(state, vm.propertyNames->next);
     240    RETURN_IF_EXCEPTION(scope, { });
     241
     242    return { iterator, nextMethod };
    242243}
    243244

  • trunk/Source/JavaScriptCore/runtime/IteratorOperations.h

    r217529 r222421  
    3333namespace JSC {
    3434
    35 JSValue iteratorNext(ExecState*, JSValue iterator, JSValue);
    36 JSValue iteratorNext(ExecState*, JSValue iterator);
    37 JS_EXPORT_PRIVATE JSValue iteratorValue(ExecState*, JSValue iterator);
    38 bool iteratorComplete(ExecState*, JSValue iterator);
    39 JS_EXPORT_PRIVATE JSValue iteratorStep(ExecState*, JSValue iterator);
    40 JS_EXPORT_PRIVATE void iteratorClose(ExecState*, JSValue iterator);
     35struct IterationRecord {
     36    JSValue iterator;
     37    JSValue nextMethod;
     38};
     39
     40JSValue iteratorNext(ExecState*, IterationRecord, JSValue argument = JSValue());
     41JS_EXPORT_PRIVATE JSValue iteratorValue(ExecState*, JSValue iterResult);
     42bool iteratorComplete(ExecState*, JSValue iterResult);
     43JS_EXPORT_PRIVATE JSValue iteratorStep(ExecState*, IterationRecord);
     44JS_EXPORT_PRIVATE void iteratorClose(ExecState*, IterationRecord);
    4145JS_EXPORT_PRIVATE JSObject* createIteratorResultObject(ExecState*, JSValue, bool done);
    4246
    … …  
    4448
    4549JS_EXPORT_PRIVATE JSValue iteratorMethod(ExecState&, JSObject*);
    46 JS_EXPORT_PRIVATE JSValue iteratorForIterable(ExecState&, JSObject*, JSValue iteratorMethod);
     50JS_EXPORT_PRIVATE IterationRecord iteratorForIterable(ExecState&, JSObject*, JSValue iteratorMethod);
     51JS_EXPORT_PRIVATE IterationRecord iteratorForIterable(ExecState*, JSValue iterable);
    4752
    4853JS_EXPORT_PRIVATE JSValue iteratorMethod(ExecState&, JSObject*);
    4954JS_EXPORT_PRIVATE bool hasIteratorMethod(ExecState&, JSValue);
    50 
    51 JS_EXPORT_PRIVATE JSValue iteratorForIterable(ExecState*, JSValue iterable);
    5255
    5356template<typename CallBackType>
    … …  
    5760    auto scope = DECLARE_THROW_SCOPE(vm);
    5861
    59     JSValue iterator = iteratorForIterable(exec, iterable);
     62    IterationRecord iterationRecord = iteratorForIterable(exec, iterable);
    6063    RETURN_IF_EXCEPTION(scope, void());
    6164    while (true) {
    62         JSValue next = iteratorStep(exec, iterator);
     65        JSValue next = iteratorStep(exec, iterationRecord);
    6366        if (UNLIKELY(scope.exception()) || next.isFalse())
    6467            return;
    … …  
    7073        if (UNLIKELY(scope.exception())) {
    7174            scope.release();
    72             iteratorClose(exec, iterator);
     75            iteratorClose(exec, iterationRecord);
    7376            return;
    7477        }
    … …  
    8285    auto scope = DECLARE_THROW_SCOPE(vm);
    8386
    84     auto iterator = iteratorForIterable(state, iterable, iteratorMethod);
     87    auto iterationRecord = iteratorForIterable(state, iterable, iteratorMethod);
    8588    RETURN_IF_EXCEPTION(scope, void());
    8689    while (true) {
    87         JSValue next = iteratorStep(&state, iterator);
     90        JSValue next = iteratorStep(&state, iterationRecord);
    8891        if (UNLIKELY(scope.exception()) || next.isFalse())
    8992            return;
    … …  
    9598        if (UNLIKELY(scope.exception())) {
    9699            scope.release();
    97             iteratorClose(&state, iterator);
     100            iteratorClose(&state, iterationRecord);
    98101            return;
    99102        }

  • trunk/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h

    r221849 r222421  
    7878
    7979template<typename ViewClass>
    80 inline JSObject* constructGenericTypedArrayViewFromIterator(ExecState* exec, Structure* structure, JSValue iterator)
    81 {
    82     VM& vm = exec->vm();
    83     auto scope = DECLARE_THROW_SCOPE(vm);
    84 
    85     if (!iterator.isObject())
    86         return throwTypeError(exec, scope, ASCIILiteral("Symbol.Iterator for the first argument did not return an object."));
     80inline JSObject* constructGenericTypedArrayViewFromIterator(ExecState* exec, Structure* structure, JSObject* iterable, JSValue iteratorMethod)
     81{
     82    VM& vm = exec->vm();
     83    auto scope = DECLARE_THROW_SCOPE(vm);
    8784
    8885    MarkedArgumentBuffer storage;
    89     while (true) {
    90         JSValue next = iteratorStep(exec, iterator);
    91         RETURN_IF_EXCEPTION(scope, nullptr);
    92 
    93         if (next.isFalse())
    94             break;
    95 
    96         JSValue nextItem = iteratorValue(exec, next);
    97         RETURN_IF_EXCEPTION(scope, nullptr);
    98 
    99         storage.append(nextItem);
    100     }
     86    forEachInIterable(*exec, iterable, iteratorMethod, [&] (VM&, ExecState&, JSValue value) {
     87        storage.append(value);
     88    });
     89    RETURN_IF_EXCEPTION(scope, nullptr);
    10190
    10291    ViewClass* result = ViewClass::createUninitialized(exec, structure, storage.size());
    … …  
    173162                    || hasAnyArrayStorage(object->indexingType()))) {
    174163
    175                     CallData callData;
    176                     CallType callType = getCallData(iteratorFunc, callData);
    177                     if (callType == CallType::None)
    178                         return throwTypeError(exec, scope, ASCIILiteral("Symbol.Iterator for the first argument cannot be called."));
    179 
    180                     ArgList arguments;
    181                     JSValue iterator = call(exec, iteratorFunc, callType, callData, object, arguments);
    182                     RETURN_IF_EXCEPTION(scope, nullptr);
    183 
    184                     scope.release();
    185                     return constructGenericTypedArrayViewFromIterator<ViewClass>(exec, structure, iterator);
     164                    return constructGenericTypedArrayViewFromIterator<ViewClass>(exec, structure, object, iteratorFunc);
    186165            }
    187166
Note: See TracChangeset for help on using the changeset viewer.