Closed Bug 346904 Opened 14 years ago Closed 14 years ago

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

Categories

(Core :: JavaScript Engine, defect, P2)

PowerPC
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla1.8.1

People

(Reporter: jruderman, Assigned: brendan)

Details

(Keywords: testcase, verified1.8.1, Whiteboard: [181approval pending])

Attachments

(1 file)

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?)
> (What is unary + for, anyway?)

It coerces its argument to be a number, at least.
Also worth fixing in 1.8.1.

/be
Assignee: general → brendan
Priority: -- → P2
Target Milestone: --- → mozilla1.8.1
Status: NEW → ASSIGNED
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+
Fixed on trunk.

/be
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
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+
(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
(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]'
(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
verified fixed 1.9 20060818 windows/mac(ppc|tel)/linux
Status: RESOLVED → VERIFIED
Whiteboard: [181approval pending]
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+
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8.1
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.