Issue #1952 - m-c 1380962/2: Remove unnecessary lambda function in FunctionToString

2 files changed, 58 insertions, 47 deletions

diff --git a/js/src/jit-test/tests/basic/bug1380962.js b/js/src/jit-test/tests/basic/bug1380962.js
new file mode 100644
index 0000000000..66548961d5
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug1380962.js
@@ -0,0 +1,15 @@
+// Source string has balanced parentheses even when the source code was discarded.
+
+setDiscardSource(true);
+eval("var f = function() { return 0; };");
+assertEq(f.toSource(), "(function() {\n    [sourceless code]\n})");
+// Source string has balanced parentheses even when the source code was discarded.
+
+setDiscardSource(true);
+eval("var f = function() { return 0; };");
+assertEq(f.toSource(), "(function() {\n    [sourceless code]\n})");
+// Source string has balanced parentheses even when the source code was discarded.
+
+setDiscardSource(true);
+eval("var f = function() { return 0; };");
+assertEq(f.toSource(), "(function() {\n    [sourceless code]\n})");
diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp
index e71a495028..b4aeee51c2 100644
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -978,8 +978,6 @@ js::FunctionToString(JSContext* cx, HandleFunction fun, bool prettyPrint)
         }
     }
 
-    bool funIsNonArrowLambda = fun->isLambda() && !fun->isArrow();
-
     // Default class constructors are self-hosted, but have their source
     // objects overridden to refer to the span of the class statement or
     // expression. Non-default class constructors are never self-hosted. So,
@@ -989,10 +987,7 @@ js::FunctionToString(JSContext* cx, HandleFunction fun, bool prettyPrint)
 
     // If we're not in pretty mode, put parentheses around lambda functions
     // so that eval returns lambda, not function statement.
-    if (haveSource && !prettyPrint && funIsNonArrowLambda) {
-        if (!out.append('('))
-            return nullptr;
-    }
+    bool addParentheses = haveSource && !prettyPrint && (fun->isLambda() && !fun->isArrow());
 
     if (haveSource && !script->scriptSource()->hasSourceData() &&
         !JSScript::loadSource(cx, script->scriptSource(), &haveSource))
@@ -1000,72 +995,73 @@ js::FunctionToString(JSContext* cx, HandleFunction fun, bool prettyPrint)
         return nullptr;
     }
 
-    auto AppendPrelude = [cx, &out, &fun]() {
+    if (addParentheses) {
+        if (!out.append('('))
+            return nullptr;
+    }
+
+    if (haveSource) {
+        Rooted<JSFlatString*> src(cx, JSScript::sourceDataForToString(cx, script));
+        if (!src)
+            return nullptr;
+
+        if (!out.append(src))
+            return nullptr;
+    } else {
         if (fun->isAsync()) {
             if (!out.append("async "))
-                return false;
+                return nullptr;
         }
 
         if (!fun->isArrow()) {
             if (!out.append("function"))
-                return false;
+                return nullptr;
 
             if (fun->isStarGenerator()) {
                 if (!out.append('*'))
-                    return false;
+                    return nullptr;
             }
         }
 
         if (fun->explicitName()) {
             if (!out.append(' '))
-                return false;
+                return nullptr;
             if (fun->isBoundFunction() && !fun->hasBoundFunctionNamePrefix()) {
                 if (!out.append(cx->names().boundWithSpace))
-                    return false;
+                    return nullptr;
             }
             if (!out.append(fun->explicitName()))
-                return false;
-        }
-        return true;
-    };
-
-    if (haveSource) {
-        Rooted<JSFlatString*> src(cx, JSScript::sourceDataForToString(cx, script));
-        if (!src)
-            return nullptr;
-
-        if (!out.append(src))
-            return nullptr;
-
-        if (!prettyPrint && funIsNonArrowLambda) {
-            if (!out.append(')'))
                 return nullptr;
         }
-    } else if (fun->isInterpreted() &&
-               (!fun->isSelfHostedBuiltin() ||
-                fun->infallibleIsDefaultClassConstructor(cx)))
-    {
-        // Default class constructors should always haveSource except;
-        //
-        // 1. Source has been discarded for the whole compartment.
-        //
-        // 2. The source is marked as "lazy", i.e., retrieved on demand, and
-        // the embedding has not provided a hook to retrieve sources.
-        MOZ_ASSERT_IF(fun->infallibleIsDefaultClassConstructor(cx),
-                      !cx->runtime()->sourceHook ||
-                      !script->scriptSource()->sourceRetrievable() ||
-                      fun->compartment()->behaviors().discardSource());
-        if (!AppendPrelude() ||
-            !out.append("() {\n    [sourceless code]\n}"))
+
+        if (fun->isInterpreted() &&
+            (!fun->isSelfHostedBuiltin() ||
+             fun->infallibleIsDefaultClassConstructor(cx)))
         {
-            return nullptr;
+            // Default class constructors should always haveSource except;
+            //
+            // 1. Source has been discarded for the whole compartment.
+            //
+            // 2. The source is marked as "lazy", i.e., retrieved on demand, and
+            // the embedding has not provided a hook to retrieve sources.
+            MOZ_ASSERT_IF(fun->infallibleIsDefaultClassConstructor(cx),
+                          !cx->runtime()->sourceHook ||
+                          !script->scriptSource()->sourceRetrievable() ||
+                          fun->compartment()->behaviors().discardSource());
+
+            if (!out.append("() {\n    [sourceless code]\n}"))
+                return nullptr;
+        } else {
+            if (!out.append("() {\n    [native code]\n}"))
+                return nullptr;
         }
-    } else {
+    }
 
-        if (!AppendPrelude() ||
-            !out.append("() {\n    [native code]\n}"))
+    if (addParentheses) {
+        if (!out.append(')'))
             return nullptr;
     }
+
     return out.finishString();
 }