155285 - Array.join(undefined) should use ','

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[rogerl (gone)]

Try:
>a = [1,2]
>a.join(undefined)
1undefined2

Should be:
1,2

(ECMA 15.4.4.5, step 3)

Comment 1

[rogerl (gone)]

Test suite expects 'undefined':

    Testcase ecma/Array/15.4.4.3-1.js failed
    var TEST_ARRAY = new Array(null, void 0, true, false, 123, new Object(), new
Boolean(true) ); TEST_ARRAY.join(void 0) = ,,true,false,123,[object Object],true
FAILED! expected:
undefinedundefinedtrueundefinedfalseundefined123undefined[object
Object]undefinedtrue

Comment 2

[rogerl (gone)]

Attachment: Test for undefined value in argv[0]

Comment 3

[Steven Cole]

Comment on attachment 89953 [details] [diff] [review]
Test for undefined value in argv[0]

>-    if (argc == 0)
>+    if (argc == 0 || JSVAL_IS_VOID(argv[0]))

Shouldn't this just be 
  if (JSVAL_IS_VOID(argv[0]))
???

(Unspecified args with positions < function.length have value JSVAL_VOID in the
argv[] array, so that if argc is zero then argv[0] is JSVAL_VOID, since
Array.join.length is 1.)

--scole

Comment 4

[rogerl (gone)]

Attachment: Removed argc test

I'll buy that. Thanks.

Comment 5

[Phil Schwartau]

cc'ing reviewers; adding js1.5 keyword; adding as dependency to bug 149801 -

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 89973 [details] [diff] [review]
Removed argc test

sr=[e-mail], and I say scole already did a bang-up r= job.  Roger,
can you get this into the trunk and mail drivers asking for branch approval? 
Thanks,

/be

Comment 7

[rogerl (gone)]

Fix checked in to trunk. Mailing drivers (but I'm on vacation thru Friday so 
someone else will have to check it in if it gets approval).

Comment 8

[chris hofmann]

Comment on attachment 89973 [details] [diff] [review]
Removed argc test

a=chofmann for 1.0.1  add fixed1.0.1 keyword after checking in on the branch

Comment 9

[chris hofmann]

Comment on attachment 89973 [details] [diff] [review]
Removed argc test

a=chofmann for 1.0.1  add fixed1.0.1 keyword after checking in on the branch

Comment 10

[Brendan Eich [:brendan]]

I'll check in.

/be

Comment 11

[Brendan Eich [:brendan]]

Fixed on the branch, too.

/be

Comment 12

[Phil Schwartau]

Verified FIXED. Testcase corrected:

         mozilla/js/tests/ecma/Array/15.4.4.3-1.js


This now passes in the debug/optimized JS shells. It also passes
in the rhino/rhinoi shells, since Roger also made the fix there. 
Also fixed on MOZILLA_1_0_BRANCH; adding verified1.0.1 keyword -