Remove always-true --harmony-function-tostring runtime flag

It was shipped in Chrome 66.

Bug: v8:4958, v8:8255, v8:8238
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: I52fb826f4f245bc6484d8f406ecf99bc17d268ee
Reviewed-on: https://chromium-review.googlesource.com/c/1254123
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56350}

src/bootstrapper.cc

@@ -4385,7 +4385,6 @@ void Bootstrapper::ExportFromRuntime(Isolate* isolate,
   void Genesis::InitializeGlobal_##id() {}
 
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_do_expressions)
-EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_function_tostring)
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_public_fields)
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_private_fields)
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_static_fields)

src/builtins/builtins-function.cc

@@ -41,11 +41,7 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
     IncrementalStringBuilder builder(isolate);
     builder.AppendCharacter('(');
     builder.AppendCString(token);
-    if (FLAG_harmony_function_tostring) {
+    builder.AppendCString(" anonymous(");
-      builder.AppendCString(" anonymous(");
-    } else {
-      builder.AppendCharacter('(');
-    }
     bool parenthesis_in_arg_string = false;
     if (argc > 1) {
       for (int i = 1; i < argc; ++i) {
@@ -55,31 +51,10 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
             isolate, param, Object::ToString(isolate, args.at(i)), Object);
         param = String::Flatten(isolate, param);
         builder.AppendString(param);
-        if (!FLAG_harmony_function_tostring) {
-          // 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.
-          DisallowHeapAllocation no_gc;  // Ensure vectors stay valid.
-          String::FlatContent param_content = param->GetFlatContent();
-          for (int i = 0, length = param->length(); i < length; ++i) {
-            if (param_content.Get(i) == ')') {
-              parenthesis_in_arg_string = true;
-              break;
-            }
-          }
-        }
-      }
-      if (!FLAG_harmony_function_tostring) {
-        // 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.
-        builder.AppendCString("\n/*``*/");
       }
     }
-    if (FLAG_harmony_function_tostring) {
+    builder.AppendCharacter('\n');
-      builder.AppendCharacter('\n');
+    parameters_end_pos = builder.Length();
-      parameters_end_pos = builder.Length();
-    }
     builder.AppendCString(") {\n");
     if (argc > 0) {
       Handle<String> body;
@@ -303,7 +278,7 @@ BUILTIN(FunctionPrototypeToString) {
   }
   // With the revised toString behavior, all callable objects are valid
   // receivers for this method.
-  if (FLAG_harmony_function_tostring && receiver->IsJSReceiver() &&
+  if (receiver->IsJSReceiver() &&
       JSReceiver::cast(*receiver)->map()->is_callable()) {
     return ReadOnlyRoots(isolate).function_native_code_string();
   }

src/compiler.cc

@@ -1243,8 +1243,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
   // is unused (just 0), which means it's an available field to use to indicate
   // this separation. But to make sure we're not causing other false hits, we
   // negate the scope position.
-  if (FLAG_harmony_function_tostring &&
+  if (restriction == ONLY_SINGLE_FUNCTION_LITERAL &&
-      restriction == ONLY_SINGLE_FUNCTION_LITERAL &&
       parameters_end_pos != kNoSourcePosition) {
     // use the parameters_end_pos as the eval_scope_position in the eval cache.
     DCHECK_EQ(eval_scope_position, 0);

src/flag-definitions.h

@@ -236,7 +236,6 @@ DEFINE_IMPLICATION(harmony_class_fields, harmony_private_fields)
 // Features that are shipping (turned on by default, but internal flag remains).
 #define HARMONY_SHIPPING_BASE(V)                                         \
   V(harmony_sharedarraybuffer, "harmony sharedarraybuffer")              \
-  V(harmony_function_tostring, "harmony Function.prototype.toString")    \
   V(harmony_import_meta, "harmony import.meta property")                 \
   V(harmony_dynamic_import, "harmony dynamic import")                    \
   V(harmony_array_prototype_values, "harmony Array.prototype.values")    \

src/objects.cc

@@ -13338,64 +13338,16 @@ Handle<String> JSFunction::ToString(Handle<JSFunction> function) {
     return NativeCodeFunctionSourceString(shared_info);
   }
 
-  if (FLAG_harmony_function_tostring) {
+  if (shared_info->function_token_position() == kNoSourcePosition) {
-    if (shared_info->function_token_position() == kNoSourcePosition) {
+    // If the function token position isn't valid, return [native code] to
-      // If the function token position isn't valid, return [native code] to
+    // ensure calling eval on the returned source code throws rather than
-      // ensure calling eval on the returned source code throws rather than
+    // giving inconsistent call behaviour.
-      // giving inconsistent call behaviour.
+    isolate->CountUsage(
-      isolate->CountUsage(v8::Isolate::UseCounterFeature::
+        v8::Isolate::UseCounterFeature::kFunctionTokenOffsetTooLongForToString);
-                              kFunctionTokenOffsetTooLongForToString);
+    return NativeCodeFunctionSourceString(shared_info);
-      return NativeCodeFunctionSourceString(shared_info);
-    }
-    return Handle<String>::cast(
-        SharedFunctionInfo::GetSourceCodeHarmony(shared_info));
   }
-
+  return Handle<String>::cast(
-  IncrementalStringBuilder builder(isolate);
+      SharedFunctionInfo::GetSourceCodeHarmony(shared_info));
-  FunctionKind kind = shared_info->kind();
-  if (!IsArrowFunction(kind)) {
-    if (IsConciseMethod(kind)) {
-      if (IsAsyncGeneratorFunction(kind)) {
-        builder.AppendCString("async *");
-      } else if (IsGeneratorFunction(kind)) {
-        builder.AppendCharacter('*');
-      } else if (IsAsyncFunction(kind)) {
-        builder.AppendCString("async ");
-      }
-    } else {
-      if (IsAsyncGeneratorFunction(kind)) {
-        builder.AppendCString("async function* ");
-      } else if (IsGeneratorFunction(kind)) {
-        builder.AppendCString("function* ");
-      } else if (IsAsyncFunction(kind)) {
-        builder.AppendCString("async function ");
-      } else {
-        builder.AppendCString("function ");
-      }
-    }
-    if (shared_info->name_should_print_as_anonymous()) {
-      builder.AppendCString("anonymous");
-    } else if (!shared_info->is_anonymous_expression()) {
-      builder.AppendString(handle(shared_info->Name(), isolate));
-    }
-  }
-  if (shared_info->is_wrapped()) {
-    builder.AppendCharacter('(');
-    Handle<FixedArray> args(
-        Script::cast(shared_info->script())->wrapped_arguments(), isolate);
-    int argc = args->length();
-    for (int i = 0; i < argc; i++) {
-      if (i > 0) builder.AppendCString(", ");
-      builder.AppendString(Handle<String>(String::cast(args->get(i)), isolate));
-    }
-    builder.AppendCString(") {\n");
-  }
-  builder.AppendString(
-      Handle<String>::cast(SharedFunctionInfo::GetSourceCode(shared_info)));
-  if (shared_info->is_wrapped()) {
-    builder.AppendCString("\n}");
-  }
-  return builder.Finish().ToHandleChecked();
 }
 
 void Oddball::Initialize(Isolate* isolate, Handle<Oddball> oddball,

src/parsing/parser-base.h

@@ -2407,10 +2407,8 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
 
       ExpressionT value = impl()->ParseFunctionLiteral(
           *name, scanner()->location(), kSkipFunctionNameCheck, kind,
-          FLAG_harmony_function_tostring ? name_token_position
+          name_token_position, FunctionLiteral::kAccessorOrMethod,
-                                         : kNoSourcePosition,
+          language_mode(), nullptr, CHECK_OK_CUSTOM(NullLiteralProperty));
-          FunctionLiteral::kAccessorOrMethod, language_mode(), nullptr,
-          CHECK_OK_CUSTOM(NullLiteralProperty));
 
       *property_kind = ClassLiteralProperty::METHOD;
       ClassLiteralPropertyT result = factory()->NewClassLiteralProperty(
@@ -2441,10 +2439,8 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
 
       FunctionLiteralT value = impl()->ParseFunctionLiteral(
           *name, scanner()->location(), kSkipFunctionNameCheck, kind,
-          FLAG_harmony_function_tostring ? name_token_position
+          name_token_position, FunctionLiteral::kAccessorOrMethod,
-                                         : kNoSourcePosition,
+          language_mode(), nullptr, CHECK_OK_CUSTOM(NullLiteralProperty));
-          FunctionLiteral::kAccessorOrMethod, language_mode(), nullptr,
-          CHECK_OK_CUSTOM(NullLiteralProperty));
 
       *property_kind =
           is_get ? ClassLiteralProperty::GETTER : ClassLiteralProperty::SETTER;
@@ -2639,9 +2635,8 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
 
       ExpressionT value = impl()->ParseFunctionLiteral(
           name, scanner()->location(), kSkipFunctionNameCheck, kind,
-          FLAG_harmony_function_tostring ? next_beg_pos : kNoSourcePosition,
+          next_beg_pos, FunctionLiteral::kAccessorOrMethod, language_mode(),
-          FunctionLiteral::kAccessorOrMethod, language_mode(), nullptr,
+          nullptr, CHECK_OK_CUSTOM(NullLiteralProperty));
-          CHECK_OK_CUSTOM(NullLiteralProperty));
 
       ObjectLiteralPropertyT result = factory()->NewObjectLiteralProperty(
           name_expression, value, ObjectLiteralProperty::COMPUTED,
@@ -2672,9 +2667,8 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
 
       FunctionLiteralT value = impl()->ParseFunctionLiteral(
           name, scanner()->location(), kSkipFunctionNameCheck, kind,
-          FLAG_harmony_function_tostring ? next_beg_pos : kNoSourcePosition,
+          next_beg_pos, FunctionLiteral::kAccessorOrMethod, language_mode(),
-          FunctionLiteral::kAccessorOrMethod, language_mode(), nullptr,
+          nullptr, CHECK_OK_CUSTOM(NullLiteralProperty));
-          CHECK_OK_CUSTOM(NullLiteralProperty));
 
       ObjectLiteralPropertyT result = factory()->NewObjectLiteralProperty(
           name_expression, value,

test/cctest/test-compiler.cc

@@ -766,11 +766,6 @@ void TestCompileFunctionInContextToStringImpl() {
 #undef CHECK_NOT_CAUGHT
 }
 
-TEST(CompileFunctionInContextHarmonyFunctionToString) {
-  v8::internal::FLAG_harmony_function_tostring = true;
-  TestCompileFunctionInContextToStringImpl();
-}
-
 TEST(CompileFunctionInContextFunctionToString) {
   TestCompileFunctionInContextToStringImpl();
 }

test/message/README.md

@@ -1,7 +1,7 @@
 # JavaScript tests with expected output
 
-Tests in test/message pass if the output matches the expected output. Message
+Tests in `test/message` pass if the output matches the expected output.
-tests are particularly useful when checking for exact error messages.
+Message tests are particularly useful when checking for exact error messages.
 
 Tests and their expected output must have the same filename, with the `.js` and
 `.out` extension.
@@ -15,11 +15,13 @@ foo.out
 handle output from multiple runs, e.g.,  `--stress-opt`. Without an exception,
 the output will be generated several times and the comparison will fail.
 
-You can use a regex in the expected output. Instead of the exact
+You can use a regex in the expected output instead of the exact
-path. use
+path:
- ```
+
+```
 *%(basename)s:7: SyntaxError: Detected cycle while resolving name 'a'
 ```
+
 Empty lines are ignored in the comparison, but whitespaces are not.
 
 Exact details of the test runner are in [testcfg.py](testcfg.py).

test/mjsunit/es6/proxy-function-tostring.js

@@ -1,7 +1,6 @@
 // Copyright 2018 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-//
-// Flags: --noharmony-function-tostring
 
-assertThrows(() => new Proxy(function() {}, {}).toString(), TypeError);
+assertEquals(new Proxy(function() {}, {}).toString(),
+             'function () { [native code] }');

test/mjsunit/harmony/function-tostring.js

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-function-tostring
-
 var prefix = "/*before*/";
 var suffix = "/*after*/";
 

test/mjsunit/regress/regress-707066.js

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Flags: --harmony-function-tostring
-
 // There was a bug in CreateDynamicFunction where a stack overflow
 // situation caused an assertion failure.
 

test/test262/test262.status

@@ -364,9 +364,6 @@
   'language/global-code/script-decl-func-err-non-configurable': [FAIL],
   'language/global-code/script-decl-var-collision': [FAIL],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=4958
-  'built-ins/Function/prototype/toString/*': ['--harmony-function-tostring'],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=5116
   'built-ins/TypedArray/prototype/fill/fill-values-conversion-operations-consistent-nan': [PASS, FAIL],