Bug 143865 – Implement `Object.is`

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Jordan Harband]

This function simply exposes `sameValue` to JS.

[Jordan Harband]

Created attachment 251121 [details]
Patch

[Jordan Harband]

Created attachment 251122 [details]
Patch

For PropertyDescriptor.cpp[0], why not just this:

if (std::isnan(x) && std::isnan(y))
    return true;

[0] https://bugs.webkit.org/attachment.cgi?id=251122&action=diff#a/Source/JavaScriptCore/runtime/PropertyDescriptor.cpp_sec1

[Build Bot]

Comment on attachment 251122 [details]
Patch

Attachment 251122 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4778729989472256

New failing tests:
js/Object-is.html

[Build Bot]

Created attachment 251124 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 251122 [details]
Patch

Attachment 251122 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5054959737372672

New failing tests:
js/Object-is.html

[Build Bot]

Created attachment 251125 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Jordan Harband]

Created attachment 251127 [details]
Patch

[Jordan Harband]

Comment on attachment 251124 [details]
Archive of layout-test-results from ews103 for mac-mavericks

my bsd

[Jordan Harband]

(In reply to comment #3)
> For PropertyDescriptor.cpp[0], why not just this:
> 
> if (std::isnan(x) && std::isnan(y))
>     return true;
> 
> [0]
> https://bugs.webkit.org/attachment.cgi?id=251122&action=diff#a/Source/
> JavaScriptCore/runtime/PropertyDescriptor.cpp_sec1

If one is NaN and the other's not, there's no purpose in casting to a number, it'd be false. The logic is more like `if (xNaN || yNaN) { return xNaN && yNaN }`

[Jordan Harband]

Created attachment 251131 [details]
Patch

[Darin Adler]

Comment on attachment 251131 [details]
Patch

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

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:588
> +    JSValue value1 = exec->argument(0);
> +    JSValue value2 = exec->argument(1);
> +    return JSValue::encode(jsBoolean(sameValue(exec, value1, value2)));

No reason to make these local variables, since there is no side effect here. This would read better as a one liner:

    return JSValue::encode(jsBoolean(sameValue(exec, exec->argument(0), exec->argument(1))));

[Jordan Harband]

(In reply to comment #12)
> Comment on attachment 251131 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251131&action=review
> 
> > Source/JavaScriptCore/runtime/ObjectConstructor.cpp:588
> > +    JSValue value1 = exec->argument(0);
> > +    JSValue value2 = exec->argument(1);
> > +    return JSValue::encode(jsBoolean(sameValue(exec, value1, value2)));
> 
> No reason to make these local variables, since there is no side effect here.
> This would read better as a one liner:
> 
>     return JSValue::encode(jsBoolean(sameValue(exec, exec->argument(0),
> exec->argument(1))));


I tend to prefer intermediate variables for readability and step-through debugging, but sounds good, I'll make this change right now

[Jordan Harband]

Created attachment 251140 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 251140 [details]
Patch

Clearing flags on attachment: 251140

Committed r183006: <http://trac.webkit.org/changeset/183006>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.