299641 - the "LeftHandSideExpression" of a 'for .. in' statement is executed too often.

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[mozilla]

User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.4; Linux) KHTML/3.4.0 (like Gecko)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.6) 

According to section 12.6.4 of the ECMAscript spec the LeftHandSideExpression 
should be executed as often as there are properties in the object (returned by 
Expression). [ the syntax is: 'for ( LeftHandSideExpression in Expression ) 
Statement']. 
Firefox executes it one more time (probably first executing the LHSExpr before 
verifying, that there are still elements). 

Reproducible: Always

Comment 1

[mozilla]

Attachment: test-case.

"infun" is printed 3 times, eventhough there are only 2 properties
in o. -> there should be only 2 "infun"s.

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 188202 [details]
test-case.

Wrong testcase?

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: correct js shell testcase

This should print 0, not 1.

/be

Comment 4

[mozilla]

sorry. didn't get your first comment (probably missed it). 
your test-case illustrates the point. thx. 

Comment 5

[Brendan Eich [:brendan]]

Attachment: proposed fix

This is probably what you were after, but it required some knowledge of when to
fake out a subroutine with an auto-storage-class tree node ;-).

/be

Comment 6

[Brendan Eich [:brendan]]

Trying to use ASSIGNED to mean "have patch"....

/be

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 189137 [details] [diff] [review]
proposed fix

>+             * Set left and right so pn appears to be a TOK_LB node, instead
>+             * the TOK_DOT node it is (see the TOK_FOR case in js_EmitTree).

You're missing an 'of' at the end of the first line. r=me with that fixed.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 189137 [details] [diff] [review]
proposed fix

Another spot-fix for an ECMA conformance bug.

/be

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 189137 [details] [diff] [review]
proposed fix

Another spot-fix for an ECMA conformance bug.

/be

Comment 10

[Brendan Eich [:brendan]]

Blug, didn't mean to double submit (or to sr my own patch).

Fixed.

/be

Comment 11

[Bob Clary [:bc] (inactive)]

Checking in regress-299641.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-299641.js,v  <--  regress-299641.js
initial revision: 1.1

Comment 12

[Brendan Eich [:brendan]]

This was fixed well before 1.8 branched -- why does it need the fixed1.8 keyword?

/be

Comment 13

[Scott MacGregor]

one the queries was broken and we thought we were seeing some false positives
due to the missing keyword, but it turned out to be our query string. Didn't
realize that was going to inconvience you, sorry :)

Comment 14

[Brendan Eich [:brendan]]

No inconvenience, just trying to keep score!

/be

Comment 15

[Bob Clary [:bc] (inactive)]

verified fixed 1.8.x and trunk.