Closed
Bug 585524
Opened 14 years ago
Closed 14 years ago
JM: Incorrect prediction of free variables when using WITH
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
| Tracking | Status | |
|---|---|---|
| blocking2.0 | --- | final+ |
People
(Reporter: dvander, Assigned: dvander)
References
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(2 files, 1 obsolete file)
|
(deleted),
patch
|
Details | Diff | Splinter Review | |
|
(deleted),
patch
|
dmandelin
:
review+
|
Details | Diff | Splinter Review |
closures/t036.js has something like this:
function g() {
with (o) {
var f = function() { appendToActual(x); }
}
f();
}
We're emitting GETGNAME for |f|, which is wrong. It must be NAME.
|
Assignee | |
Comment 1•14 years ago
|
||
If I remove the |var|, or add a |var f| outside of the |with|, the test passes. The problem is that BindVarOrConst bails out immediately if there's a WITH statement.
|
|
Assignee | |
Comment 2•14 years ago
|
||
I won't lie, this patch makes me feel really dirty. If there's a "var" inside a "with" block, it creates a hoisted definition & local variable. Passes reftests and the attached test case.
Attachment #464273 -
Flags: review?(brendan)
|
|
Assignee | |
Comment 3•14 years ago
|
||
Comment on attachment 464273 [details] [diff] [review]
fix...?
Fix is bogus per discussion in IRC - turns |const| declarations into |var|.
Attachment #464273 -
Flags: review?(brendan) → review-
|
||
Comment 4•14 years ago
|
||
Bug 593556 has a patch now, so shouldn't it block this bug rather than vice versa? Restore the blocker if I am wrong.
/be
|
|
Assignee | |
Updated•14 years ago
|
blocking2.0: --- → final+
|
|
Assignee | |
Comment 5•14 years ago
|
||
Use the same fix as for bug 584603, where def-use chains are broken in a similar way.
Attachment #464273 -
Attachment is obsolete: true
Attachment #492715 -
Flags: review?(dmandelin)
|
||
Comment 6•14 years ago
|
||
Comment on attachment 492715 [details] [diff] [review]
quick fix
|TCF_FUN_SKIP_GNAME_ANALYSIS| is too operational. Something that describes the semantics/traits of the function is better, e.g., |TCF_FUN_HAS_IMPLICIT_NAMES|.
r+ with a semantic-level name.
Attachment #492715 -
Flags: review?(dmandelin) → review+
|
|
Assignee | |
Comment 7•14 years ago
|
||
Whiteboard: fixed-in-tracemonkey
|
||
Comment 8•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•