Closed
Bug 321547
Opened 19 years ago
Closed 19 years ago
E4X: toString, toSource of a function containing E4X gives incorrect output
Categories
(Core :: JavaScript Engine, defect, P3)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.9alpha1
People
(Reporter: BijuMailList, Assigned: mrbkap)
Details
(Keywords: testcase, verified1.8.1, Whiteboard: [patch])
Attachments
(2 files, 1 obsolete file)
3.54 KB,
text/html
|
Details | |
5.51 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051219 Firefox/1.6a1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051219 Firefox/1.6a1 toString, toSource of a function containing E4X syintax ".." gives incorrect output by putting the next nodename in "" Reproducible: Always Steps to Reproduce: do.. function a(){ var x=<a><b><c>value c</c></b></a>; return x..c; } alert(a.toString()); Actual Results: function a() { var x = <a><b><c>value c</c></b></a>; return x.."c"; } Expected Results: function a() { var x = <a><b><c>value c</c></b></a>; return x..c; }
Comment 2•19 years ago
|
||
The .. operator added by E4X is not like other binary operators, where each operand is evaluated before the operator is applied. Instead, it implicitly quotes its right operand. This asymmetry is a shame, because it makes .. unusable for a Perl-ish "range" (a..b for two integer-valued variables a and b) notation. The bytecode emitted for c on the right of .. in the testcase is JSOP_STRING, which reflects this implicit quoting, so of course SpiderMonkey decompiles the right operand in quotes. Fixable with a source note; SRC_LABEL will do. /be
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•19 years ago
|
||
/cvsroot/mozilla/js/tests/e4x/Regress/regress-321547.js,v <-- regress-321547.js
Flags: testcase+
Comment 4•19 years ago
|
||
mrbkap, you game to take this? An easy decompiler bug, and we know how much you love the decompiler! ;-) /be
Assignee | ||
Comment 5•19 years ago
|
||
Sure, I'll take this.
Assignee: general → mrbkap
Priority: -- → P3
Target Milestone: --- → mozilla1.9alpha
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [patch]
Assignee | ||
Comment 6•19 years ago
|
||
This seems to do the trick, though it seems mildly wasteful, since SRC_LABEL has arity 1, which is unneeded. Am I missing something?
Attachment #215976 -
Flags: review?(brendan)
Comment 7•19 years ago
|
||
Comment on attachment 215976 [details] [diff] [review] patch v1 > SRC_RESERVED0 = 10, /* reserved for future use */ Hmm, maybe we should claim this, and call it SRC_UNQUOTE. >+ JSBool ok, quoteString; No single-use variables, in general. > BEGIN_LITOPX_CASE(JSOP_STRING) >+ sn = js_GetSrcNote(jp->script, pc); >+ quoteString = !inXML && (!sn || SN_TYPE(sn) != SRC_LABEL); > rval = QuoteString(&ss->sprinter, ATOM_TO_STRING(atom), >- (jschar)(inXML ? 0 : '"')); >+ (jschar)(quoteString ? '"' : 0)); In particular, (jschar)((inXML || (sn && SN_TYPE(sn) == SRC_UNQUOTE)) ? 0 : '"')); fits in 80 columns, avoids logical negations (confusing and unsightly), and does not seem hard to read to me. /be
Assignee | ||
Comment 8•19 years ago
|
||
Much better.
Attachment #215976 -
Attachment is obsolete: true
Attachment #215979 -
Flags: review?(brendan)
Attachment #215976 -
Flags: review?(brendan)
Comment 9•19 years ago
|
||
Comment on attachment 215979 [details] [diff] [review] patch v2 Yeah! /be
Attachment #215979 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 10•19 years ago
|
||
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
OS: Windows XP → All
Hardware: PC → All
Resolution: --- → FIXED
Comment 12•18 years ago
|
||
fixed by Bug 336373 on the 1.8.1 branch. verified fixed 1.8.1 with windows/macppc/linux 20060707
Keywords: verified1.8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•