GeneratorFunction() makes generator instances

src/v8natives.js

Index: src/v8natives.js
diff --git a/src/v8natives.js b/src/v8natives.js
index fd8b7f20689e6a87d89cc9db30c358caa84b7413..a92526c72f11554e479e1a5b32f63e7cf55f4f80 100644
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -1756,24 +1756,23 @@ function FunctionBind(this_arg) { // Length is 1.
 }
 
 
-function NewFunction(arg1) {  // length == 1
-  var n = %_ArgumentsLength();
-  var p = '';
-  if (n > 1) {
-    p = new InternalArray(n - 1);
-    for (var i = 0; i < n - 1; i++) p[i] = %_Arguments(i);
-    p = Join(p, n - 1, ',', NonStringToString);
+// InternalArray -> string -> boolean -> Function

[rossberg, 2013/05/13 11:33:46]

Nit: JS functions aren't curried. ;)

[wingo, 2013/05/14 10:11:50]

My notational ignorance showing again :)

+function NewFunctionHelper(formals, body, is_generator) {
+  var formals_string = '';
+  if (formals.length) {

[rossberg, 2013/05/13 11:33:46]

Nit: ... > 0

[wingo, 2013/05/14 10:11:50]

Done.

+    formals_string = Join(formals, formals.length, ',', NonStringToString);
     // If the formal parameters string include ) - an illegal
     // character - it may make the combined function expression
     // compile. We avoid this problem by checking for this early on.
-    if (p.indexOf(')') != -1) throw MakeSyntaxError('paren_in_arg_string',[]);
+    if (formals_string.indexOf(')') != -1)
+      throw MakeSyntaxError('paren_in_arg_string',[]);

[rossberg, 2013/05/13 11:33:46]

Nit: can you insert a space before []?

[wingo, 2013/05/14 10:11:50]

Done.

     // If the formal parameters include an unbalanced block comment, the
     // function must be rejected. Since JavaScript does not allow nested
     // comments we can include a trailing block comment to catch this.
-    p += '\n/' + '**/';
+    formals_string += '\n/' + '**/';
   }
-  var body = (n > 0) ? ToString(%_Arguments(n - 1)) : '';
-  var source = '(function(' + p + ') {\n' + body + '\n})';
+  var head = is_generator ? '(function*(' : '(function(';
+  var source = head + formals_string + ') {\n' + body + '\n})';
 
   var global_receiver = %GlobalReceiver(global);
   var f = %_CallFunction(global_receiver, %CompileString(source, true));
@@ -1783,6 +1782,18 @@ function NewFunction(arg1) {  // length == 1
 }
 
 
+function NewFunction(arg1) {  // length == 1

[Michael Starzinger, 2013/05/13 11:35:39]

Since we are at it, can we rename this to FunctionConstructor, this would be in
line with the rest of the code-base. I have no clue why this one is named
differently. The name is not visible to JavaScript anyways.

[wingo, 2013/05/14 10:11:50]

Done.  I renamed the nelper to NewFunction, as it seemed more descriptive than
FunctionConstructorHelper; let me know if you prefer another name.

+  var n = %_ArgumentsLength();
+  if (n) {

[rossberg, 2013/05/13 11:33:46]

Nit: n > 0

But I'd actually avoid the whole copying and simply forward the arguments
object.

[wingo, 2013/05/14 10:11:50]

Fixed the nit.  We can't copy arguments as-is because the last item is the
function body, and the formals array is passed to Join().  We could rework that
logic but this function seems a bit fragile; I thought perhaps leaving it as it
was would be best.

[rossberg, 2013/05/14 14:05:46]

Yes, but the code not only copies, but also takes apart the arguments array in
two places, which are completely identical. I see no reason not to push all that
into the helper function.

+    var formals = new InternalArray(n - 1);
+    for (var i = 0; i < n - 1; i++) formals[i] = %_Arguments(i);
+    return NewFunctionHelper(formals, ToString(%_Arguments(n - 1)), false);
+  } else {
+    return NewFunctionHelper(new InternalArray(0), '', false);
+  }
+}
+
+
 // ----------------------------------------------------------------------------
 
 function SetUpFunction() {