uneval turns double negation into decrement, and negation+decrement into syntax error

VERIFIED FIXED in mozilla1.8.1

Status

()

defect
P2
normal
VERIFIED FIXED
13 years ago
13 years ago

People

(Reporter: jruderman, Assigned: brendan)

Tracking

(Blocks 1 bug, {testcase, verified1.8.1})

Trunk
mozilla1.8.1
PowerPC
macOS
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [181approval pending])

Attachments

(1 attachment)

(Reporter)

Description

13 years ago
http://www.squarefree.com/shell/shell.html, Mac trunk nightly.

> function () { return - --x; }
function () { return ---x; }

> function () { return ---x; }
SyntaxError on line 1: syntax error

Either
(1) uneval needs to leave a space between the negation and decrement operators, or
(2) the tokenizer needs to tokenize --- in a way that doesn't lead to a syntax error.  

I'm guessing based on "The source text is scanned from left to right , repeatedly taking the longest possible sequence of characters as the next input element." in the Ecmascript spec that (2) would be incorrect, so (1) is probably the right fix.

And here's something that's even worse, because it changes the meaning of the function without causing a syntax error:

> function () { - - x }
function () { --x; }

The same things happen with unary + and increment.  (What is unary + for, anyway?)
(Reporter)

Comment 1

13 years ago
> (What is unary + for, anyway?)

It coerces its argument to be a number, at least.
(Assignee)

Comment 2

13 years ago
Also worth fixing in 1.8.1.

/be
Assignee: general → brendan
Priority: -- → P2
Target Milestone: --- → mozilla1.8.1
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

13 years ago
This is a super-safe and simple patch to fix a toString/eval round-trip bug.

/be
Attachment #234270 - Flags: review?(mrbkap)
Attachment #234270 - Flags: approval1.8.1?
Attachment #234270 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 4

13 years ago
Fixed on trunk.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 5

13 years ago
Checking in regress-346904.js;
/cvsroot/mozilla/js/tests/js1_5/Expressions/regress-346904.js,v  <--  regress-346904.js
initial revision: 1.1

note this assumes +++x is a syntax error however, IE6 thinks +++x is ok.
Flags: in-testsuite+
(Assignee)

Comment 6

13 years ago
(In reply to comment #5)
> note this assumes +++x is a syntax error however, IE6 thinks +++x is ok.

What value does it compute if x is 0 before-hand?  How about -1?

It's clearly violating ECMA-262 -- news at 11.

/be

Comment 7

13 years ago
(In reply to comment #6)

IE throws a runtime error that the number can't be assigned to:
x = 0
+++x
ReferenceError: Cannot assign to '[number: 0]'
(Assignee)

Comment 8

13 years ago
(In reply to comment #7)
> (In reply to comment #6)
> 
> IE throws a runtime error that the number can't be assigned to:
> x = 0
> +++x
> ReferenceError: Cannot assign to '[number: 0]'

Oh, I thought by "IE thinks +++x is ok" you meant no error was thrown at runtime or compile-time.  ECMA-262 allows such errors to be reported early, but they must be reported some time (see chapter 16).  So IE is ok, news at 11:30.

/be

Comment 9

13 years ago
verified fixed 1.9 20060818 windows/mac(ppc|tel)/linux
Status: RESOLVED → VERIFIED
Whiteboard: [181approval pending]
(Reporter)

Updated

13 years ago
Blocks: 349611

Comment 10

13 years ago
Comment on attachment 234270 [details] [diff] [review]
simple fix: make the decompiler token be "- " / "+ "

a=schrep for drivers - approving all [181approval pending] bugs now that the tree is open.
Attachment #234270 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 11

13 years ago
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8.1

Comment 12

13 years ago
verified fixed 1.8, 1.9 20060824 windows/mac*/linux
You need to log in before you can comment on or make changes to this bug.