Bug 1463939 - Make NativeIterator::obj_ private, give it a clearer name, and add public accessors to observe or modify it. r=jandem

--HG--
extra : rebase_source : d86fd601a5a3c4ff7c7b8f8668a7ccaeaac70e6e

js/src/jit/BaselineCacheIRCompiler.cpp

@@ -1983,8 +1983,8 @@ BaselineCacheIRCompiler::emitGuardAndGetIterator()
     masm.branchTest32(Assembler::NonZero, Address(niScratch, offsetof(NativeIterator, flags)),
                       Imm32(JSITER_ACTIVE|JSITER_UNREUSABLE), failure->label());
 
-    // Pre-write barrier for store to 'obj'.
-    Address iterObjAddr(niScratch, offsetof(NativeIterator, obj));
+    // Pre-write barrier for store to 'objectBeingIterated_'.
+    Address iterObjAddr(niScratch, NativeIterator::offsetOfObjectBeingIterated());
     EmitPreBarrier(masm, iterObjAddr, MIRType::Object);
 
     // Mark iterator as active.
@@ -1992,7 +1992,7 @@ BaselineCacheIRCompiler::emitGuardAndGetIterator()
     masm.storePtr(obj, iterObjAddr);
     masm.or32(Imm32(JSITER_ACTIVE), iterFlagsAddr);
 
-    // Post-write barrier for stores to 'obj'.
+    // Post-write barrier for stores to 'objectBeingIterated_'.
     emitPostBarrierSlot(output, TypedOrValueRegister(MIRType::Object, AnyRegister(obj)), scratch1);
 
     // Chain onto the active iterator stack. Note that Baseline CacheIR stub

js/src/jit/IonCacheIRCompiler.cpp

@@ -2312,8 +2312,8 @@ IonCacheIRCompiler::emitGuardAndGetIterator()
     masm.branchTest32(Assembler::NonZero, Address(niScratch, offsetof(NativeIterator, flags)),
                       Imm32(JSITER_ACTIVE|JSITER_UNREUSABLE), failure->label());
 
-    // Pre-write barrier for store to 'obj'.
-    Address iterObjAddr(niScratch, offsetof(NativeIterator, obj));
+    // Pre-write barrier for store to 'objectBeingIterated_'.
+    Address iterObjAddr(niScratch, NativeIterator::offsetOfObjectBeingIterated());
     EmitPreBarrier(masm, iterObjAddr, MIRType::Object);
 
     // Mark iterator as active.
@@ -2321,7 +2321,7 @@ IonCacheIRCompiler::emitGuardAndGetIterator()
     masm.storePtr(obj, iterObjAddr);
     masm.or32(Imm32(JSITER_ACTIVE), iterFlagsAddr);
 
-    // Post-write barrier for stores to 'obj'.
+    // Post-write barrier for stores to 'objectBeingIterated_'.
     emitPostBarrierSlot(output, TypedOrValueRegister(MIRType::Object, AnyRegister(obj)), scratch1);
 
     // Chain onto the active iterator stack.

js/src/proxy/CrossCompartmentWrapper.cpp

@@ -283,7 +283,7 @@ Reify(JSContext* cx, JSCompartment* origin, HandleObject objp)
     Rooted<PropertyIteratorObject*> iterObj(cx, &objp->as<PropertyIteratorObject>());
     NativeIterator* ni = iterObj->getNativeIterator();
 
-    RootedObject obj(cx, ni->obj);
+    RootedObject obj(cx, &ni->objectBeingIterated());
     {
         AutoCloseIterator close(cx, iterObj);
 

js/src/vm/Iteration.cpp

@@ -57,7 +57,7 @@ static const gc::AllocKind ITERATOR_FINALIZE_KIND = gc::AllocKind::OBJECT2_BACKG
 void
 NativeIterator::trace(JSTracer* trc)
 {
-    TraceNullableEdge(trc, &obj, "obj");
+    TraceNullableEdge(trc, &objectBeingIterated_, "objectBeingIterated_");
 
     // The SuppressDeletedPropertyHelper loop can GC, so make sure that if the
     // GC removes any elements from the list, it won't remove this one.
@@ -658,7 +658,7 @@ NativeIterator::allocateSentinel(JSContext* maybecx)
 NativeIterator::NativeIterator(JSContext* cx, Handle<PropertyIteratorObject*> propIter,
                                Handle<JSObject*> objBeingIterated, const AutoIdVector& props,
                                uint32_t numGuards, uint32_t guardKey, bool* hadError)
-  : obj(objBeingIterated),
+  : objectBeingIterated_(objBeingIterated),
     iterObj_(propIter),
     // NativeIterator initially acts (before full initialization) as if it
     // contains no guards...
@@ -773,14 +773,6 @@ IteratorHashPolicy::match(PropertyIteratorObject* obj, const Lookup& lookup)
                     ni->guardCount());
 }
 
-static inline void
-UpdateNativeIterator(NativeIterator* ni, JSObject* obj)
-{
-    // Update the object for which the native iterator is associated, so
-    // SuppressDeletedPropertyHelper will recognize the iterator as a match.
-    ni->obj = obj;
-}
-
 static inline bool
 CanCompareIterableObjectToCache(JSObject* obj)
 {
@@ -898,7 +890,7 @@ js::GetIterator(JSContext* cx, HandleObject obj)
     uint32_t numGuards = 0;
     if (PropertyIteratorObject* iterobj = LookupInIteratorCache(cx, obj, &numGuards)) {
         NativeIterator* ni = iterobj->getNativeIterator();
-        UpdateNativeIterator(ni, obj);
+        ni->changeObjectBeingIterated(*obj);
         RegisterEnumerator(ObjectRealm::get(obj), ni);
         return iterobj;
     }
@@ -1253,7 +1245,7 @@ static bool
 SuppressDeletedProperty(JSContext* cx, NativeIterator* ni, HandleObject obj,
                         Handle<JSFlatString*> str)
 {
-    if (ni->obj != obj)
+    if (ni->objectBeingIterated() != *obj)
         return true;
 
     // Optimization for the following common case:

js/src/vm/Iteration.h

@@ -32,11 +32,10 @@ class PropertyIteratorObject;
 
 struct NativeIterator
 {
-  public:
-    // Object being iterated.
-    GCPtrObject obj = {};
-
   private:
+    // Object being iterated.  Non-null except in NativeIterator sentinels.
+    GCPtrObject objectBeingIterated_ = {};
+
     // Internal iterator object.
     JSObject* iterObj_ = nullptr;
 
@@ -88,6 +87,14 @@ struct NativeIterator
     /** Initialize a |JSCompartment::enumerators| sentinel. */
     NativeIterator();
 
+    JSObject& objectBeingIterated() const {
+        return *objectBeingIterated_;
+    }
+
+    void changeObjectBeingIterated(JSObject& obj) {
+        objectBeingIterated_ = &obj;
+    }
+
     HeapReceiverGuard* guardsBegin() const {
         static_assert(alignof(HeapReceiverGuard) <= alignof(NativeIterator),
                       "NativeIterator must be aligned to begin storing "
@@ -143,13 +150,6 @@ struct NativeIterator
         return next_;
     }
 
-    static inline size_t offsetOfNext() {
-        return offsetof(NativeIterator, next_);
-    }
-    static inline size_t offsetOfPrev() {
-        return offsetof(NativeIterator, prev_);
-    }
-
     void incCursor() {
         propertyCursor_++;
     }
@@ -173,6 +173,10 @@ struct NativeIterator
 
     void trace(JSTracer* trc);
 
+    static constexpr size_t offsetOfObjectBeingIterated() {
+        return offsetof(NativeIterator, objectBeingIterated_);
+    }
+
     static constexpr size_t offsetOfGuardsEnd() {
         return offsetof(NativeIterator, guardsEnd_);
     }
@@ -184,6 +188,14 @@ struct NativeIterator
     static constexpr size_t offsetOfPropertiesEnd() {
         return offsetof(NativeIterator, propertiesEnd_);
     }
+
+    static constexpr size_t offsetOfNext() {
+        return offsetof(NativeIterator, next_);
+    }
+
+    static constexpr size_t offsetOfPrev() {
+        return offsetof(NativeIterator, prev_);
+    }
 };
 
 class PropertyIteratorObject : public NativeObject

js/src/vm/JSCompartment-inl.h

@@ -177,7 +177,7 @@ js::ObjectRealm::objectMaybeInIteration(JSObject* obj)
 
     // If the list contains a single object, check if it's |obj|.
     if (next->next() == enumerators)
-        return next->obj == obj;
+        return &next->objectBeingIterated() == obj;
 
     return true;
 }