Closed Bug 478526 Opened 15 years ago Closed 15 years ago

TM: trace more Date() functions

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 463238

People

(Reporter: gal, Assigned: gal)

References

Details

Attachments

(1 file, 2 obsolete files)

      No description provided.
Attached patch patch (obsolete) — Splinter Review
Assignee: general → gal
Blocks: 458016
Attached patch v2 (obsolete) — Splinter Review
Add setTime and the Date(double) constructor.
Attachment #362365 - Attachment is obsolete: true
Attachment #362370 - Attachment is obsolete: true
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.
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.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: