585524 - JM: Incorrect prediction of free variables when using WITH

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

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.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: test case

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.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix...?

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.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 464273 [details] [diff] [review]
fix...?

Fix is bogus per discussion in IRC - turns |const| declarations into |var|.

Comment 4

[Brendan Eich [:brendan]]

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

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: quick fix

Use the same fix as for bug 584603, where def-use chains are broken in a similar way.

Comment 6

[David Mandelin [: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.

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/eedeb03352c7

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/eedeb03352c7