Make it possible to define arguments for CompileFunctionInContext

Also make sure that the function body really produces only one function literal.

LOG=y
BUG=none
R=yangguo@chromium.org

Review URL: https://codereview.chromium.org/925433002

Cr-Commit-Position: refs/heads/master@{#26617}

include/v8.h

@@ -1325,7 +1325,7 @@ class V8_EXPORT ScriptCompiler {
    * Compile a function for a given context. This is equivalent to running
    *
    * with (obj) {
-   *   return function() { ... }
+   *   return function(args) { ... }
    * }
    *
    * It is possible to specify multiple context extensions (obj in the above
@@ -1333,6 +1333,7 @@ class V8_EXPORT ScriptCompiler {
    */
   static Local<Function> CompileFunctionInContext(
       Isolate* isolate, Source* source, Local<Context> context,
+      size_t arguments_count, Local<String> arguments[],
       size_t context_extension_count, Local<Object> context_extensions[]);
 
  private:

src/api.cc

@@ -1651,14 +1651,104 @@ Local<Script> ScriptCompiler::CompileModule(Isolate* v8_isolate, Source* source,
 }
 
 
+class IsIdentifierHelper {
+ public:
+  IsIdentifierHelper() : is_identifier_(false), first_char_(true) {}
+
+  bool Check(i::String* string) {
+    i::ConsString* cons_string = i::String::VisitFlat(this, string, 0);
+    if (cons_string == NULL) return is_identifier_;
+    // We don't support cons strings here.
+    return false;
+  }
+  void VisitOneByteString(const uint8_t* chars, int length) {
+    for (int i = 0; i < length; ++i) {
+      if (first_char_) {
+        first_char_ = false;
+        is_identifier_ = unicode_cache_.IsIdentifierStart(chars[0]);
+      } else {
+        is_identifier_ &= unicode_cache_.IsIdentifierPart(chars[i]);
+      }
+    }
+  }
+  void VisitTwoByteString(const uint16_t* chars, int length) {
+    for (int i = 0; i < length; ++i) {
+      if (first_char_) {
+        first_char_ = false;
+        is_identifier_ = unicode_cache_.IsIdentifierStart(chars[0]);
+      } else {
+        is_identifier_ &= unicode_cache_.IsIdentifierPart(chars[i]);
+      }
+    }
+  }
+
+ private:
+  bool is_identifier_;
+  bool first_char_;
+  i::UnicodeCache unicode_cache_;
+  DISALLOW_COPY_AND_ASSIGN(IsIdentifierHelper);
+};
+
+
 Local<Function> ScriptCompiler::CompileFunctionInContext(
     Isolate* v8_isolate, Source* source, Local<Context> v8_context,
+    size_t arguments_count, Local<String> arguments[],
     size_t context_extension_count, Local<Object> context_extensions[]) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
   ON_BAILOUT(isolate, "v8::ScriptCompiler::CompileFunctionInContext()",
              return Local<Function>());
   LOG_API(isolate, "ScriptCompiler::CompileFunctionInContext()");
   ENTER_V8(isolate);
+
+  i::Handle<i::String> source_string;
+  if (arguments_count) {
+    source_string =
+        Utils::OpenHandle(*v8::String::NewFromUtf8(v8_isolate, "(function("));
+    for (size_t i = 0; i < arguments_count; ++i) {
+      IsIdentifierHelper helper;
+      if (!helper.Check(*Utils::OpenHandle(*arguments[i]))) {
+        return Local<Function>();
+      }
+      i::MaybeHandle<i::String> maybe_source =
+          isolate->factory()->NewConsString(source_string,
+                                            Utils::OpenHandle(*arguments[i]));
+      if (!maybe_source.ToHandle(&source_string)) {
+        return Local<Function>();
+      }
+      if (i + 1 == arguments_count) continue;
+      maybe_source = isolate->factory()->NewConsString(
+          source_string,
+          isolate->factory()->LookupSingleCharacterStringFromCode(','));
+      if (!maybe_source.ToHandle(&source_string)) {
+        return Local<Function>();
+      }
+    }
+    i::Handle<i::String> brackets =
+        Utils::OpenHandle(*v8::String::NewFromUtf8(v8_isolate, "){"));
+    i::MaybeHandle<i::String> maybe_source =
+        isolate->factory()->NewConsString(source_string, brackets);
+    if (!maybe_source.ToHandle(&source_string)) {
+      return Local<Function>();
+    }
+  } else {
+    source_string =
+        Utils::OpenHandle(*v8::String::NewFromUtf8(v8_isolate, "(function(){"));
+  }
+
+  int scope_position = source_string->length();
+  i::MaybeHandle<i::String> maybe_source = isolate->factory()->NewConsString(
+      source_string, Utils::OpenHandle(*source->source_string));
+  if (!maybe_source.ToHandle(&source_string)) {
+    return Local<Function>();
+  }
+  // Include \n in case the source contains a line end comment.
+  i::Handle<i::String> brackets =
+      Utils::OpenHandle(*v8::String::NewFromUtf8(v8_isolate, "\n})"));
+  maybe_source = isolate->factory()->NewConsString(source_string, brackets);
+  if (!maybe_source.ToHandle(&source_string)) {
+    return Local<Function>();
+  }
+
   i::Handle<i::Context> context = Utils::OpenHandle(*v8_context);
   i::Handle<i::SharedFunctionInfo> outer_info(context->closure()->shared(),
                                               isolate);
@@ -1668,13 +1758,21 @@ Local<Function> ScriptCompiler::CompileFunctionInContext(
     i::Handle<i::JSFunction> closure(context->closure(), isolate);
     context = isolate->factory()->NewWithContext(closure, context, extension);
   }
+
   EXCEPTION_PREAMBLE(isolate);
-  i::MaybeHandle<i::JSFunction> result = i::Compiler::GetFunctionFromEval(
-      Utils::OpenHandle(*source->source_string), outer_info, context, i::SLOPPY,
-      i::NO_PARSE_RESTRICTION, 0 /* scope_position */);
-  has_pending_exception = result.is_null();
+  i::MaybeHandle<i::JSFunction> maybe_fun = i::Compiler::GetFunctionFromEval(
+      source_string, outer_info, context, i::SLOPPY,
+      i::ONLY_SINGLE_FUNCTION_LITERAL, scope_position);
+  i::Handle<i::JSFunction> fun;
+  has_pending_exception = !maybe_fun.ToHandle(&fun);
+  EXCEPTION_BAILOUT_CHECK(isolate, Local<Function>());
+
+  i::MaybeHandle<i::Object> result = i::Execution::Call(
+      isolate, fun, Utils::OpenHandle(*v8_context->Global()), 0, NULL);
+  i::Handle<i::Object> final_result;
+  has_pending_exception = !result.ToHandle(&final_result);
   EXCEPTION_BAILOUT_CHECK(isolate, Local<Function>());
-  return Utils::ToLocal(result.ToHandleChecked());
+  return Utils::ToLocal(i::Handle<i::JSFunction>::cast(final_result));
 }
 
 

test/cctest/test-compiler.cc

@@ -416,7 +416,7 @@ TEST(CompileFunctionInContext) {
       "x = r * cos(PI);"
       "y = r * sin(PI / 2);"));
   v8::Local<v8::Function> fun = v8::ScriptCompiler::CompileFunctionInContext(
-      CcTest::isolate(), &script_source, env.local(), 1, &math);
+      CcTest::isolate(), &script_source, env.local(), 0, NULL, 1, &math);
   CHECK(!fun.IsEmpty());
   fun->Call(env->Global(), 0, NULL);
   CHECK(env->Global()->Has(v8_str("a")));
@@ -449,7 +449,7 @@ TEST(CompileFunctionInContextComplex) {
   ext[1] = v8::Local<v8::Object>::Cast(env->Global()->Get(v8_str("b")));
   v8::ScriptCompiler::Source script_source(v8_str("result = x + y + z"));
   v8::Local<v8::Function> fun = v8::ScriptCompiler::CompileFunctionInContext(
-      CcTest::isolate(), &script_source, env.local(), 2, ext);
+      CcTest::isolate(), &script_source, env.local(), 0, NULL, 2, ext);
   CHECK(!fun.IsEmpty());
   fun->Call(env->Global(), 0, NULL);
   CHECK(env->Global()->Has(v8_str("result")));
@@ -459,6 +459,61 @@ TEST(CompileFunctionInContextComplex) {
 }
 
 
+TEST(CompileFunctionInContextArgs) {
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  LocalContext env;
+  CompileRun("var a = {x: 23};");
+  v8::Local<v8::Object> ext[1];
+  ext[0] = v8::Local<v8::Object>::Cast(env->Global()->Get(v8_str("a")));
+  v8::ScriptCompiler::Source script_source(v8_str("result = x + b"));
+  v8::Local<v8::String> arg = v8_str("b");
+  v8::Local<v8::Function> fun = v8::ScriptCompiler::CompileFunctionInContext(
+      CcTest::isolate(), &script_source, env.local(), 1, &arg, 1, ext);
+  CHECK(!fun.IsEmpty());
+  v8::Local<v8::Value> b_value = v8::Number::New(CcTest::isolate(), 42.0);
+  fun->Call(env->Global(), 1, &b_value);
+  CHECK(env->Global()->Has(v8_str("result")));
+  v8::Local<v8::Value> result = env->Global()->Get(v8_str("result"));
+  CHECK(result->IsNumber());
+  CHECK_EQ(65.0, result->NumberValue());
+}
+
+
+TEST(CompileFunctionInContextComments) {
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  LocalContext env;
+  CompileRun("var a = {x: 23, y: 1, z: 2};");
+  v8::Local<v8::Object> ext[1];
+  ext[0] = v8::Local<v8::Object>::Cast(env->Global()->Get(v8_str("a")));
+  v8::ScriptCompiler::Source script_source(
+      v8_str("result = /* y + */ x + b // + z"));
+  v8::Local<v8::String> arg = v8_str("b");
+  v8::Local<v8::Function> fun = v8::ScriptCompiler::CompileFunctionInContext(
+      CcTest::isolate(), &script_source, env.local(), 1, &arg, 1, ext);
+  CHECK(!fun.IsEmpty());
+  v8::Local<v8::Value> b_value = v8::Number::New(CcTest::isolate(), 42.0);
+  fun->Call(env->Global(), 1, &b_value);
+  CHECK(env->Global()->Has(v8_str("result")));
+  v8::Local<v8::Value> result = env->Global()->Get(v8_str("result"));
+  CHECK(result->IsNumber());
+  CHECK_EQ(65.0, result->NumberValue());
+}
+
+
+TEST(CompileFunctionInContextNonIdentifierArgs) {
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  LocalContext env;
+  v8::ScriptCompiler::Source script_source(v8_str("result = 1"));
+  v8::Local<v8::String> arg = v8_str("b }");
+  v8::Local<v8::Function> fun = v8::ScriptCompiler::CompileFunctionInContext(
+      CcTest::isolate(), &script_source, env.local(), 1, &arg, 0, NULL);
+  CHECK(fun.IsEmpty());
+}
+
+
 #ifdef ENABLE_DISASSEMBLER
 static Handle<JSFunction> GetJSFunction(v8::Handle<v8::Object> obj,
                                  const char* property_name) {