Bug 1320388: Move JSFunction::HAS_REST to JSScript and LazyScript

Issue #78
[Depends on] Bug 883377: Implement ES6 function "name" property
semantics

16 files changed, 52 insertions, 37 deletions

diff --git a/js/src/builtin/ReflectParse.cpp b/js/src/builtin/ReflectParse.cpp
index 748ff7351..e150ed729 100644
--- a/js/src/builtin/ReflectParse.cpp
+++ b/js/src/builtin/ReflectParse.cpp
@@ -3423,7 +3423,7 @@ ASTSerializer::function(ParseNode* pn, ASTType type, MutableHandleValue dst)
     NodeVector defaults(cx);
 
     RootedValue body(cx), rest(cx);
-    if (func->hasRest())
+    if (pn->pn_funbox->hasRest())
         rest.setUndefined();
     else
         rest.setNull();
diff --git a/js/src/builtin/TypedObject.cpp b/js/src/builtin/TypedObject.cpp
index b7297c894..ae74f01bf 100644
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@@ -230,7 +230,7 @@ const Class js::ScalarTypeDescr::class_ = {
 
 const JSFunctionSpec js::ScalarTypeDescr::typeObjectMethods[] = {
     JS_SELF_HOSTED_FN("toSource", "DescrToSource", 0, 0),
-    JS_SELF_HOSTED_FN("array", "ArrayShorthand", 1, JSFUN_HAS_REST),
+    JS_SELF_HOSTED_FN("array", "ArrayShorthand", 1, 0),
     JS_SELF_HOSTED_FN("equivalent", "TypeDescrEquivalent", 1, 0),
     JS_FS_END
 };
diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp
index 1e9d8f224..6ceb3ed7a 100644
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -1053,7 +1053,7 @@ BytecodeEmitter::EmitterScope::enterFunction(BytecodeEmitter* bce, FunctionBox*
             if (p) {
                 MOZ_ASSERT(bi.kind() == BindingKind::FormalParameter);
                 MOZ_ASSERT(!funbox->hasDestructuringArgs);
-                MOZ_ASSERT(!funbox->function()->hasRest());
+                MOZ_ASSERT(!funbox->hasRest());
                 p->value() = loc;
                 continue;
             }
@@ -8017,7 +8017,7 @@ BytecodeEmitter::isRestParameter(ParseNode* pn, bool* result)
 
     FunctionBox* funbox = sc->asFunctionBox();
     RootedFunction fun(cx, funbox->function());
-    if (!fun->hasRest()) {
+    if (!funbox->hasRest()) {
         *result = false;
         return true;
     }
@@ -8960,7 +8960,7 @@ BytecodeEmitter::emitFunctionFormalParameters(ParseNode* pn)
     EmitterScope* funScope = innermostEmitterScope;
 
     bool hasParameterExprs = funbox->hasParameterExprs;
-    bool hasRest = funbox->function()->hasRest();
+    bool hasRest = funbox->hasRest();
 
     uint16_t argSlot = 0;
     for (ParseNode* arg = pn->pn_head; arg != funBody; arg = arg->pn_next, argSlot++) {
diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp
index f42546eb5..78e47ceb3 100644
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -465,6 +465,7 @@ FunctionBox::FunctionBox(ExclusiveContext* cx, LifoAlloc& alloc, ObjectBox* trac
     usesApply(false),
     usesThis(false),
     usesReturn(false),
+    hasRest_(false),
     funCxFlags()
 {
     // Functions created at parse time may be set singleton after parsing and
@@ -477,7 +478,6 @@ void
 FunctionBox::initFromLazyFunction()
 {
     JSFunction* fun = function();
-    length = fun->nargs() - fun->hasRest();
     if (fun->lazyScript()->isDerivedClassConstructor())
         setDerivedClassConstructor();
     if (fun->lazyScript()->needsHomeObject())
@@ -492,8 +492,6 @@ FunctionBox::initStandaloneFunction(Scope* enclosingScope)
     // Standalone functions are Function or Generator constructors and are
     // always scoped to the global.
     MOZ_ASSERT(enclosingScope->is<GlobalScope>());
-    JSFunction* fun = function();
-    length = fun->nargs() - fun->hasRest();
     enclosingScope_ = enclosingScope;
     allowNewTarget_ = true;
     thisBinding_ = ThisBinding::Function;
@@ -2214,6 +2212,8 @@ Parser<SyntaxParseHandler>::finishFunction()
         lazy->setStrict();
     lazy->setGeneratorKind(funbox->generatorKind());
     lazy->setAsyncKind(funbox->asyncKind());
+    if (funbox->hasRest())
+        lazy->setHasRest();
     if (funbox->isLikelyConstructorWrapper())
         lazy->setLikelyConstructorWrapper();
     if (funbox->isDerivedClassConstructor())
@@ -2757,7 +2757,7 @@ Parser<ParseHandler>::functionArguments(YieldHandling yieldHandling, FunctionSyn
                 }
 
                 hasRest = true;
-                funbox->function()->setHasRest();
+                funbox->setHasRest();
 
                 if (!tokenStream.getToken(&tt))
                     return false;
diff --git a/js/src/frontend/SharedContext.h b/js/src/frontend/SharedContext.h
index 39df47c20..a6ac542f6 100644
--- a/js/src/frontend/SharedContext.h
+++ b/js/src/frontend/SharedContext.h
@@ -471,6 +471,7 @@ class FunctionBox : public ObjectBox, public SharedContext
     bool            usesApply:1;            /* contains an f.apply() call */
     bool            usesThis:1;             /* contains 'this' */
     bool            usesReturn:1;           /* contains a 'return' statement */
+    bool            hasRest_:1;             /* has rest parameter */
 
     FunctionContextFlags funCxFlags;
 
@@ -539,6 +540,11 @@ class FunctionBox : public ObjectBox, public SharedContext
     bool isAsync() const { return asyncKind() == AsyncFunction; }
     bool isArrow() const { return function()->isArrow(); }
 
+    bool hasRest() const { return hasRest_; }
+    void setHasRest() {
+        hasRest_ = true;
+    }
+
     void setGeneratorKind(GeneratorKind kind) {
         // A generator kind can be set at initialization, or when "yield" is
         // first seen.  In both cases the transition can only happen from
@@ -567,7 +573,7 @@ class FunctionBox : public ObjectBox, public SharedContext
     void setHasInnerFunctions()            { funCxFlags.hasInnerFunctions         = true; }
 
     bool hasSimpleParameterList() const {
-        return !function()->hasRest() && !hasParameterExprs && !hasDestructuringArgs;
+        return !hasRest() && !hasParameterExprs && !hasDestructuringArgs;
     }
 
     bool hasMappedArgsObj() const {
diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
index e6fc1f98b..bbf467808 100644
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3435,10 +3435,7 @@ JS::NewFunctionFromSpec(JSContext* cx, const JSFunctionSpec* fs, HandleId id)
         {
             return nullptr;
         }
-        JSFunction* fun = &funVal.toObject().as<JSFunction>();
-        if (fs->flags & JSFUN_HAS_REST)
-            fun->setHasRest();
-        return fun;
+        return &funVal.toObject().as<JSFunction>();
     }
 
     RootedAtom atom(cx, IdToFunctionName(cx, id));
diff --git a/js/src/jsapi.h b/js/src/jsapi.h
index 332ce8562..cbef0f8fb 100644
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -875,11 +875,7 @@ class MOZ_STACK_CLASS SourceBufferHolder final
 
 #define JSFUN_CONSTRUCTOR      0x400    /* native that can be called as a ctor */
 
-//                             0x800    /* Unused */
-
-#define JSFUN_HAS_REST        0x1000    /* function has ...rest parameter. */
-
-#define JSFUN_FLAGS_MASK      0x1e00    /* | of all the JSFUN_* flags */
+#define JSFUN_FLAGS_MASK       0x600    /* | of all the JSFUN_* flags */
 
 /*
  * If set, will allow redefining a non-configurable property, but only on a
diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp
index 1e1b76d5d..2359e28a2 100644
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1329,8 +1329,7 @@ JSFunction::getLength(JSContext* cx, uint16_t* length)
     if (self->isInterpretedLazy() && !self->getOrCreateScript(cx))
         return false;
 
-    *length = self->hasScript() ? self->nonLazyScript()->funLength()
-                                : (self->nargs() - self->hasRest());
+    *length = self->isNative() ? self->nargs() : self->nonLazyScript()->funLength();
     return true;
 }
 
@@ -1620,7 +1619,7 @@ const JSFunctionSpec js::function_methods[] = {
     JS_FN(js_apply_str,      fun_apply,      2,0),
     JS_FN(js_call_str,       fun_call,       1,0),
     JS_FN("isGenerator",     fun_isGenerator,0,0),
-    JS_SELF_HOSTED_FN("bind", "FunctionBind", 2, JSFUN_HAS_REST),
+    JS_SELF_HOSTED_FN("bind", "FunctionBind", 2, 0),
     JS_SYM_FN(hasInstance, fun_symbolHasInstance, 1, JSPROP_READONLY | JSPROP_PERMANENT),
     JS_FS_END
 };
diff --git a/js/src/jsfun.h b/js/src/jsfun.h
index 88af5c22d..65da5e06f 100644
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -61,7 +61,6 @@ class JSFunction : public js::NativeObject
                                        function-statement) */
         SELF_HOSTED      = 0x0080,  /* function is self-hosted builtin and must not be
                                        decompilable nor constructible. */
-        HAS_REST         = 0x0100,  /* function has a rest (...) parameter */
         INTERPRETED_LAZY = 0x0200,  /* function is interpreted but doesn't have a script yet */
         RESOLVED_LENGTH  = 0x0400,  /* f.length has been resolved (see fun_resolve). */
         RESOLVED_NAME    = 0x0800,  /* f.name has been resolved (see fun_resolve). */
@@ -95,7 +94,7 @@ class JSFunction : public js::NativeObject
         NO_XDR_FLAGS = RESOLVED_LENGTH | RESOLVED_NAME,
 
         STABLE_ACROSS_CLONES = CONSTRUCTOR | EXPR_BODY | HAS_GUESSED_ATOM | LAMBDA |
-                               SELF_HOSTED |  HAS_REST | FUNCTION_KIND_MASK
+                               SELF_HOSTED | FUNCTION_KIND_MASK
     };
 
     static_assert((INTERPRETED | INTERPRETED_LAZY) == js::JS_FUNCTION_INTERPRETED_BITS,
@@ -183,7 +182,6 @@ class JSFunction : public js::NativeObject
     bool hasGuessedAtom()           const { return flags() & HAS_GUESSED_ATOM; }
     bool isLambda()                 const { return flags() & LAMBDA; }
     bool isBoundFunction()          const { return flags() & BOUND_FUN; }
-    bool hasRest()                  const { return flags() & HAS_REST; }
     bool isInterpretedLazy()        const { return flags() & INTERPRETED_LAZY; }
     bool hasScript()                const { return flags() & INTERPRETED; }
 
@@ -264,11 +262,6 @@ class JSFunction : public js::NativeObject
         this->nargs_ = nargs;
     }
 
-    // Can be called multiple times by the parser.
-    void setHasRest() {
-        flags_ |= HAS_REST;
-    }
-
     void setIsBoundFunction() {
         MOZ_ASSERT(!isBoundFunction());
         flags_ |= BOUND_FUN;
diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp
index b568b4b30..929251d8b 100644
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -316,6 +316,7 @@ js::XDRScript(XDRState<mode>* xdr, HandleScope scriptEnclosingScope, HandleScrip
         IsLegacyGenerator,
         IsStarGenerator,
         IsAsync,
+        HasRest,
         OwnSource,
         ExplicitUseStrict,
         SelfHosted,
@@ -431,6 +432,8 @@ js::XDRScript(XDRState<mode>* xdr, HandleScope scriptEnclosingScope, HandleScrip
             scriptBits |= (1 << IsStarGenerator);
         if (script->asyncKind() == AsyncFunction)
             scriptBits |= (1 << IsAsync);
+        if (script->hasRest())
+            scriptBits |= (1 << HasRest);
         if (script->hasSingletons())
             scriptBits |= (1 << HasSingleton);
         if (script->treatAsRunOnce())
@@ -582,6 +585,8 @@ js::XDRScript(XDRState<mode>* xdr, HandleScope scriptEnclosingScope, HandleScrip
 
         if (scriptBits & (1 << IsAsync))
             script->setAsyncKind(AsyncFunction);
+        if (scriptBits & (1 << HasRest))
+            script->setHasRest();
     }
 
     JS_STATIC_ASSERT(sizeof(jsbytecode) == 1);
@@ -2637,6 +2642,8 @@ JSScript::initFromFunctionBox(ExclusiveContext* cx, HandleScript script,
     script->isGeneratorExp_ = funbox->isGenexpLambda;
     script->setGeneratorKind(funbox->generatorKind());
     script->setAsyncKind(funbox->asyncKind());
+    if (funbox->hasRest())
+        script->setHasRest();
 
     PositionalFormalParameterIter fi(script);
     while (fi && !fi.closedOver())
@@ -3295,6 +3302,7 @@ js::detail::CopyScript(JSContext* cx, HandleScript src, HandleScript dst,
     dst->needsHomeObject_ = src->needsHomeObject();
     dst->isDefaultClassConstructor_ = src->isDefaultClassConstructor();
     dst->isAsync_ = src->asyncKind() == AsyncFunction;
+    dst->hasRest_ = src->hasRest_;
 
     if (nconsts != 0) {
         GCPtrValue* vector = Rebase<GCPtrValue>(dst, src, src->consts()->vector);
@@ -4028,6 +4036,7 @@ LazyScript::Create(ExclusiveContext* cx, HandleFunction fun,
     p.shouldDeclareArguments = false;
     p.hasThisBinding = false;
     p.isAsync = false;
+    p.hasRest = false;
     p.numClosedOverBindings = closedOverBindings.length();
     p.numInnerFunctions = innerFunctions.length();
     p.generatorKindBits = GeneratorKindAsBits(NotGenerator);
@@ -4169,7 +4178,7 @@ JSScript::hasLoops()
 bool
 JSScript::mayReadFrameArgsDirectly()
 {
-    return argumentsHasVarBinding() || (function() && function()->hasRest());
+    return argumentsHasVarBinding() || hasRest();
 }
 
 static inline void
diff --git a/js/src/jsscript.h b/js/src/jsscript.h
index ffd4b1e2e..690bc225d 100644
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -1012,6 +1012,8 @@ class JSScript : public js::gc::TenuredCell
 
     bool isAsync_:1;
 
+    bool hasRest_:1;
+
     // Add padding so JSScript is gc::Cell aligned. Make padding protected
     // instead of private to suppress -Wunused-private-field compiler warnings.
   protected:
@@ -1308,6 +1310,13 @@ class JSScript : public js::gc::TenuredCell
         isAsync_ = kind == js::AsyncFunction;
     }
 
+    bool hasRest() const {
+        return hasRest_;
+    }
+    void setHasRest() {
+        hasRest_ = true;
+    }
+
     void setNeedsHomeObject() {
         needsHomeObject_ = true;
     }
@@ -1940,6 +1949,7 @@ class LazyScript : public gc::TenuredCell
         uint32_t treatAsRunOnce : 1;
         uint32_t isDerivedClassConstructor : 1;
         uint32_t needsHomeObject : 1;
+        uint32_t hasRest : 1;
     };
 
     union {
@@ -2068,6 +2078,13 @@ class LazyScript : public gc::TenuredCell
         p_.isAsync = kind == AsyncFunction;
     }
 
+    bool hasRest() const {
+        return p_.hasRest;
+    }
+    void setHasRest() {
+        p_.hasRest = true;
+    }
+
     bool strict() const {
         return p_.strict;
     }
diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp
index 32a85dc13..7adeed620 100644
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -2894,7 +2894,7 @@ static const JSFunctionSpec string_static_methods[] = {
     JS_INLINABLE_FN("fromCharCode", js::str_fromCharCode, 1, 0, StringFromCharCode),
     JS_INLINABLE_FN("fromCodePoint", js::str_fromCodePoint, 1, 0, StringFromCodePoint),
 
-    JS_SELF_HOSTED_FN("raw",             "String_static_raw",           2,JSFUN_HAS_REST),
+    JS_SELF_HOSTED_FN("raw",             "String_static_raw",           2,0),
     JS_SELF_HOSTED_FN("substring",       "String_static_substring",     3,0),
     JS_SELF_HOSTED_FN("substr",          "String_static_substr",        3,0),
     JS_SELF_HOSTED_FN("slice",           "String_static_slice",         3,0),
diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp
index fbf526ae5..51e809345 100644
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -282,8 +282,6 @@ MakeDefaultConstructor(JSContext* cx, JSOp op, JSAtom* atom, HandleObject proto)
 
     ctor->setIsConstructor();
     ctor->setIsClassConstructor();
-    if (derived)
-        ctor->setHasRest();
 
     MOZ_ASSERT(ctor->infallibleIsDefaultClassConstructor(cx));
 
diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp
index 6737e774c..fd604c6bf 100644
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -3022,7 +3022,7 @@ JSRuntime::cloneSelfHostedFunctionScript(JSContext* cx, HandlePropertyName name,
     MOZ_ASSERT(!targetFun->isInterpretedLazy());
 
     MOZ_ASSERT(sourceFun->nargs() == targetFun->nargs());
-    MOZ_ASSERT(sourceFun->hasRest() == targetFun->hasRest());
+    MOZ_ASSERT(sourceScript->hasRest() == targetFun->nonLazyScript()->hasRest());
 
     // The target function might have been relazified after its flags changed.
     targetFun->setFlags(targetFun->flags() | sourceFun->flags());
diff --git a/js/src/vm/Stack.cpp b/js/src/vm/Stack.cpp
index 439bb1ed4..87e95c893 100644
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -85,7 +85,7 @@ InterpreterFrame::isNonGlobalEvalFrame() const
 JSObject*
 InterpreterFrame::createRestParameter(JSContext* cx)
 {
-    MOZ_ASSERT(callee().hasRest());
+    MOZ_ASSERT(script()->hasRest());
     unsigned nformal = callee().nargs() - 1, nactual = numActualArgs();
     unsigned nrest = (nactual > nformal) ? nactual - nformal : 0;
     Value* restvp = argv() + nformal;
diff --git a/js/src/wasm/AsmJS.cpp b/js/src/wasm/AsmJS.cpp
index 4dbc9b387..d2f331bf1 100644
--- a/js/src/wasm/AsmJS.cpp
+++ b/js/src/wasm/AsmJS.cpp
@@ -3249,7 +3249,7 @@ static bool
 CheckFunctionHead(ModuleValidator& m, ParseNode* fn)
 {
     JSFunction* fun = FunctionObject(fn);
-    if (fun->hasRest())
+    if (fn->pn_funbox->hasRest())
         return m.fail(fn, "rest args not allowed");
     if (fun->isExprBody())
         return m.fail(fn, "expression closures not allowed");