Biju Reporter
	Description • 19 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;
}

	Brendan Eich [:brendan]
	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

	Bob Clary [:bc] (inactive)
	Comment 3 • 19 years ago

/cvsroot/mozilla/js/tests/e4x/Regress/regress-321547.js,v  <--  regress-321547.js

	Brendan Eich [:brendan]
	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

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 5 • 19 years ago

Sure, I'll take this.

	Blake Kaplan (:mrbkap) (inactive) 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?

	Brendan Eich [: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

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 8 • 19 years ago

Much better.

	Brendan Eich [:brendan]
	Comment 9 • 19 years ago

Comment on attachment 215979 [details] [diff] [review]
patch v2

Yeah!

/be

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 10 • 19 years ago

Fix checked into trunk.

	Bob Clary [:bc] (inactive)
	Comment 11 • 19 years ago

verified fixed 20060401 win/mac/linux

	Bob Clary [:bc] (inactive)
	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