Closed Bug 323979 Opened 18 years ago Closed 18 years ago

E4X literals with embedded expressions unsufficiently constant-folded

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha1

People

(Reporter: brendan, Assigned: mrbkap)

References

Details

(Keywords: verified1.8.0.2, verified1.8.1, Whiteboard: [tcn-dl])

Attachments

(1 file)

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
Assignee: general → mrbkap
Priority: -- → P3
*sigh*, this used to work much better until bug 318922 introduced a typo. I should have noticed this when reviewing.
Blocks: 318922
Status: NEW → ASSIGNED
Attached patch Bad typo, bad!Splinter Review
Attachment #209507 - Flags: review?(brendan)
Comment on attachment 209507 [details] [diff] [review]
Bad typo, bad!

shaver says r=him.
Attachment #209507 - Flags: review?(brendan) → review+
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
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.
Attachment #209507 - Flags: approval1.8.1?
Attachment #209507 - Flags: approval1.8.0.2?
Attachment #209507 - Flags: approval1.8.1? → branch-1.8.1?(brendan)
Comment on attachment 209507 [details] [diff] [review]
Bad typo, bad!

This is a no-brainer.

/be
Attachment #209507 - Flags: branch-1.8.1?(brendan)
Attachment #209507 - Flags: branch-1.8.1+
Attachment #209507 - Flags: approval1.8.0.2?
Attachment #209507 - Flags: approval1.8.0.2+
Flags: testcase-
Fix checked into the 1.8 branches.
Please expicitely state expected output for the testcase in comment 0.  Any testing guidance would also be appreciated
Whiteboard: [tcn-dl]
(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
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>
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.