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 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
     };