From b2adf364d17f02b66c2bde8af5065c724e97a17e Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Mon, 11 Mar 2013 15:50:01 -0600 Subject: [PATCH] Bug 849420 - Use MaybeRooted instead of Shape::AutoRooter, r=sfink. --- js/src/frontend/Parser.cpp | 3 +-- js/src/gc/RootMarking.cpp | 13 ------------- js/src/jsapi.cpp | 8 ++++---- js/src/jsapi.h | 1 - js/src/jsdbgapi.cpp | 3 +-- js/src/jsiter.cpp | 3 +-- js/src/jsobj.cpp | 6 +++--- js/src/jsopcode.cpp | 5 ++--- js/src/vm/ObjectImpl.cpp | 2 +- js/src/vm/ScopeObject.cpp | 8 ++++---- js/src/vm/Shape.cpp | 2 +- js/src/vm/Shape.h | 39 +++++++++++--------------------------- 12 files changed, 29 insertions(+), 64 deletions(-) diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index bcc89584cc4..fcbc64cecab 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -2340,8 +2340,7 @@ static inline bool ForEachLetDef(JSContext *cx, ParseContext *pc, HandleStaticBlockObject blockObj, Op op) { - for (Shape::Range r = blockObj->lastProperty()->all(); !r.empty(); r.popFront()) { - Shape::Range::AutoRooter rooter(cx, &r); + for (Shape::Range r(cx, blockObj->lastProperty()); !r.empty(); r.popFront()) { Shape &shape = r.front(); /* Beware the destructuring dummy slots. */ diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index d4a1a72e581..1bdd95e7c79 100644 --- a/js/src/gc/RootMarking.cpp +++ b/js/src/gc/RootMarking.cpp @@ -538,12 +538,6 @@ AutoGCRooter::trace(JSTracer *trc) return; } - case SHAPERANGE: { - Shape::Range::AutoRooter *rooter = static_cast(this); - rooter->trace(trc); - return; - } - case STACKSHAPE: { StackShape::AutoRooter *rooter = static_cast(this); if (rooter->shape->base) @@ -649,13 +643,6 @@ AutoGCRooter::traceAllWrappers(JSTracer *trc) } } -void -Shape::Range::AutoRooter::trace(JSTracer *trc) -{ - if (r->cursor) - MarkShapeRoot(trc, const_cast(&r->cursor), "Shape::Range::AutoRooter"); -} - void RegExpStatics::AutoRooter::trace(JSTracer *trc) { diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 3a434821a69..9aebde593a2 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4396,8 +4396,8 @@ JS_DeleteProperty(JSContext *cx, JSObject *objArg, const char *name) static RawShape LastConfigurableShape(JSObject *obj) { - for (Shape::Range r(obj->lastProperty()->all()); !r.empty(); r.popFront()) { - RawShape shape = &r.front(); + for (Shape::Range r(obj->lastProperty()); !r.empty(); r.popFront()) { + Shape *shape = &r.front(); if (shape->configurable()) return shape; } @@ -4425,8 +4425,8 @@ JS_ClearNonGlobalObject(JSContext *cx, JSObject *objArg) } /* Set all remaining writable plain data properties to undefined. */ - for (Shape::Range r(obj->lastProperty()->all()); !r.empty(); r.popFront()) { - RawShape shape = &r.front(); + for (Shape::Range r(obj->lastProperty()); !r.empty(); r.popFront()) { + Shape *shape = &r.front(); if (shape->isDataDescriptor() && shape->writable() && shape->hasDefaultSetter() && diff --git a/js/src/jsapi.h b/js/src/jsapi.h index a1685ee7d5e..75b7cd7c1f5 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -129,7 +129,6 @@ class JS_PUBLIC_API(AutoGCRooter) { STRINGVECTOR =-17, /* js::AutoStringVector */ SCRIPTVECTOR =-18, /* js::AutoScriptVector */ PROPDESC = -19, /* js::PropDesc::AutoRooter */ - SHAPERANGE = -20, /* js::Shape::Range::AutoRooter */ STACKSHAPE = -21, /* js::StackShape::AutoRooter */ STACKBASESHAPE=-22,/* js::StackBaseShape::AutoRooter */ GETTERSETTER =-24, /* js::AutoRooterGetterSetter */ diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 786b4ab2721..946d7781d0c 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -728,8 +728,7 @@ JS_GetPropertyDescArray(JSContext *cx, JSObject *obj_, JSPropertyDescArray *pda) return false; { - Shape::Range r(obj->lastProperty()->all()); - Shape::Range::AutoRooter rooter(cx, &r); + Shape::Range r(cx, obj->lastProperty()); RootedShape shape(cx); for (; !r.empty(); r.popFront()) { pd[i].id = JSVAL_NULL; diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index cd1bbe0c211..59a0a08d1e9 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -142,8 +142,7 @@ EnumerateNativeProperties(JSContext *cx, HandleObject pobj, unsigned flags, IdSe size_t initialLength = props->length(); /* Collect all unique properties from this object's scope. */ - Shape::Range r = pobj->lastProperty()->all(); - Shape::Range::AutoRooter root(cx, &r); + Shape::Range r(pobj->lastProperty()); for (; !r.empty(); r.popFront()) { Shape &shape = r.front(); diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 53989e54f78..2bf41409a4d 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -1080,7 +1080,7 @@ JSObject::sealOrFreeze(JSContext *cx, HandleObject obj, ImmutabilityType it) /* Get an in order list of the shapes in this object. */ AutoShapeVector shapes(cx); - for (Shape::Range r = obj->lastProperty()->all(); !r.empty(); r.popFront()) { + for (Shape::Range r(obj->lastProperty()); !r.empty(); r.popFront()) { if (!shapes.append(&r.front())) return false; } @@ -1632,7 +1632,7 @@ JS_CopyPropertiesFrom(JSContext *cx, JSObject *targetArg, JSObject *objArg) return true; AutoShapeVector shapes(cx); - for (Shape::Range r(obj->lastProperty()); !r.empty(); r.popFront()) { + for (Shape::Range r(obj->lastProperty()); !r.empty(); r.popFront()) { if (!shapes.append(&r.front())) return false; } @@ -4943,7 +4943,7 @@ JSObject::dump() if (obj->isNative()) { fprintf(stderr, "properties:\n"); Vector props; - for (Shape::Range r = obj->lastProperty()->all(); !r.empty(); r.popFront()) + for (Shape::Range r(obj->lastProperty()); !r.empty(); r.popFront()) props.append(&r.front()); for (size_t i = props.length(); i-- != 0;) DumpProperty(obj, *props[i]); diff --git a/js/src/jsopcode.cpp b/js/src/jsopcode.cpp index ee9d8e4a039..01084abe64e 100644 --- a/js/src/jsopcode.cpp +++ b/js/src/jsopcode.cpp @@ -475,8 +475,7 @@ ToDisassemblySource(JSContext *cx, jsval v, JSAutoByteString *bytes) if (!source) return false; - Shape::Range r = obj->lastProperty()->all(); - Shape::Range::AutoRooter root(cx, &r); + Shape::Range r(cx, obj->lastProperty()); while (!r.empty()) { Rooted shape(cx, &r.front()); @@ -1429,7 +1428,7 @@ ExpressionDecompiler::findLetVar(jsbytecode *pc, unsigned depth) uint32_t blockDepth = block.stackDepth(); uint32_t blockCount = block.slotCount(); if (uint32_t(depth - blockDepth) < uint32_t(blockCount)) { - for (Shape::Range r(block.lastProperty()); !r.empty(); r.popFront()) { + for (Shape::Range r(block.lastProperty()); !r.empty(); r.popFront()) { const Shape &shape = r.front(); if (shape.shortid() == int(depth - blockDepth)) return JSID_TO_ATOM(shape.propid()); diff --git a/js/src/vm/ObjectImpl.cpp b/js/src/vm/ObjectImpl.cpp index ea4aec93688..b789ad89b6e 100644 --- a/js/src/vm/ObjectImpl.cpp +++ b/js/src/vm/ObjectImpl.cpp @@ -232,7 +232,7 @@ js::ObjectImpl::checkShapeConsistency() if (shape->hasTable()) { ShapeTable &table = shape->table(); MOZ_ASSERT(shape->parent); - for (Shape::Range r(shape); !r.empty(); r.popFront()) { + for (Shape::Range r(shape); !r.empty(); r.popFront()) { Shape **spp = table.search(r.front().propid(), false); MOZ_ASSERT(SHAPE_FETCH(spp) == &r.front()); } diff --git a/js/src/vm/ScopeObject.cpp b/js/src/vm/ScopeObject.cpp index 01f0087cc2e..ae1db28e521 100644 --- a/js/src/vm/ScopeObject.cpp +++ b/js/src/vm/ScopeObject.cpp @@ -121,7 +121,7 @@ js::ScopeCoordinateToStaticScopeShape(JSContext *cx, JSScript *script, jsbytecod PropertyName * js::ScopeCoordinateName(JSContext *cx, JSScript *script, jsbytecode *pc) { - Shape::Range r = ScopeCoordinateToStaticScopeShape(cx, script, pc)->all(); + Shape::Range r(ScopeCoordinateToStaticScopeShape(cx, script, pc)); ScopeCoordinate sc(pc); while (r.front().slot() != sc.slot) r.popFront(); @@ -787,8 +787,8 @@ js::XDRStaticBlockObject(XDRState *xdr, HandleObject enclosingScope, Handl if (!shapes.growBy(count)) return false; - for (Shape::Range r(obj->lastProperty()); !r.empty(); r.popFront()) { - RawShape shape = &r.front(); + for (Shape::Range r(obj->lastProperty()); !r.empty(); r.popFront()) { + Shape *shape = &r.front(); shapes[shape->shortid()] = shape; } @@ -844,7 +844,7 @@ js::CloneStaticBlockObject(JSContext *cx, HandleObject enclosingScope, HandleslotCount())) return NULL; - for (Shape::Range r = srcBlock->lastProperty()->all(); !r.empty(); r.popFront()) + for (Shape::Range r(srcBlock->lastProperty()); !r.empty(); r.popFront()) shapes[r.front().shortid()] = &r.front(); for (Shape **p = shapes.begin(); p != shapes.end(); ++p) { diff --git a/js/src/vm/Shape.cpp b/js/src/vm/Shape.cpp index 55e357edf90..19b905ddf68 100644 --- a/js/src/vm/Shape.cpp +++ b/js/src/vm/Shape.cpp @@ -62,7 +62,7 @@ ShapeTable::init(JSRuntime *rt, RawShape lastProp) return false; hashShift = HASH_BITS - sizeLog2; - for (Shape::Range r = lastProp->all(); !r.empty(); r.popFront()) { + for (Shape::Range r(lastProp); !r.empty(); r.popFront()) { Shape &shape = r.front(); Shape **spp = search(shape.propid(), true); diff --git a/js/src/vm/Shape.h b/js/src/vm/Shape.h index a5e841049c1..b750dc32ebc 100644 --- a/js/src/vm/Shape.h +++ b/js/src/vm/Shape.h @@ -545,15 +545,21 @@ class Shape : public js::gc::Cell return parent; } + template class Range { protected: friend class Shape; - /* |cursor| is rooted manually when necessary using Range::AutoRooter. */ - RawShape cursor; + typename MaybeRooted::RootType cursor; public: - Range(RawShape shape) : cursor(shape) { } + Range(JSContext *cx, Shape *shape) : cursor(cx, shape) { + JS_STATIC_ASSERT(allowGC == CanGC); + } + + Range(Shape *shape) : cursor(NULL, shape) { + JS_STATIC_ASSERT(allowGC == NoGC); + } bool empty() const { return !cursor || cursor->isEmptyShape(); @@ -568,31 +574,8 @@ class Shape : public js::gc::Cell JS_ASSERT(!empty()); cursor = cursor->parent; } - - class AutoRooter : private AutoGCRooter - { - public: - explicit AutoRooter(JSContext *cx, Range *r_ - MOZ_GUARD_OBJECT_NOTIFIER_PARAM) - : AutoGCRooter(cx, SHAPERANGE), r(r_), skip(cx, r_) - { - MOZ_GUARD_OBJECT_NOTIFIER_INIT; - } - - friend void AutoGCRooter::trace(JSTracer *trc); - void trace(JSTracer *trc); - - private: - Range *r; - SkipRoot skip; - MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER - }; }; - Range all() { - return Range(this); - } - Class *getObjectClass() const { return base()->clasp; } JSObject *getObjectParent() const { return base()->parent; } @@ -798,7 +781,7 @@ class Shape : public js::gc::Cell RawShape shape = this; uint32_t count = 0; - for (Shape::Range r = shape->all(); !r.empty(); r.popFront()) + for (Shape::Range r(shape); !r.empty(); r.popFront()) ++count; return count; } @@ -807,7 +790,7 @@ class Shape : public js::gc::Cell JS_ASSERT(!hasTable()); RawShape shape = this; uint32_t count = 0; - for (Shape::Range r = shape->all(); !r.empty(); r.popFront()) { + for (Shape::Range r(shape); !r.empty(); r.popFront()) { ++count; if (count >= ShapeTable::MIN_ENTRIES) return true;