Bug 1171177 - Remove UNQUALIFIED_VAROBJ Shape flags in favor of Class-checking. (r=luke)
authorShu-yu Guo <shu@rfrn.org>
Sun, 21 Jun 2015 11:49:58 -0700
changeset 280720 cc5d4eaf1a5eb760a369ec619a559cd88f94bc73
parent 280719 177cfe17e0d402f04d85d538bc8c464b76fba702
child 280721 2505945b9d4335204c2901f5f6fc04f0e828494b
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1171177
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1171177 - Remove UNQUALIFIED_VAROBJ Shape flags in favor of Class-checking. (r=luke)
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/vm/GlobalObject.cpp
js/src/vm/ScopeObject.cpp
js/src/vm/Shape.h
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -211,26 +211,45 @@ class JSObject : public js::gc::Cell
     inline bool isBoundFunction() const;
     inline bool hasSpecialEquality() const;
 
     inline bool watched() const;
     bool setWatched(js::ExclusiveContext* cx) {
         return setFlags(cx, js::BaseShape::WATCHED, GENERATE_SHAPE);
     }
 
-    /* See InterpreterFrame::varObj. */
-    inline bool isQualifiedVarObj();
+    // A "qualified" varobj is the object on which "qualified" variable
+    // declarations (i.e., those defined with "var") are kept.
+    //
+    // Conceptually, when a var binding is defined, it is defined on the
+    // innermost qualified varobj on the scope chain.
+    //
+    // Function scopes (CallObjects) are qualified varobjs, and there can be
+    // no other qualified varobj that is more inner for var bindings in that
+    // function. As such, all references to local var bindings in a function
+    // may be statically bound to the function scope. This is subject to
+    // further optimization. Unaliased bindings inside functions reside
+    // entirely on the frame, not in CallObjects.
+    //
+    // Global scopes are also qualified varobjs. It is possible to statically
+    // know, for a given script, that are no more inner qualified varobjs, so
+    // free variable references can be statically bound to the global.
+    //
+    // Finally, there are non-syntactic qualified varobjs used by embedders
+    // (e.g., Gecko and XPConnect), as they often wish to run scripts under a
+    // scope that captures var bindings.
+    inline bool isQualifiedVarObj() const;
     bool setQualifiedVarObj(js::ExclusiveContext* cx) {
         return setFlags(cx, js::BaseShape::QUALIFIED_VAROBJ);
     }
 
-    inline bool isUnqualifiedVarObj();
-    bool setUnqualifiedVarObj(js::ExclusiveContext* cx) {
-        return setFlags(cx, js::BaseShape::UNQUALIFIED_VAROBJ);
-    }
+    // An "unqualified" varobj is the object on which "unqualified"
+    // assignments (i.e., bareword assignments for which the LHS does not
+    // exist on the scope chain) are kept.
+    inline bool isUnqualifiedVarObj() const;
 
     /*
      * Objects with an uncacheable proto can have their prototype mutated
      * without inducing a shape change on the object. Property cache entries
      * and JIT inline caches should not be filled for lookups across prototype
      * lookups on the object.
      */
     inline bool hasUncacheableProto() const;
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -215,34 +215,35 @@ js::DeleteElement(JSContext* cx, HandleO
         return false;
     return DeleteProperty(cx, obj, id, result);
 }
 
 
 /* * */
 
 inline bool
-JSObject::isQualifiedVarObj()
+JSObject::isQualifiedVarObj() const
 {
     if (is<js::DebugScopeObject>())
         return as<js::DebugScopeObject>().scope().isQualifiedVarObj();
-    // TODO: We would like to assert that only GlobalObject or
-    // NonSyntacticVariables object is a qualified varobj, but users of
-    // js::Execute still need to be vetted. See bug 1171177.
-    return hasAllFlags(js::BaseShape::QUALIFIED_VAROBJ);
+    bool rv = hasAllFlags(js::BaseShape::QUALIFIED_VAROBJ);
+    MOZ_ASSERT_IF(rv,
+                  is<js::GlobalObject>() ||
+                  is<js::CallObject>() ||
+                  is<js::NonSyntacticVariablesObject>() ||
+                  (is<js::DynamicWithObject>() && !as<js::DynamicWithObject>().isSyntactic()));
+    return rv;
 }
 
 inline bool
-JSObject::isUnqualifiedVarObj()
+JSObject::isUnqualifiedVarObj() const
 {
     if (is<js::DebugScopeObject>())
         return as<js::DebugScopeObject>().scope().isUnqualifiedVarObj();
-    bool rv = hasAllFlags(js::BaseShape::UNQUALIFIED_VAROBJ);
-    MOZ_ASSERT_IF(rv, is<js::GlobalObject>() || is<js::NonSyntacticVariablesObject>());
-    return rv;
+    return is<js::GlobalObject>() || is<js::NonSyntacticVariablesObject>();
 }
 
 namespace js {
 
 inline bool
 ClassCanHaveFixedData(const Class* clasp)
 {
     // Normally, the number of fixed slots given an object is the maximum
--- a/js/src/vm/GlobalObject.cpp
+++ b/js/src/vm/GlobalObject.cpp
@@ -242,28 +242,27 @@ GlobalObject::createInternal(JSContext* 
     MOZ_ASSERT(clasp->flags & JSCLASS_IS_GLOBAL);
     MOZ_ASSERT(clasp->trace == JS_GlobalObjectTraceHook);
 
     JSObject* obj = NewObjectWithGivenProto(cx, clasp, nullptr, SingletonObject);
     if (!obj)
         return nullptr;
 
     Rooted<GlobalObject*> global(cx, &obj->as<GlobalObject>());
+    MOZ_ASSERT(global->isUnqualifiedVarObj());
 
     // Initialize the private slot to null if present, as GC can call class
     // hooks before the caller gets to set this to a non-garbage value.
     if (clasp->flags & JSCLASS_HAS_PRIVATE)
         global->setPrivate(nullptr);
 
     cx->compartment()->initGlobal(*global);
 
     if (!global->setQualifiedVarObj(cx))
         return nullptr;
-    if (!global->setUnqualifiedVarObj(cx))
-        return nullptr;
     if (!global->setDelegate(cx))
         return nullptr;
 
     return global;
 }
 
 GlobalObject*
 GlobalObject::new_(JSContext* cx, const Class* clasp, JSPrincipals* principals,
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -574,22 +574,20 @@ const Class StaticNonSyntacticScopeObjec
 NonSyntacticVariablesObject::create(JSContext* cx, Handle<GlobalObject*> global)
 {
     Rooted<NonSyntacticVariablesObject*> obj(cx,
         NewObjectWithNullTaggedProto<NonSyntacticVariablesObject>(cx, TenuredObject,
                                                                   BaseShape::DELEGATE));
     if (!obj)
         return nullptr;
 
+    MOZ_ASSERT(obj->isUnqualifiedVarObj());
     if (!obj->setQualifiedVarObj(cx))
         return nullptr;
 
-    if (!obj->setUnqualifiedVarObj(cx))
-        return nullptr;
-
     obj->setEnclosingScope(global);
     return obj;
 }
 
 const Class NonSyntacticVariablesObject::class_ = {
     "NonSyntacticVariablesObject",
     JSCLASS_HAS_RESERVED_SLOTS(NonSyntacticVariablesObject::RESERVED_SLOTS) |
     JSCLASS_IS_ANONYMOUS
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -348,26 +348,20 @@ class BaseShape : public gc::TenuredCell
         BOUND_FUNCTION      =   0x40,
         HAD_ELEMENTS_ACCESS =   0x80,
         WATCHED             =  0x100,
         ITERATED_SINGLETON  =  0x200,
         NEW_GROUP_UNKNOWN   =  0x400,
         UNCACHEABLE_PROTO   =  0x800,
         IMMUTABLE_PROTOTYPE = 0x1000,
 
-        // These two flags control which scope a new variables ends up on in the
-        // scope chain. If the variable is "qualified" (i.e., if it was defined
-        // using var, let, or const) then it ends up on the lowest scope in the
-        // chain that has the QUALIFIED_VAROBJ flag set. If it's "unqualified"
-        // (i.e., if it was introduced without any var, let, or const, which
-        // incidentally is an error in strict mode) then it goes on the lowest
-        // scope in the chain with the UNQUALIFIED_VAROBJ flag set (which is
-        // typically the global).
+        // See JSObject::isQualifiedVarObj().
         QUALIFIED_VAROBJ    = 0x2000,
-        UNQUALIFIED_VAROBJ  = 0x4000,
+
+        // 0x4000 is unused.
 
         // For a function used as an interpreted constructor, whether a 'new'
         // type had constructor information cleared.
         NEW_SCRIPT_CLEARED  = 0x8000,
 
         OBJECT_FLAG_MASK    = 0xfff8
     };