Bug 1171177 - Remove UNQUALIFIED_VAROBJ Shape flags in favor of Class-checking. (r=luke)
☠☠ backed out by 937562e73564 ☠ ☠
authorShu-yu Guo <shu@rfrn.org>
Fri, 19 Jun 2015 01:21:14 -0700
changeset 280531 b41a69e1b81c4ab38dd1b0291c4bc37aaf2e85b1
parent 280530 04f4fec38c9d25b4ca1bd8760d8754ddc4c23ccb
child 280532 1b15f41920c75dd5dc84c21f21876925b4d3ab9c
push idunknown
push userunknown
push dateunknown
reviewersluke
bugs1171177
milestone41.0a1
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
     };