Closed
Bug 593556
Opened 14 years ago
Closed 14 years ago
JM: variable from outer scope is not seen in "with" block
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: jruderman, Assigned: dvander)
References
Details
(Keywords: regression, testcase, Whiteboard: fixed-in-tracemonkey[suspect-regress-ts_cold_generated_med][suspect-regress-dromaeo_css][suspect-regress-dromaeo_jslib])
Attachments
(1 file)
(deleted),
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
(function () { var c = 3; (function () { with({}) { print(c); } })(); })(); Interpreter: 3 Method JIT: undefined The first bad revision is: changeset: 52276:9852618873d9 user: David Anderson <danderson@mozilla.com> date: Mon Jul 12 15:00:58 2010 -0700 summary: [JAEGER] Remove stores from OOL transitions when not needed (bug 573083).
![]() |
||
Updated•14 years ago
|
blocking2.0: --- → ?
![]() |
Assignee | |
Comment 1•14 years ago
|
||
This may be folded into bug 585524, which is closely related.
Updated•14 years ago
|
Assignee: general → dvander
Updated•14 years ago
|
blocking2.0: ? → betaN+
![]() |
Assignee | |
Comment 2•14 years ago
|
||
Most of the diff is unindenting. This patch makes primaryExpr() build use-def information for names inside |with| statements, however, it makes sure to mark the names as deoptimized. NewBindingNode no longer steals an existing pn if it's inside a |with|. BindNameToSlot now makes sure not to emit GETGNAME for deoptimized definitions. (GETGNAME says that the name is either on the global object or is a ReferenceError.)
Attachment #473807 -
Flags: review?(brendan)
Updated•14 years ago
|
Comment 3•14 years ago
|
||
Comment on attachment 473807 [details] [diff] [review] fix attempt >+ bool inWith = stmt && stmt->type == STMT_WITH; No need for this bool, just test (stmt && stmt->type == STMT_WITH) later where you make single use of it: >+ if (inWith) >+ pn->pn_dflags = PND_DEOPTIMIZED; But clobbering dflags is not a good idea if something precomputed other flags. Why not wait a bit more for where dflags is |='ed: >+ >+ JS_ASSERT(dn->pn_defn); >+ LinkUseToDef(pn, dn, tc); >+ >+ /* Here we handle the backward function reference case. */ >+ if (tokenStream.peekToken() != TOK_LP) >+ dn->pn_dflags |= PND_FUNARG; >+ >+ pn->pn_dflags |= (dn->pn_dflags & PND_FUNARG); and do the PND_DEOPTIMIZED |= here to keep all such together? Looks good otherwise -- does it fix the testcases with 'with' and 'function' sub-statements? /be
Attachment #473807 -
Flags: review?(brendan) → review+
![]() |
Assignee | |
Comment 4•14 years ago
|
||
That clobbering was a typo. Thanks for catching. No, it does not fix the other bugs on file. Those need more changes to BindVarOrConst, I think.
![]() |
Assignee | |
Comment 5•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/eba54c4edd6f
Whiteboard: fixed-in-tracemonkey
Comment 6•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/eba54c4edd6f
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Whiteboard: fixed-in-tracemonkey → fixed-in-tracemonkey[suspect-regress-ts_cold_generated_med]
Comment 7•14 years ago
|
||
A changeset from this bug was associated with a XP Ts, Cold MED Dirty Profile regression on Firefox. boo-urns :( Previous: avg 420.356 stddev 3.679 of 30 runs up to a9d1ad0bc386 New : avg 430.832 stddev 1.905 of 5 runs since a60414d076b5 Change : +10.475 (2.49% / z=2.847) Graph : http://mzl.la/bZFaB3 The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-ts_cold_generated_med] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.
Updated•14 years ago
|
Whiteboard: fixed-in-tracemonkey[suspect-regress-ts_cold_generated_med] → fixed-in-tracemonkey[suspect-regress-ts_cold_generated_med][suspect-regress-dromaeo_css]
Comment 8•14 years ago
|
||
A changeset from this bug was associated with a Win7 Dromaeo (CSS) regression on Firefox. boo-urns :( Previous: avg 2014.419 stddev 40.480 of 30 runs up to a9d1ad0bc386 New : avg 1901.610 stddev 12.432 of 5 runs since a60414d076b5 Change : -112.809 (-5.6% / z=2.787) Graph : http://mzl.la/9gFu4n The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-dromaeo_css] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.
Updated•14 years ago
|
Whiteboard: fixed-in-tracemonkey[suspect-regress-ts_cold_generated_med][suspect-regress-dromaeo_css] → fixed-in-tracemonkey[suspect-regress-ts_cold_generated_med][suspect-regress-dromaeo_css][suspect-regress-dromaeo_jslib]
Comment 9•14 years ago
|
||
A changeset from this bug was associated with a Win7 Dromaeo (jslib) regression on Firefox. boo-urns :( Previous: avg 127.610 stddev 4.222 of 30 runs up to a9d1ad0bc386 New : avg 116.384 stddev 0.751 of 5 runs since a60414d076b5 Change : -11.226 (-8.8% / z=2.659) Graph : http://mzl.la/bZu5UP The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-dromaeo_jslib] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.
Comment 10•14 years ago
|
||
A changeset from this bug was associated with a XP Dromaeo (CSS) regression on Firefox. boo-urns :( Previous: avg 2045.275 stddev 49.676 of 30 runs up to a9d1ad0bc386 New : avg 1936.120 stddev 13.937 of 5 runs since a60414d076b5 Change : -109.155 (-5.34% / z=2.197) Graph : http://mzl.la/b0dlou The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-dromaeo_css] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.
Comment 11•14 years ago
|
||
A changeset from this bug was associated with a XP Dromaeo (jslib) regression on Firefox. boo-urns :( Previous: avg 129.703 stddev 4.099 of 30 runs up to a9d1ad0bc386 New : avg 117.954 stddev 0.660 of 5 runs since a60414d076b5 Change : -11.749 (-9.06% / z=2.866) Graph : http://mzl.la/avZij4 The regression occurred from changesets in the following range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5 The tag [suspect-regress-dromaeo_jslib] has been added to the status whiteboard; please remove it only once you have confirmed this bug is not the cause of the regression.
You need to log in
before you can comment on or make changes to this bug.
Description
•