[Webkit-unassigned] [Bug 159385] ECMAScript 2016: %TypedArray%.prototype.includes implementation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=159385

--- Comment #21 from Caio Lima <ticaiolima at gmail.com> ---
Comment on attachment 282752
  --> https://bugs.webkit.org/attachment.cgi?id=282752
Patch

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

>> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h:216
>> +    if (!valueToFind.isNumber() && !valueToFind.isDouble())
> 
> Isn't "isNumber()" a superset of "isDouble()"?
> 
> valueToFind.isNumber() false implies valueToFind.isDouble() false.

You are right. I was considering isDouble because of NaN case, bit Ironically, NaN isNumber is true.

>> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h:222
>> +    typename ViewClass::ElementType target = ViewClass::toAdaptorNativeFromValue(exec, valueToFind);
> 
> I don't think that works for includes.
> 
> For example, Uint8ClampedAdaptor will clamp positive values to 255, that's not what we need here.
> IntegralTypedArrayAdaptor does ToInt32 on the input, that's not what we need either.
> 
> Your test should have extensive coverage of those crazy cases.
> 
> ----
> 
> What you probably need is a new Adaptor function that returns a value in the right type or an error.
> 
> For example, if you pass a double to an integer type array, it returns the integer if it fits in the array type or fail.
> E.g.:
>     -The double 256.0 works for int16, fails for int8
>     -The double 254.1 fails for all integers.

Nice point. I am working in that now.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160707/1bbed0e8/attachment-0001.html>