Bug 17012 - REGRESSION: JSC can't round trip an object literal
Summary: REGRESSION: JSC can't round trip an object literal
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: Oliver Hunt
URL:
Keywords: HasReduction, Regression
Depends on:
Blocks: 13638
  Show dependency tree / graph
 
Reported: 2008-01-25 19:01 PST by Oliver Hunt
Modified: 2008-02-11 17:02 PST (History)
4 users (show)

See Also:

Attachments
Patch to fix the bug (2.96 KB, patch)
2008-01-25 22:22 PST, Oliver Hunt 		oliver: review-
Details | Formatted Diff | Diff
New version of fix (7.65 KB, patch)
2008-01-25 23:39 PST, Oliver Hunt 		mjs: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2008-01-25 19:01:42 PST 
JSC looses a pair of parenthesis when performing toString on a function, so that
  function f(){
   ({}).x;
  }
  f.toString()
produces
  function f(){
   {}.x;
  }
which is invalid.

This was found by jsfunfuzz
Comment 1 Oliver Hunt 2008-01-25 19:14:21 PST 
I have a one line fix for this
Comment 2 Sam Weinig 2008-01-25 19:15:48 PST 
This is a regression from shipping Safari.
Comment 3 Jesse Ruderman 2008-01-25 19:17:08 PST 
toSource and uneval have to put parens around functions, but toString doesn't have to.  JSC's current behavior matches Spidermonkey.
Comment 4 Jesse Ruderman 2008-01-25 19:26:16 PST 
Never mind, I misread the bug.
Comment 5 Oliver Hunt 2008-01-25 19:59:29 PST 
this also occurs with function expressions
Comment 6 Oliver Hunt 2008-01-25 22:12:49 PST 
I was wrong, it looks like our current funciton expression behaviour is sufficient
Comment 7 Oliver Hunt 2008-01-25 22:22:13 PST 
Created attachment 18697 [details]
Patch to fix the bug
Comment 8 Darin Adler 2008-01-25 22:36:05 PST 
Comment on attachment 18697 [details]
Patch to fix the bug

r=me

But is this a bug fix or not?
Comment 9 Oliver Hunt 2008-01-25 22:47:58 PST 
Comment on attachment 18697 [details]
Patch to fix the bug

This is a bug fix -- the current code means Function.toString can produce incorrect code.

Maciej suggested an approach that would not use unnecessary ()'s for object literals.
Comment 10 Oliver Hunt 2008-01-25 23:39:44 PST 
Created attachment 18699 [details]
New version of fix

No longer introduce unnecessary ()'s
Comment 11 Maciej Stachowiak 2008-01-25 23:43:50 PST 
Comment on attachment 18699 [details]
New version of fix

r=me
Comment 12 Oliver Hunt 2008-01-25 23:56:53 PST 
Landed r29802