Drop parameters on NativeFunction toString.

Summary:
Hermes, historistically, synthsize formal parameters from the
function arity, while all other engines print NativeFunction as
0-arity. This gave the JS community the assumption that this
behavior is standardized and come out heuristics that detect
NativeFunction by matching on `() { [native code] }` and resulted
in Hermes fail the check and trigger librares' slow path.

This diff drop the synthsized parameters for Hermes toString
implementation on NativeFunction to avoid breaking those heuristcs.

This fix #471.

This change alone improved `test-lodash` benchmark on Hermes
from `787` to `43` which is in the same ballpark with others on my local:
`v8 --jitless` (30), `jsc --useJIT=false` (44), and `qjs` (60).

Also see normative dicussion to ECMA-262 on enforcing the spec
closer to the reality: tc39/ecma262#2381.

Reviewed By: avp

Differential Revision: D27698165

fbshipit-source-id: bb39438bd21e34fb0c4d7c5df5d97db939648295

lib/VM/JSLib/Function.cpp

@@ -143,47 +143,51 @@ functionPrototypeToString(void *, Runtime *runtime, NativeArgs args) {
   // Extract the name.
   auto propRes = JSObject::getNamed_RJS(
       func, runtime, Predefined::getSymbolID(Predefined::name));
-  if (propRes == ExecutionStatus::EXCEPTION) {
+  if (LLVM_UNLIKELY(propRes == ExecutionStatus::EXCEPTION)) {
     return ExecutionStatus::EXCEPTION;
   }
 
   // Convert the name to string, unless it is undefined.
   if (!(*propRes)->isUndefined()) {
     auto strRes =
         toString_RJS(runtime, runtime->makeHandle(std::move(*propRes)));
-    if (strRes == ExecutionStatus::EXCEPTION) {
+    if (LLVM_UNLIKELY(strRes == ExecutionStatus::EXCEPTION)) {
       return ExecutionStatus::EXCEPTION;
     }
     strRes->get()->appendUTF16String(strBuf);
   }
 
-  // Append the named parameters.
-  strBuf.append('(');
-
-  // Extract ".length".
-  auto lengthProp = Callable::extractOwnLengthProperty_RJS(func, runtime);
-  if (lengthProp == ExecutionStatus::EXCEPTION)
-    return ExecutionStatus::EXCEPTION;
+  // Formal parameters and the rest of the body.
+  if (vmisa<NativeFunction>(*func)) {
+    // Use [native code] here because we want to work with tools like Babel
+    // which detect the string "[native code]" and use it to alter behavior
+    // during the class transform.
+    // Also print without synthesized formal parameters to avoid breaking
+    // heuristics that detect the string "() { [native code] }".
+    // \see https://github.com/facebook/hermes/issues/471
+    strBuf.append("() { [native code] }");
+  } else {
+    // Append the synthesized formal parameters.
+    strBuf.append('(');
 
-  // The value of the property is not guaranteed to be meaningful, so clamp it
-  // to [0..65535] for sanity.
-  uint32_t paramCount = (uint32_t)std::min(65535.0, std::max(0.0, *lengthProp));
+    // Extract ".length".
+    auto lengthProp = Callable::extractOwnLengthProperty_RJS(func, runtime);
+    if (lengthProp == ExecutionStatus::EXCEPTION)
+      return ExecutionStatus::EXCEPTION;
 
-  for (uint32_t i = 0; i < paramCount; ++i) {
-    if (i != 0)
-      strBuf.append(", ");
-    char buf[16];
-    ::snprintf(buf, sizeof(buf), "a%u", i);
-    strBuf.append(buf);
-  }
+    // The value of the property is not guaranteed to be meaningful, so clamp it
+    // to [0..65535] for sanity.
+    uint32_t paramCount =
+        (uint32_t)std::min(65535.0, std::max(0.0, *lengthProp));
+
+    for (uint32_t i = 0; i < paramCount; ++i) {
+      if (i != 0)
+        strBuf.append(", ");
+      char buf[16];
+      ::snprintf(buf, sizeof(buf), "a%u", i);
+      strBuf.append(buf);
+    }
 
-  // The rest of the body.
-  if (vmisa<NativeFunction>(*func)) {
-    // Use [native code] here because we want to work with tools like
-    // Babel which detect the string [native code] and use it to alter
-    // behavior during the class transform.
-    strBuf.append(") { [native code] }");
-  } else {
     // Avoid using the [native code] string to prevent extra wrapping overhead
     // in, e.g., Babel's class extension mechanism.
     strBuf.append(") { [bytecode] }");

test/hermes/function-constructor.js

@@ -55,15 +55,25 @@ print(function(a,b,c){});
 // CHECK-NEXT: function (a0, a1, a2) { [bytecode] }
 print(function(a,b,c,d,e,f,g,h,i,j,k){});
 // CHECK-NEXT: function (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10) { [bytecode] }
-print(Map.toString());
-// CHECK-NEXT: function Map() { [native code] }
+
+// Reconfigured .length
+function foo(x) {}
+Object.defineProperty(foo, "length", { value: 5 })
+print(foo);
+// CHECK-NEXT: function foo(a0, a1, a2, a3, a4) { [bytecode] }
 
 // Non-string .length
 var foo = function badlength(a, b, c){};
 Object.defineProperty(foo, "length", {value:"aa"});
 print(foo);
 // CHECK-NEXT: function badlength() { [bytecode] }
 
+// NativeFunctions are printed as 0-arity.
+print(Map);
+// CHECK-NEXT: function Map() { [native code] }
+print(Math.pow);
+// CHECK-NEXT: function pow() { [native code] }
+
 print('call');
 // CHECK-LABEL: call
 var f = function(a, b) {