Closed Bug 585524 Opened 9 years ago Closed 9 years ago

JM: Incorrect prediction of free variables when using WITH

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

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)

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.
Attached patch test caseSplinter Review
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.
Attached patch fix...? (obsolete) — Splinter Review
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)
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-
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
No longer blocks: 593556
Depends on: 593556
blocking2.0: --- → final+
Attached patch quick fixSplinter Review
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 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+
http://hg.mozilla.org/mozilla-central/rev/eedeb03352c7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.