Bug 17018 – Incorrect code generated from Function.toString for get/setters in object literals

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Oliver Hunt]

If you have a function:
function f() {
   ({ set g(x){} });
}

Then f.toString will produce incorrect (non-compilable) output as it quotes the identifier string:
function f() {
   ({ set "g"(x){} });
}

Found by jsfunfuzz -- maybe i had recopmilation set to not test these cases when i was running these?

[Oliver Hunt]

Fix appears to be
diff --git a/JavaScriptCore/kjs/nodes2string.cpp b/JavaScriptCore/kjs/nodes2string.cpp
index 0432b7f..79a0675 100644
--- a/JavaScriptCore/kjs/nodes2string.cpp
+++ b/JavaScriptCore/kjs/nodes2string.cpp
@@ -370,11 +370,11 @@ void PropertyNode::streamTo(SourceStream& s) const
         case Setter: {
             const FuncExprNode* func = static_cast<const FuncExprNode*>(assign.get());
             if (type == Getter)
-                s << "get \""; 
+                s << "get "; 
             else
-                s << "set \"";
+                s << "set ";
             s << escapeStringForPrettyPrinting(name().ustring())
-                << "\"(" << func->param << ')' << func->body;
+                << "(" << func->param << ')' << func->body;
             break;
         }
     }


But I can't see why they were ever escaped strings -- is there a case i am missing where 'set "foo"(x)' would be valid?

[Oliver Hunt]

Created attachment 18706 [details]
Patch!

[Darin Adler]

Comment on attachment 18706 [details]
Patch!

r=me

What about getters and setters with unusual names?

[Oliver Hunt]

Fixed in r29812

[Jesse Ruderman]

*** Bug 13621 has been marked as a duplicate of this bug. ***