E4X literals with embedded expressions unsufficiently constant-folded

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
JavaScript Engine
P3
normal
VERIFIED FIXED
12 years ago
11 years ago

People

(Reporter: brendan, Assigned: mrbkap)

Tracking

({verified1.8.0.2, verified1.8.1})

Trunk
mozilla1.9alpha1
verified1.8.0.2, verified1.8.1
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tcn-dl])

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
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)

Updated

12 years ago
Assignee: general → mrbkap
Priority: -- → P3
(Assignee)

Comment 1

12 years ago
*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
(Assignee)

Comment 2

12 years ago
Created attachment 209507 [details] [diff] [review]
Bad typo, bad!
Attachment #209507 - Flags: review?(brendan)
(Assignee)

Comment 3

12 years ago
Comment on attachment 209507 [details] [diff] [review]
Bad typo, bad!

shaver says r=him.
Attachment #209507 - Flags: review?(brendan) → review+
(Assignee)

Comment 4

12 years ago
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 5

12 years ago
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)
(Reporter)

Comment 6

12 years ago
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+

Updated

11 years ago
Flags: testcase-
(Assignee)

Comment 7

11 years ago
Fix checked into the 1.8 branches.
Keywords: fixed1.8.0.2, fixed1.8.1
Please expicitely state expected output for the testcase in comment 0.  Any testing guidance would also be appreciated
Whiteboard: [tcn-dl]
(Reporter)

Comment 9

11 years ago
(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

11 years ago
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
Keywords: fixed1.8.0.2, fixed1.8.1 → verified1.8.0.2, verified1.8.1
You need to log in before you can comment on or make changes to this bug.