Last Comment Bug 323979 - E4X literals with embedded expressions unsufficiently constant-folded
: E4X literals with embedded expressions unsufficiently constant-folded
Status: VERIFIED FIXED
[tcn-dl]
: verified1.8.0.2, verified1.8.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P3 normal (vote)
: mozilla1.9alpha1
Assigned To: Blake Kaplan (:mrbkap) (please use needinfo!)
:
Mentors:
Depends on:
Blocks: 318922
  Show dependency treegraph
 
Reported: 2006-01-18 22:36 PST by Brendan Eich [:brendan]
Modified: 2006-03-02 12:55 PST (History)
7 users (show)
bob: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Bad typo, bad! (1.05 KB, patch)
2006-01-24 14:33 PST, Blake Kaplan (:mrbkap) (please use needinfo!)
mrbkap: review+
brendan: approval‑branch‑1.8.1+
brendan: approval1.8.0.2+
Details | Diff | Review

Description Brendan Eich [:brendan] 2006-01-18 22:36:32 PST
js> function f(){var x = <x>{<y/>}</x>;return x}
js> f
function f() {
    var x = <x>{<y/>}</x>;
    return x;
}
js> dis(f)
main:
00000:  startxml
00001:  string "<"
00004:  string "x"
00007:  add
00008:  string ">"
00011:  add
00012:  startxmlexpr
00013:  startxml
00014:  string "<"
00017:  string "y"
00020:  add
00021:  string "/>"
00024:  add
00025:  toxml
00026:  xmleltexpr
00027:  add
00028:  startxml
00029:  string "</"
00032:  string "x"
00035:  add
00036:  string ">"
00039:  add
00040:  add
00041:  toxml
00042:  setvar 0
00045:  pop
00046:  getvar 0
00049:  return
00050:  stop
 
Source notes:
  0:    42 [  42] xdelta
  1:    42 [   0] var

/be
Comment 1 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-01-24 14:29:48 PST
*sigh*, this used to work much better until bug 318922 introduced a typo. I should have noticed this when reviewing.
Comment 2 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-01-24 14:33:30 PST
Created attachment 209507 [details] [diff] [review]
Bad typo, bad!
Comment 3 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-01-24 16:22:14 PST
Comment on attachment 209507 [details] [diff] [review]
Bad typo, bad!

shaver says r=him.
Comment 4 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-01-24 16:25:14 PST
Fix checked into trunk.
Comment 5 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-01-24 16:27:26 PST
Comment on attachment 209507 [details] [diff] [review]
Bad typo, bad!

I don't know if this is truely fodder for 1.8.0.2, but it seems like a no-brainer for Firefox 2.
Comment 6 Brendan Eich [:brendan] 2006-01-31 12:48:04 PST
Comment on attachment 209507 [details] [diff] [review]
Bad typo, bad!

This is a no-brainer.

/be
Comment 7 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-02-23 15:40:02 PST
Fix checked into the 1.8 branches.
Comment 8 Dave Liebreich [:davel] 2006-03-02 11:42:41 PST
Please expicitely state expected output for the testcase in comment 0.  Any testing guidance would also be appreciated
Comment 9 Brendan Eich [:brendan] 2006-03-02 12:38:50 PST
(In reply to comment #8)
> Please expicitely state expected output for the testcase in comment 0.

Run the same input with a fixed js shell and see for yourself.

> Any testing guidance would also be appreciated

One thought is that we don't want to insist on every last detail of bytecode and disassembly format.  We would like to know that the adjacent XML lexemes were in fact concatenated by the JS compiler as much as possible.  The best way to do this would be to have a stable AST format we can diff.  That's a hefty project, subject for a followup bug, and likely to happen for JS2 / ECMAScript Edition 4 work in SpiderMonkey.  It's not going to happen just to help verify this bug.

/be
Comment 10 Bob Clary [:bc:] 2006-03-02 12:55:54 PST
v winxp

C:\work\mozilla\builds\ff>trunk\mozilla\js\src\WINNT5.1_DBG.OBJ\js -x
js> function f(){var x = <x>{<y/>}</x>;return x}
js> f
function f() {
    var x = <x>{<y/>}</x>;
    return x;
}
js> dis(f)
main:
00000:  startxml
00001:  string "<x>"
00004:  startxmlexpr
00005:  startxml
00006:  string "<y/>"
00009:  toxml
00010:  xmleltexpr
00011:  add
00012:  string "</x>"
00015:  add
00016:  toxml
00017:  setvar 0
00020:  pop
00021:  getvar 0
00024:  return
00025:  stop

Source notes:
  0:    17 [  17] xdelta
  1:    17 [   0] var
js>

C:\work\mozilla\builds\ff>1.5\mozilla\js\src\WINNT5.1_DBG.OBJ\js -x
js> function f(){var x = <x>{<y/>}</x>;return x}
js> f
function f() {
    var x = <x>{<y/>}</x>;
    return x;
}
js> dis(f)
main:
00000:  startxml
00001:  string "<x>"
00004:  startxmlexpr
00005:  startxml
00006:  string "<y/>"
00009:  toxml
00010:  xmleltexpr
00011:  add
00012:  string "</x>"
00015:  add
00016:  toxml
00017:  setvar 0
00020:  pop
00021:  getvar 0
00024:  return

Source notes:
  0:    17 [  17] xdelta
  1:    17 [   0] var

C:\work\mozilla\builds\ff>1.5.0\mozilla\js\src\WINNT5.1_DBG.OBJ\js -x
js> function f(){var x = <x>{<y/>}</x>;return x}
js> f
function f() {
    var x = <x>{<y/>}</x>;
    return x;
}
js> dis(f)
main:
00000:  startxml
00001:  string "<x>"
00004:  startxmlexpr
00005:  startxml
00006:  string "<y/>"
00009:  toxml
00010:  xmleltexpr
00011:  add
00012:  string "</x>"
00015:  add
00016:  toxml
00017:  setvar 0
00020:  pop
00021:  getvar 0
00024:  return

Source notes:
  0:    17 [  17] xdelta
  1:    17 [   0] var
js>

Note You need to log in before you can comment on or make changes to this bug.