Commit aeb8f332 authored by Josh Wolfe's avatar Josh Wolfe Committed by Commit Bot

[api] remove --harmony-function-tostring behavior from CompileFunctionInContext

The ')'-in-parameter checking is not necessary for
CompileFunctionInContext. The arguments array is expected to be an
array of identifiers, not an array of arbitrary strings that get
concatenated.

Furthermore, there's no reason to have the .toString() representation
look like it came from CreateDynamicFunction(), and in fact inserting
line breaks makes it more complicated to map line and column numbers
correctly.

Overall, the --harmony-function-tostring behavior only makes
CompileFunctionInContext worse, so this CL removes it.

R=littledan@chromium.org, adamk@chromium.org, caitp@igalia.com
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Bug: v8:4958
Change-Id: Ifbc8a83216ca6a6979da1199972aa65f4bee36c3
Reviewed-on: https://chromium-review.googlesource.com/558220Reviewed-by: 's avatarDaniel Ehrenberg <littledan@chromium.org>
Commit-Queue: Josh Wolfe <jwolfe@igalia.com>
Cr-Commit-Position: refs/heads/master@{#46456}
parent ede92883
...@@ -2376,14 +2376,9 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext( ...@@ -2376,14 +2376,9 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
Function); Function);
TRACE_EVENT0("v8", "V8.ScriptCompiler"); TRACE_EVENT0("v8", "V8.ScriptCompiler");
i::Handle<i::String> source_string; i::Handle<i::String> source_string;
int parameters_end_pos = i::kNoSourcePosition;
auto factory = isolate->factory(); auto factory = isolate->factory();
if (arguments_count) { if (arguments_count) {
if (i::FLAG_harmony_function_tostring) { source_string = factory->NewStringFromStaticChars("(function(");
source_string = factory->NewStringFromStaticChars("(function anonymous(");
} else {
source_string = factory->NewStringFromStaticChars("(function(");
}
for (size_t i = 0; i < arguments_count; ++i) { for (size_t i = 0; i < arguments_count; ++i) {
IsIdentifierHelper helper; IsIdentifierHelper helper;
if (!helper.Check(*Utils::OpenHandle(*arguments[i]))) { if (!helper.Check(*Utils::OpenHandle(*arguments[i]))) {
...@@ -2402,25 +2397,12 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext( ...@@ -2402,25 +2397,12 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
RETURN_ON_FAILED_EXECUTION(Function); RETURN_ON_FAILED_EXECUTION(Function);
} }
i::Handle<i::String> brackets; i::Handle<i::String> brackets;
if (i::FLAG_harmony_function_tostring) { brackets = factory->NewStringFromStaticChars("){");
// Append linefeed and signal that text beyond the linefeed is not part of
// the formal parameters.
brackets = factory->NewStringFromStaticChars("\n) {\n");
parameters_end_pos = source_string->length() + 1;
} else {
brackets = factory->NewStringFromStaticChars("){");
}
has_pending_exception = !factory->NewConsString(source_string, brackets) has_pending_exception = !factory->NewConsString(source_string, brackets)
.ToHandle(&source_string); .ToHandle(&source_string);
RETURN_ON_FAILED_EXECUTION(Function); RETURN_ON_FAILED_EXECUTION(Function);
} else { } else {
if (i::FLAG_harmony_function_tostring) { source_string = factory->NewStringFromStaticChars("(function(){");
source_string =
factory->NewStringFromStaticChars("(function anonymous(\n) {\n");
parameters_end_pos = source_string->length() - 4;
} else {
source_string = factory->NewStringFromStaticChars("(function(){");
}
} }
int scope_position = source_string->length(); int scope_position = source_string->length();
...@@ -2470,7 +2452,7 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext( ...@@ -2470,7 +2452,7 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
has_pending_exception = has_pending_exception =
!i::Compiler::GetFunctionFromEval( !i::Compiler::GetFunctionFromEval(
source_string, outer_info, context, i::SLOPPY, source_string, outer_info, context, i::SLOPPY,
i::ONLY_SINGLE_FUNCTION_LITERAL, parameters_end_pos, i::ONLY_SINGLE_FUNCTION_LITERAL, i::kNoSourcePosition,
eval_scope_position, eval_position, line_offset, eval_scope_position, eval_position, line_offset,
column_offset - scope_position, name_obj, source->resource_options) column_offset - scope_position, name_obj, source->resource_options)
.ToHandle(&fun); .ToHandle(&fun);
......
...@@ -595,9 +595,7 @@ TEST(CompileFunctionInContextHarmonyFunctionToString) { ...@@ -595,9 +595,7 @@ TEST(CompileFunctionInContextHarmonyFunctionToString) {
v8::Local<v8::String> result = v8::Local<v8::String> result =
fun->ToString(env.local()).ToLocalChecked(); fun->ToString(env.local()).ToLocalChecked();
v8::Local<v8::String> expected = v8_str( v8::Local<v8::String> expected = v8_str(
"function anonymous(event\n" "function(event){return event\n"
") {\n"
"return event\n"
"}"); "}");
CHECK(expected->Equals(env.local(), result).FromJust()); CHECK(expected->Equals(env.local(), result).FromJust());
} }
...@@ -621,9 +619,7 @@ TEST(CompileFunctionInContextHarmonyFunctionToString) { ...@@ -621,9 +619,7 @@ TEST(CompileFunctionInContextHarmonyFunctionToString) {
v8::Local<v8::String> result = v8::Local<v8::String> result =
fun->ToString(env.local()).ToLocalChecked(); fun->ToString(env.local()).ToLocalChecked();
v8::Local<v8::String> expected = v8_str( v8::Local<v8::String> expected = v8_str(
"function anonymous(\n" "function(){return 0\n"
") {\n"
"return 0\n"
"}"); "}");
CHECK(expected->Equals(env.local(), result).FromJust()); CHECK(expected->Equals(env.local(), result).FromJust());
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment