Closed Bug 418638 Opened 14 years ago Closed 14 years ago

Bugs in JS shell functions scatter() and sleep()

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

Attachments

(1 file, 3 obsolete files)

To my shame I posted patch v11 in bug 404879 without testing the new snippet of code.  It's even more busted than v9 and v10, of course.

I also found some other minor bugs, poking around.  scatter() passes PR_LOCAL_THREAD to PR_CreateThread; that should be PR_GLOBAL_THREAD.  sleep() doesn't detect when it's passed 0 arguments.  It doesn't do anything meaningful when an argument that isn't a number.  It should throw in both cases.
Attached patch v1 (obsolete) — Splinter Review
On second thought, sleep(NaN) or sleep("string") will do sleep(0).  I would have it throw a RangeError or such, but there's no really good, direct way to do that.  Also JavaScript has a proud tradition of behaving badly in these cases.  Separate bug.
Assignee: general → jorendorff
Status: NEW → ASSIGNED
Attachment #304579 - Flags: review?(crowder)
Attached patch v2 (obsolete) — Splinter Review
Changes the NaN check to use a macro.
Attachment #304579 - Attachment is obsolete: true
Attachment #304582 - Flags: review?(crowder)
Attachment #304579 - Flags: review?(crowder)
Comment on attachment 304582 [details] [diff] [review]
v2

Worth speculating about whether JSDOUBLE_IS_NaN should be public; but for another bug.
Attachment #304582 - Flags: review?(crowder) → review+
Attached patch v3 (obsolete) — Splinter Review
This additionally fixes a stupid crash when n==0, that is, scatter([]).
Attachment #304582 - Attachment is obsolete: true
Attachment #304590 - Flags: review?(crowder)
Attached patch v3Splinter Review
How about the right patch.
Attachment #304590 - Attachment is obsolete: true
Attachment #304591 - Flags: review?(crowder)
Attachment #304590 - Flags: review?(crowder)
As we evolve toward JS2/ES4, ask yourself not what crazy loosy-goosey conversion would JS1 do, but what type annotations would you want on the function you are adding, and what built-in and few conversions might apply.

/be
Attachment #304591 - Flags: review?(crowder) → review+
js.c:3.200
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.