Closed Bug 418638 Opened 15 years ago Closed 15 years ago

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

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Version:
Other Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

Attachments

(1 file, 3 obsolete files)

v3
3.69 KB, patch
crowderbt
: review+
Details | Diff | Splinter Review 
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 v3 — — Splinter 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: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.