7703 - Rhino: variable in loop, interpreted mode

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Norris Boyd]

Subject:
             [Rhino] defining variables in loops
        Date:
             Sun, 06 Jun 1999 18:21:39 -0700
       From:
             Patrick Beard <[e-mail]>
 Organization:
             Netscape Communications
 Newsgroups:
             netscape.public.mozilla.jseng




I have the following function:

types = [];
function inspect(object)
{
   for (prop in object) {
      var x = object[prop];
      types[types.length] = (typeof x);
   }
}

If I define the following object:

var o = {a: 1, b: 2};

And then call this function with o, the types array looks like this:

["undefined", "undefined"]

However, if I rewrite inspect like so:

function inspect(object)
{
   for (prop in object) {
      types[types.length] = (typeof object[prop]);
   }
}

then types is what I expect:

["number", "number"]

So my question is, is it legal in JavaScript to define a variable in the
body of a loop? If so, then we evidently have a recently introduced bug
in Rhino, because this used to work.

--
// Patrick C. Beard
// Netscape Communications

Comment 1

[Norris Boyd]

Hmm. I can't reproduce this. Can you, roger?

Comment 2

[Patrick C. Beard]

Try with -version 120, that seems to be the key. Or, after it succeeds, evaluate
version(120) and it should fail.

Comment 3

[rogerl (gone)]

typeof <var> wasn't working for activation frame vars, the function had a frame
since 'length' was referenced in a version 120 context. Checked in fix.

Comment 4

[cbegle]

i'm now crashing running this test.  the stack trace just loops beetween calls
to js_Interpret and js_Invoke:
js_Interpret(JSContext * 0x004114f0, long * 0x00033584) line 1057 + 3 bytes
js_Invoke(JSContext * 0x004114f0, unsigned int 0, unsigned int 0) line 670 + 13
bytes

[...]

it's crashing here with the error on that line expression cannot be evaluated

    /*
     * Allocate operand and pc stack slots for the script's worst-case depth.
     */
    depth = (ptrdiff_t)script->depth;
->    newsp = js_AllocStack(cx, (uintN)(2 * depth), &mark);
    if (!newsp) {
	ok = JS_FALSE;
	sp = NULL;
	goto out;
    }

Comment 5

[cbegle]

the test will soon be checked in as
mozilla/js/tests/js1_2/regress/regress-7703.js

i ran it in rhino to see if it would come up with any kind of interesting
error, and got a StackOverflowException.

Comment 6

[cbegle]

okay, i fixed the test. it shouldn't crash now. sorry about that.

Comment 7

[cbegle]

okay.  test updated. it now succeeds.  --cmb.