Changeset 269670 in webkit


Ignore:
Timestamp:
Nov 10, 2020 6:28:59 PM (21 months ago)
Author:
Ross Kirsling
Message:

Align %TypedArray% behavior with recent spec adjustments
​https://bugs.webkit.org/show_bug.cgi?id=218776

Reviewed by Yusuke Suzuki.

JSTests:

  • stress/reflect-set.js:
  • stress/typedarray-functions-with-neutered.js:
  • stress/typedarray-includes.js:
  • stress/typedarray-indexOf.js:
  • stress/typedarray-join.js: Added.
  • stress/typedarray-lastIndexOf.js:

Update tests.

  • test262/expectations.yaml:

Mark a handful of test cases as temporarily failing.
These will disappear in a future test262 update.

Source/JavaScriptCore:

The recent spec changes for typed arrays with detached buffers had certain ripple effects,
namely the following two PRs which will be presented in next week's TC39 meeting.
Since no controversy is expected, this patch addresses them now, though test262 adjustments are forthcoming.

  1. ​https://github.com/tc39/ecma262/pull/2210 It is correct that ta[i] = n doesn't throw when ta has a detached buffer or i is otherwise OOB, but by not throwing, Reflect.set(ta, i, n) is obliged to return true.
  1. ​https://github.com/tc39/ecma262/pull/2221 Until now, %TypedArray%.prototype.{includes, indexOf, join, lastIndexOf} lacked a rigorous specification; in particular, each has a parameter that may detach the buffer upon valueOf or toString, and the expected behavior was not made clear. It seems most sensible to do what the corresponding Array methods do upon array.length = 0: make use of the cached length but don't access indices, such that indexOf/lastIndexOf return -1 while includes/join act as if the elements were all undefined.
  • runtime/JSGenericTypedArrayViewInlines.h:

(JSC::JSGenericTypedArrayView<Adaptor>::put):
(JSC::JSGenericTypedArrayView<Adaptor>::defineOwnProperty):
(JSC::JSGenericTypedArrayView<Adaptor>::putByIndex):

  • runtime/JSGenericTypedArrayViewPrototypeFunctions.h:

(JSC::genericTypedArrayViewProtoFuncIncludes):
(JSC::genericTypedArrayViewProtoFuncIndexOf):
(JSC::genericTypedArrayViewProtoFuncJoin):
(JSC::genericTypedArrayViewProtoFuncLastIndexOf):

Location:
trunk
Files:
1 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r269667 r269670  
     12020-11-10  Ross Kirsling  <ross.kirsling@sony.com>
     2
     3        Align %TypedArray% behavior with recent spec adjustments
     4        https://bugs.webkit.org/show_bug.cgi?id=218776
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        * stress/reflect-set.js:
     9        * stress/typedarray-functions-with-neutered.js:
     10        * stress/typedarray-includes.js:
     11        * stress/typedarray-indexOf.js:
     12        * stress/typedarray-join.js: Added.
     13        * stress/typedarray-lastIndexOf.js:
     14        Update tests.
     15
     16        * test262/expectations.yaml:
     17        Mark a handful of test cases as temporarily failing.
     18        These will disappear in a future test262 update.
     19
    1202020-11-10  Michael Catanzaro  <mcatanzaro@gnome.org>
    221

  • trunk/JSTests/stress/reflect-set.js

    r212196 r269670  
    692692        shouldBe(Reflect.get(object, 0), 42);
    693693        shouldBe(receiver.hasOwnProperty(0), false);
     694
     695        var object = new TypedArray(1);
     696        transferArrayBuffer(object.buffer);
     697        shouldBe(Reflect.set(object, 0, 42), true);
     698        shouldBe(Reflect.get(object, 0), undefined);
    694699    };
    695700})());

  • trunk/JSTests/stress/typedarray-functions-with-neutered.js

    r268640 r269670  
    7979    { func:proto.copyWithin, args:["prim", "prim", "prim"] },
    8080    { func:proto.fill, args:["prim", "prim", "prim"] },
    81     { func:proto.indexOf, args:["na", "prim"] },
    82     { func:proto.includes, args:["na", "prim"] },
    83     { func:proto.join, args:["prim"] },
    84     { func:proto.lastIndexOf, args:["na", "prim"] },
    8581    { func:proto.set, args:["array", "prim"] },
    8682    { func:proto.slice, args:["prim", "prim"] },

  • trunk/JSTests/stress/typedarray-includes.js

    r264304 r269670  
    22
    33for (constructor of typedArrays) {
    4     let a = new constructor(10);
     4    a = new constructor(10);
    55    passed = true;
    66    result = a.includes({ valueOf() { passed = false; return 1; } });
    77    shouldBeTrue("passed");
    88    shouldBeFalse("result");
     9
     10    shouldBeTrue("a.includes(undefined, { valueOf() { transferArrayBuffer(a.buffer); return 0; } })");
     11    shouldThrow("a.includes(undefined)");
     12
     13    a = new constructor(1);
     14    shouldBeFalse("a.includes(null, { valueOf() { transferArrayBuffer(a.buffer); return 0; } })");
    915}
    1016finishJSTest();

  • trunk/JSTests/stress/typedarray-indexOf.js

    r264304 r269670  
    4545    shouldBe("a.indexOf({1: ''})", "-1");
    4646    shouldBe("a.indexOf(\"\")", "-1");
     47
     48    shouldBe("a.indexOf(undefined, { valueOf() { transferArrayBuffer(a.buffer); return 0; } })", "-1");
     49    shouldThrow("a.indexOf(undefined)");
    4750}
    4851

  • trunk/JSTests/stress/typedarray-lastIndexOf.js

    r264304 r269670  
    4242    shouldBe("a.lastIndexOf({1: ''})", "-1");
    4343    shouldBe("a.lastIndexOf(\"\")", "-1");
     44
     45    shouldBe("a.lastIndexOf(undefined, { valueOf() { transferArrayBuffer(a.buffer); return 0; } })", "-1");
     46    shouldThrow("a.lastIndexOf(undefined)");
    4447}
    4548

  • trunk/JSTests/test262/expectations.yaml

    r269574 r269670  
    839839  default: 'Test262Error: Expected a TypeError but got a Test262Error (Testing with Float64Array.)'
    840840  strict mode: 'Test262Error: Expected a TypeError but got a Test262Error (Testing with Float64Array.)'
     841test/built-ins/TypedArray/prototype/includes/detached-buffer-tointeger.js:
     842  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
     843  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
    841844test/built-ins/TypedArray/prototype/map/callbackfn-detachbuffer.js:
    842845  default: 'Test262Error: Expected a TypeError but got a Test262Error (Testing with Float64Array.)'
    … …  
    935938  default: 'Test262Error: `sample[0] = 1` is false Expected SameValue(«1», «false») to be true (Testing with Float64Array.)'
    936939  strict mode: 'Test262Error: `sample[0] = 1` is false Expected SameValue(«1», «false») to be true (Testing with Float64Array.)'
     940test/built-ins/TypedArrayConstructors/internals/Set/key-is-minus-zero.js:
     941  default: 'Test262Error: Reflect.set(sample, "-0", 1) must return false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
     942  strict mode: 'Test262Error: Reflect.set(sample, "-0", 1) must return false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
     943test/built-ins/TypedArrayConstructors/internals/Set/key-is-not-integer.js:
     944  default: 'Test262Error: Reflect.set(sample, "1.1", 1) must return false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
     945  strict mode: 'Test262Error: Reflect.set(sample, "1.1", 1) must return false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
     946test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds.js:
     947  default: 'Test262Error: Reflect.set(sample, "-1", 1) must return false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
     948  strict mode: 'Test262Error: Reflect.set(sample, "-1", 1) must return false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
    937949test/built-ins/TypedArrayConstructors/internals/Set/tonumber-value-detached-buffer.js:
    938   default: 'Test262Error: Expected SameValue(«undefined», «false») to be true (Testing with Float64Array.)'
    939   strict mode: 'Test262Error: Expected SameValue(«undefined», «false») to be true (Testing with Float64Array.)'
     950  default: 'Test262Error: Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
     951  strict mode: 'Test262Error: Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
    940952test/intl402/DateTimeFormat/prototype/formatRange/en-US.js:
    941953  default: 'Test262Error: Expected SameValue(«1/3/2019 – 1/5/2019», «1/3/2019 – 1/5/2019») to be true'
    … …  
    25392551  default: 'Test262Error: Expected a SyntaxError but got a ReferenceError'
    25402552  strict mode: 'Test262Error: Expected a SyntaxError but got a ReferenceError'
    2541 test/language/statements/class/elements/privatefieldget-primitive-receiver.js:
    2542   default: 'Bad exit code: 5'
    2543   strict mode: 'Bad exit code: 5'
    2544 test/language/statements/class/elements/privatefieldput-primitive-receiver.js:
    2545   default: 'Bad exit code: 5'
    2546   strict mode: 'Bad exit code: 5'
    25472553test/language/statements/class/gen-method-static/forbidden-ext/b2/cls-decl-gen-meth-static-forbidden-ext-indirect-access-own-prop-caller-get.js:
    25482554  default: 'TypeError: Function.caller used to retrieve generator body'

  • trunk/Source/JavaScriptCore/ChangeLog

    r269660 r269670  
     12020-11-10  Ross Kirsling  <ross.kirsling@sony.com>
     2
     3        Align %TypedArray% behavior with recent spec adjustments
     4        https://bugs.webkit.org/show_bug.cgi?id=218776
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        The recent spec changes for typed arrays with detached buffers had certain ripple effects,
     9        namely the following two PRs which will be presented in next week's TC39 meeting.
     10        Since no controversy is expected, this patch addresses them now, though test262 adjustments are forthcoming.
     11
     12        1. https://github.com/tc39/ecma262/pull/2210
     13           It is correct that `ta[i] = n` doesn't throw when `ta` has a detached buffer or `i` is otherwise OOB,
     14           but by not throwing, Reflect.set(ta, i, n) is obliged to return true.
     15
     16        2. https://github.com/tc39/ecma262/pull/2221
     17           Until now, %TypedArray%.prototype.{includes, indexOf, join, lastIndexOf} lacked a rigorous specification;
     18           in particular, each has a parameter that may detach the buffer upon valueOf or toString, and the expected
     19           behavior was not made clear. It seems most sensible to do what the corresponding Array methods do upon
     20           `array.length = 0`: make use of the cached length but don't access indices, such that indexOf/lastIndexOf
     21           return -1 while includes/join act as if the elements were all `undefined`.
     22
     23        * runtime/JSGenericTypedArrayViewInlines.h:
     24        (JSC::JSGenericTypedArrayView<Adaptor>::put):
     25        (JSC::JSGenericTypedArrayView<Adaptor>::defineOwnProperty):
     26        (JSC::JSGenericTypedArrayView<Adaptor>::putByIndex):
     27        * runtime/JSGenericTypedArrayViewPrototypeFunctions.h:
     28        (JSC::genericTypedArrayViewProtoFuncIncludes):
     29        (JSC::genericTypedArrayViewProtoFuncIndexOf):
     30        (JSC::genericTypedArrayViewProtoFuncJoin):
     31        (JSC::genericTypedArrayViewProtoFuncLastIndexOf):
     32
    1332020-11-10  John Wilander  <wilander@apple.com>
    234

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

    r269343 r269670  
    366366        // Cases like '-0', '1.1', etc. are still obliged to give the RHS a chance to throw.
    367367        toNativeFromValue<Adaptor>(globalObject, value);
    368         return false;
     368        return true;
    369369    }
    370370
    … …  
    404404
    405405        if (descriptor.value())
    406             RELEASE_AND_RETURN(scope, thisObject->putByIndex(thisObject, globalObject, index.value(), descriptor.value(), shouldThrow));
     406            RELEASE_AND_RETURN(scope, thisObject->setIndex(globalObject, index.value(), descriptor.value()));
    407407
    408408        return true;
    … …  
    450450{
    451451    JSGenericTypedArrayView* thisObject = jsCast<JSGenericTypedArrayView*>(cell);
    452     return thisObject->setIndex(globalObject, propertyName, value);
     452    thisObject->setIndex(globalObject, propertyName, value);
     453    return true;
    453454}
    454455

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

    r269531 r269670  
    199199
    200200    if (thisObject->isDetached())
    201         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
     201        return JSValue::encode(jsBoolean(valueToFind.isUndefined()));
    202202
    203203    typename ViewClass::ElementType* array = thisObject->typedVector();
    … …  
    244244
    245245    if (thisObject->isDetached())
    246         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
     246        return JSValue::encode(jsNumber(-1));
    247247
    248248    typename ViewClass::ElementType* array = thisObject->typedVector();
    … …  
    270270        return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
    271271
    272     // 22.2.3.14
     272    unsigned length = thisObject->length();
    273273    auto joinWithSeparator = [&] (StringView separator) -> EncodedJSValue {
    274         ViewClass* thisObject = jsCast<ViewClass*>(callFrame->thisValue());
    275         unsigned length = thisObject->length();
    276 
    277274        JSStringJoiner joiner(globalObject, separator, length);
    278275        RETURN_IF_EXCEPTION(scope, encodedJSValue());
    279         for (unsigned i = 0; i < length; i++) {
    280             joiner.append(globalObject, thisObject->getIndexQuickly(i));
    281             RETURN_IF_EXCEPTION(scope, encodedJSValue());
     276        if (!thisObject->isDetached()) {
     277            for (unsigned i = 0; i < length; i++) {
     278                joiner.append(globalObject, thisObject->getIndexQuickly(i));
     279                RETURN_IF_EXCEPTION(scope, encodedJSValue());
     280            }
     281        } else {
     282            for (unsigned i = 0; i < length; i++)
     283                joiner.appendEmptyString();
    282284        }
    283285        RELEASE_AND_RETURN(scope, JSValue::encode(joiner.join(globalObject)));
    … …  
    293295    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    294296
    295     if (thisObject->isDetached())
    296         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
    297297    auto viewWithString = separatorString->viewWithUnderlyingString(globalObject);
    298298    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    … …  
    332332
    333333    if (thisObject->isDetached())
    334         return throwVMTypeError(globalObject, scope, typedArrayBufferHasBeenDetachedErrorMessage);
     334        return JSValue::encode(jsNumber(-1));
    335335
    336336    auto targetOption = ViewClass::toAdaptorNativeFromValueWithoutCoercion(valueToFind);
Note: See TracChangeset for help on using the changeset viewer.