Bug 17027 - Incorrect Function.toString behaviour with read/modify/write operators performed on negative numbers
Summary: Incorrect Function.toString behaviour with read/modify/write operators perfor...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: HasReduction
Depends on:
Blocks: 13638
  Show dependency treegraph
 
Reported: 2008-01-26 18:20 PST by Oliver Hunt
Modified: 2008-02-11 17:02 PST (History)
2 users (show)

See Also:


Attachments
patch (11.79 KB, patch)
2008-01-26 20:05 PST, Darin Adler
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2008-01-26 18:20:38 PST
Function.toString loses necessary parentheses when performing read/modify/write operations like >>>=, etc on *negative* numbers.

eg.
function f() {
   (-1) >>>= 1;
}
becomes
function f() {
  -1 >>>= 1;
}
Comment 1 Darin Adler 2008-01-26 19:10:50 PST
I think the trick here is that in SourceStream::operator<<(double) we need to treat the "-" sign from the negative number as an operator, since that's how the lexer is going to treat it.

Thus we need to check the precedence against the precedence of unary minus and set needParens in that case. This can all be fixed on the first line of that function where we initialize a needParens bool.

Assigning to myself, assuming that if Ollie was going to fix this he'd have assigned it to himself.
Comment 2 Darin Adler 2008-01-26 19:22:28 PST
I found an even better way to fix this, by just returning the appropriate precedence. I'll make the test exhaustive now.
Comment 3 Oliver Hunt 2008-01-26 19:35:45 PST
That's basically what i was thinking :D

However i decided to get food :D
Comment 4 Darin Adler 2008-01-26 20:05:26 PST
Created attachment 18715 [details]
patch
Comment 5 Oliver Hunt 2008-01-26 20:15:05 PST
Comment on attachment 18715 [details]
patch

r=m1, with the additional tests i requested on irc
Comment 6 Darin Adler 2008-01-26 20:23:15 PST
Committed revision 29815.