If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

TM: trace more Date() functions

RESOLVED DUPLICATE of bug 463238

Status

()

Core
JavaScript Engine
RESOLVED DUPLICATE of bug 463238
9 years ago
9 years ago

People

(Reporter: gal, Assigned: gal)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

9 years ago
Created attachment 362365 [details] [diff] [review]
patch
Assignee: general → gal
(Assignee)

Updated

9 years ago
Blocks: 458016
(Assignee)

Comment 2

9 years ago
Created attachment 362370 [details] [diff] [review]
v2

Add setTime and the Date(double) constructor.
Attachment #362365 - Attachment is obsolete: true
(Assignee)

Comment 3

9 years ago
Created attachment 362371 [details] [diff] [review]
v2, with whitespace fixes
Attachment #362370 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Attachment #362371 - Flags: review?(jorendorff)
Comment on attachment 362371 [details] [diff] [review]
v2, with whitespace fixes

In jsbuiltins.h:
> -#define _JS_CTYPE_INT32_RETRY       _JS_CTYPE(int32,                  _JS_I32, --, --, FAIL_NEG)
> +#define _JS_CTYPE_INT32_RETRY_NEG   _JS_CTYPE(int32,                  _JS_I32, --, --, FAIL_NEG)
>  #define _JS_CTYPE_UINT32            _JS_CTYPE(uint32,                 _JS_I32, --, --, INFALLIBLE)
>  #define _JS_CTYPE_DOUBLE            _JS_CTYPE(jsdouble,               _JS_F64, "","d", INFALLIBLE)
> +#define _JS_CTYPE_DOUBLE_RETRY_NEG  _JS_CTYPE(jsdouble,               _JS_F64, --, --, FAIL_NEG)

Please also update the comment at line 149.

> +#define DEFINE_TRACER_DATE_GETTER(name, tfunc, cfunc)

tfunc and cfunc are not used in this macro.  They should be deleted.

> -    if (vp && !JS_InstanceOf(cx, obj, &js_DateClass, vp + 2))
> +    if (vp && !JS_InstanceOf(cx, obj, &js_DateClass, vp ? vp + 2 : NULL))

This change is unnecessary given the LHS of the &&, and removing vp here would be wrong because if vp is null we really want to assert, not return JS_FALSE without having reported an error.

r=me with those changes.
Attachment #362371 - Flags: review?(jorendorff) → review+
It is kind of subtle what's going on here in the case that `this` isn't a Date on trace (which I think can happen if `this` isn't a Date at record time).  We end up calling JS_InstanceOf() with NULL vp, which will return JS_FALSE, so we'll bail out and retry the call from the interpreter.

I think it works but I'd feel better with a test.
(Assignee)

Comment 6

9 years ago
Comment on attachment 362371 [details] [diff] [review]
v2, with whitespace fixes

Also test what happens if we do apply(foo, getYear) and foo is not a date object.
(Assignee)

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 463238
You need to log in before you can comment on or make changes to this bug.