E4X: toString, toSource of a function containing E4X gives incorrect output

VERIFIED FIXED in mozilla1.9alpha1

Status

()

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

People

(Reporter: Biju, Assigned: mrbkap)

Tracking

({testcase, verified1.8.1})

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

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

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

Comment 1

12 years ago
Created attachment 206860 [details]
test cases e4x_toSourceErr.html

Updated

12 years ago
Keywords: testcase
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

12 years ago
/cvsroot/mozilla/js/tests/e4x/Regress/regress-321547.js,v  <--  regress-321547.js
Flags: testcase+
mrbkap, you game to take this?  An easy decompiler bug, and we know how much you love the decompiler! ;-)

/be
(Assignee)

Comment 5

12 years ago
Sure, I'll take this.
Assignee: general → mrbkap
Priority: -- → P3
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Whiteboard: [patch]
(Assignee)

Comment 6

12 years ago
Created attachment 215976 [details] [diff] [review]
patch v1

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 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

12 years ago
Created attachment 215979 [details] [diff] [review]
patch v2

Much better.
Attachment #215976 - Attachment is obsolete: true
Attachment #215979 - Flags: review?(brendan)
Attachment #215976 - Flags: review?(brendan)
Comment on attachment 215979 [details] [diff] [review]
patch v2

Yeah!

/be
Attachment #215979 - Flags: review?(brendan) → review+
(Assignee)

Comment 10

12 years ago
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
OS: Windows XP → All
Hardware: PC → All
Resolution: --- → FIXED

Comment 11

12 years ago
verified fixed 20060401 win/mac/linux
Status: RESOLVED → VERIFIED

Comment 12

12 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.