Closed
Bug 433628
Opened 17 years ago
Closed 17 years ago
using for(in) on a function argument yields "undefined" keys.
Categories
(Tamarin Graveyard :: Self-hosting compiler (ESC), defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: henri_torgemane, Assigned: lhansen)
References
Details
Attachments
(2 files)
30.39 KB,
patch
|
jodyer
:
review+
|
Details | Diff | Splinter Review |
106.33 KB,
application/octet-stream
|
Details |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14
Build Identifier:
Smallest testcase:
var obj={a:1,b:2};
function test(o) {
var s="";
for (var k in o) s+=k;
return s;
}
print(test(obj))
expected result is "ab", but actual result is "undefinedundefined".
Reproducible: Always
Reporter | ||
Comment 1•17 years ago
|
||
This occurs because ESC uses getlocal/setlocal to access function-scoped variables, but cgForInStmt() always uses its index as a named variable.
If the loop index variable is scoped outside of the function, things work:
var obj={a:1,b:2}
var loopIndex;
function test(o) {
var s="";
for (loopIndex in o) s+=loopIndex;
return s;
}
print(test(obj));
I have a broken fix, which involves commenting out every statement that mentions the variable "name" in cgForInStmt(), and instead calling cgSetProp(ctx, init); right before the call to cgStmt.
It's a fix because it resolves the loop index correctly.
It's broken because it only accepts the form "for (index in expr)". even "for (var index in expr)" breaks.
I suspect there's a neat function somewhere that would convert "init" into something cgSetProp can consume, but I don't know the codebase enough.
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → lhansen
Status: ASSIGNED → NEW
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•17 years ago
|
||
IMO for-in handling in the compiler is pretty much broken, needs a rewrite. Sigh.
Assignee | ||
Comment 4•17 years ago
|
||
Broad reengineering of for-in loops; this should handle most cases (it handles all the cases in this bug and in the dup) and should be set up for eg proper destructuring support when we get to that.
Also some other cleanup here, this fixes the abcencoder (heavy use of for-in there) and adds some editorializing comments to the ast file, where it also fixes some array-structural types.
Attachment #323662 -
Flags: review?(jodyer)
Assignee | ||
Comment 5•17 years ago
|
||
Complements the source patch in the previous attachment.
Comment on attachment 323662 [details] [diff] [review]
Patch
Agree with your comments regarding TempName and PropName. TempName should removed and PropName peeled to Name.
Attachment #323662 -
Flags: review?(jodyer) → review+
Assignee | ||
Comment 8•17 years ago
|
||
Pushed.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•