Store JSStrings instead of jsids in native key iterators,
bug 713754. r=dvander
--- a/js/src/jsatominlines.h
+++ b/js/src/jsatominlines.h
@@ -172,21 +172,21 @@ IndexToId(JSContext *cx, uint32_t index,
*idp = INT_TO_JSID(index);
return true;
}
extern bool IndexToIdSlow(JSContext *cx, uint32_t index, jsid *idp);
return IndexToIdSlow(cx, index, idp);
}
-static JS_ALWAYS_INLINE JSString *
+static JS_ALWAYS_INLINE JSFlatString *
IdToString(JSContext *cx, jsid id)
{
if (JSID_IS_STRING(id))
- return JSID_TO_STRING(id);
+ return JSID_TO_ATOM(id);
if (JS_LIKELY(JSID_IS_INT(id)))
- return js_IntToString(cx, JSID_TO_INT(id));
- return js::ToStringSlow(cx, IdToValue(id));
+ return js_IntToString(cx, JSID_TO_INT(id))->ensureFlat(cx);
+ return ToStringSlow(cx, IdToValue(id))->ensureFlat(cx);
}
} // namespace js
#endif /* jsatominlines_h___ */
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -1345,23 +1345,19 @@ IteratorMore(JSContext *cx, JSObject *it
static inline bool
IteratorNext(JSContext *cx, JSObject *iterobj, Value *rval)
{
if (iterobj->isIterator()) {
NativeIterator *ni = iterobj->getNativeIterator();
if (ni->isKeyIter()) {
JS_ASSERT(ni->props_cursor < ni->props_end);
- jsid id = *ni->current();
- if (JSID_IS_ATOM(id)) {
- rval->setString(JSID_TO_STRING(id));
- ni->incCursor();
- return true;
- }
- /* Take the slow path if we have to stringify a numeric property name. */
+ rval->setString(*ni->current());
+ ni->incCursor();
+ return true;
}
}
return js_IteratorNext(cx, iterobj, rval);
}
/*
* For bytecodes which push values and then fall through, make sure the
* types of the pushed values are consistent with type inference information.
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -116,17 +116,18 @@ Class js::IteratorClass = {
}
};
static const gc::AllocKind ITERATOR_FINALIZE_KIND = gc::FINALIZE_OBJECT2;
void
NativeIterator::mark(JSTracer *trc)
{
- MarkIdRange(trc, begin(), end(), "props");
+ for (HeapPtr<JSFlatString> *str = begin(); str < end(); str++)
+ MarkString(trc, *str, "prop");
if (obj)
MarkObject(trc, obj, "obj");
}
static void
iterator_finalize(JSContext *cx, JSObject *obj)
{
JS_ASSERT(obj->isIterator());
@@ -502,24 +503,30 @@ NewIteratorObject(JSContext *cx, uintN f
return NewBuiltinClassInstance(cx, &IteratorClass);
}
NativeIterator *
NativeIterator::allocateIterator(JSContext *cx, uint32_t slength, const AutoIdVector &props)
{
size_t plength = props.length();
NativeIterator *ni = (NativeIterator *)
- cx->malloc_(sizeof(NativeIterator) + plength * sizeof(jsid) + slength * sizeof(Shape *));
+ cx->malloc_(sizeof(NativeIterator)
+ + plength * sizeof(JSString *)
+ + slength * sizeof(Shape *));
if (!ni)
return NULL;
- ni->props_array = ni->props_cursor = (HeapId *) (ni + 1);
+ ni->props_array = ni->props_cursor = (HeapPtr<JSFlatString> *) (ni + 1);
ni->props_end = ni->props_array + plength;
if (plength) {
- for (size_t i = 0; i < plength; i++)
- ni->props_array[i].init(props[i]);
+ for (size_t i = 0; i < plength; i++) {
+ JSFlatString *str = IdToString(cx, props[i]);
+ if (!str)
+ return NULL;
+ ni->props_array[i].init(str);
+ }
}
return ni;
}
inline void
NativeIterator::init(JSObject *obj, uintN flags, uint32_t slength, uint32_t key)
{
this->obj.init(obj);
@@ -911,46 +918,50 @@ js_CloseIterator(JSContext *cx, JSObject
* We do not suppress enumeration of a property deleted along an object's
* prototype chain. Only direct deletions on the object are handled.
*
* This function can suppress multiple properties at once. The |predicate|
* argument is an object which can be called on an id and returns true or
* false. It also must have a method |matchesAtMostOne| which allows us to
* stop searching after the first deletion if true.
*/
-template<typename IdPredicate>
+template<typename StringPredicate>
static bool
-SuppressDeletedPropertyHelper(JSContext *cx, JSObject *obj, IdPredicate predicate)
+SuppressDeletedPropertyHelper(JSContext *cx, JSObject *obj, StringPredicate predicate)
{
JSObject *iterobj = cx->enumerators;
while (iterobj) {
again:
NativeIterator *ni = iterobj->getNativeIterator();
/* This only works for identified surpressed keys, not values. */
if (ni->isKeyIter() && ni->obj == obj && ni->props_cursor < ni->props_end) {
/* Check whether id is still to come. */
- HeapId *props_cursor = ni->current();
- HeapId *props_end = ni->end();
- for (HeapId *idp = props_cursor; idp < props_end; ++idp) {
+ HeapPtr<JSFlatString> *props_cursor = ni->current();
+ HeapPtr<JSFlatString> *props_end = ni->end();
+ for (HeapPtr<JSFlatString> *idp = props_cursor; idp < props_end; ++idp) {
if (predicate(*idp)) {
/*
* Check whether another property along the prototype chain
* became visible as a result of this deletion.
*/
if (obj->getProto()) {
JSObject *proto = obj->getProto();
JSObject *obj2;
JSProperty *prop;
- if (!proto->lookupGeneric(cx, *idp, &obj2, &prop))
+ jsid id;
+ if (!ValueToId(cx, StringValue(*idp), &id))
+ return false;
+ id = js_CheckForStringIndex(id);
+ if (!proto->lookupGeneric(cx, id, &obj2, &prop))
return false;
if (prop) {
uintN attrs;
if (obj2->isNative())
attrs = ((Shape *) prop)->attributes();
- else if (!obj2->getGenericAttributes(cx, *idp, &attrs))
+ else if (!obj2->getGenericAttributes(cx, id, &attrs))
return false;
if (attrs & JSPROP_ENUMERATE)
continue;
}
}
/*
@@ -963,17 +974,17 @@ SuppressDeletedPropertyHelper(JSContext
/*
* No property along the prototype chain stepped in to take the
* property's place, so go ahead and delete id from the list.
* If it is the next property to be enumerated, just skip it.
*/
if (idp == props_cursor) {
ni->incCursor();
} else {
- for (HeapId *p = idp; p + 1 != props_end; p++)
+ for (HeapPtr<JSFlatString> *p = idp; p + 1 != props_end; p++)
*p = *(p + 1);
ni->props_end = ni->end() - 1;
}
/* Don't reuse modified native iterators. */
ni->flags |= JSITER_UNREUSABLE;
if (predicate.matchesAtMostOne())
@@ -981,61 +992,52 @@ SuppressDeletedPropertyHelper(JSContext
}
}
}
iterobj = ni->next;
}
return true;
}
-class SingleIdPredicate {
- jsid id;
+class SingleStringPredicate {
+ JSFlatString *str;
public:
- SingleIdPredicate(jsid id) : id(id) {}
+ SingleStringPredicate(JSFlatString *str) : str(str) {}
- bool operator()(jsid id) { return id == this->id; }
+ bool operator()(JSFlatString *str) { return EqualStrings(str, this->str); }
bool matchesAtMostOne() { return true; }
};
bool
js_SuppressDeletedProperty(JSContext *cx, JSObject *obj, jsid id)
{
- id = js_CheckForStringIndex(id);
- return SuppressDeletedPropertyHelper(cx, obj, SingleIdPredicate(id));
+ JSFlatString *str = IdToString(cx, id);
+ if (!str)
+ return false;
+ return SuppressDeletedPropertyHelper(cx, obj, SingleStringPredicate(str));
}
bool
js_SuppressDeletedElement(JSContext *cx, JSObject *obj, uint32_t index)
{
jsid id;
if (!IndexToId(cx, index, &id))
return false;
- JS_ASSERT(id == js_CheckForStringIndex(id));
- return SuppressDeletedPropertyHelper(cx, obj, SingleIdPredicate(id));
+ return js_SuppressDeletedProperty(cx, obj, id);
}
class IndexRangePredicate {
uint32_t begin, end;
public:
IndexRangePredicate(uint32_t begin, uint32_t end) : begin(begin), end(end) {}
- bool operator()(jsid id) {
- if (JSID_IS_INT(id)) {
- jsint i = JSID_TO_INT(id);
- return i > 0 && begin <= uint32_t(i) && uint32_t(i) < end;
- }
-
- if (JS_LIKELY(JSID_IS_ATOM(id))) {
- JSAtom *atom = JSID_TO_ATOM(id);
- uint32_t index;
- return atom->isIndex(&index) && begin <= index && index < end;
- }
-
- return false;
+ bool operator()(JSFlatString *str) {
+ uint32_t index;
+ return str->isIndex(&index) && begin <= index && index < end;
}
bool matchesAtMostOne() { return false; }
};
bool
js_SuppressDeletedElements(JSContext *cx, JSObject *obj, uint32_t begin, uint32_t end)
{
@@ -1078,17 +1080,20 @@ js_IteratorMore(JSContext *cx, JSObject
cx->clearPendingException();
cx->iterValue.setMagic(JS_NO_ITER_VALUE);
rval->setBoolean(false);
return true;
}
} else {
JS_ASSERT(!ni->isKeyIter());
- jsid id = *ni->current();
+ jsid id;
+ if (!ValueToId(cx, StringValue(*ni->current()), &id))
+ return false;
+ id = js_CheckForStringIndex(id);
ni->incCursor();
if (!ni->obj->getGeneric(cx, id, rval))
return false;
if ((ni->flags & JSITER_KEYVALUE) && !NewKeyValuePair(cx, id, *rval, rval))
return false;
}
/* Cache the value returned by iterobj.next() so js_IteratorNext() can find it. */
@@ -1105,17 +1110,17 @@ js_IteratorNext(JSContext *cx, JSObject
if (iterobj->isIterator()) {
/*
* Implement next directly as all the methods of the native iterator are
* read-only and permanent.
*/
NativeIterator *ni = iterobj->getNativeIterator();
if (ni->isKeyIter()) {
JS_ASSERT(ni->props_cursor < ni->props_end);
- *rval = IdToValue(*ni->current());
+ *rval = StringValue(*ni->current());
ni->incCursor();
if (rval->isString())
return true;
JSString *str;
jsint i;
if (rval->isInt32() && StaticStrings::hasInt(i = rval->toInt32())) {
--- a/js/src/jsiter.h
+++ b/js/src/jsiter.h
@@ -57,40 +57,40 @@
*/
#define JSITER_ACTIVE 0x1000
#define JSITER_UNREUSABLE 0x2000
namespace js {
struct NativeIterator {
HeapPtrObject obj;
- HeapId *props_array;
- HeapId *props_cursor;
- HeapId *props_end;
+ HeapPtr<JSFlatString> *props_array;
+ HeapPtr<JSFlatString> *props_cursor;
+ HeapPtr<JSFlatString> *props_end;
const Shape **shapes_array;
uint32_t shapes_length;
uint32_t shapes_key;
uint32_t flags;
JSObject *next; /* Forms cx->enumerators list, garbage otherwise. */
bool isKeyIter() const { return (flags & JSITER_FOREACH) == 0; }
- inline HeapId *begin() const {
+ inline HeapPtr<JSFlatString> *begin() const {
return props_array;
}
- inline HeapId *end() const {
+ inline HeapPtr<JSFlatString> *end() const {
return props_end;
}
size_t numKeys() const {
return end() - begin();
}
- HeapId *current() const {
+ HeapPtr<JSFlatString> *current() const {
JS_ASSERT(props_cursor < props_end);
return props_cursor;
}
void incCursor() {
props_cursor = props_cursor + 1;
}
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -668,17 +668,21 @@ Reify(JSContext *cx, JSCompartment *orig
*/
size_t length = ni->numKeys();
bool isKeyIter = ni->isKeyIter();
AutoIdVector keys(cx);
if (length > 0) {
if (!keys.resize(length))
return false;
for (size_t i = 0; i < length; ++i) {
- keys[i] = ni->begin()[i];
+ jsid id;
+ if (!ValueToId(cx, StringValue(ni->begin()[i]), &id))
+ return false;
+ id = js_CheckForStringIndex(id);
+ keys[i] = id;
if (!origin->wrapId(cx, &keys[i]))
return false;
}
}
close.clear();
if (!js_CloseIterator(cx, iterObj))
return false;
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -6223,25 +6223,21 @@ mjit::Compiler::iterNext(ptrdiff_t offse
notFast = masm.branchTest32(Assembler::NonZero, T3, Imm32(JSITER_FOREACH));
stubcc.linkExit(notFast, Uses(1));
RegisterID T2 = frame.allocReg();
/* Get cursor. */
masm.loadPtr(Address(T1, offsetof(NativeIterator, props_cursor)), T2);
- /* Test if the jsid is a string. */
+ /* Get the next string in the iterator. */
masm.loadPtr(T2, T3);
- masm.move(T3, T4);
- masm.andPtr(Imm32(JSID_TYPE_MASK), T4);
- notFast = masm.branchTestPtr(Assembler::NonZero, T4, T4);
- stubcc.linkExit(notFast, Uses(1));
/* It's safe to increase the cursor now. */
- masm.addPtr(Imm32(sizeof(jsid)), T2, T4);
+ masm.addPtr(Imm32(sizeof(JSString*)), T2, T4);
masm.storePtr(T4, Address(T1, offsetof(NativeIterator, props_cursor)));
frame.freeReg(T4);
frame.freeReg(T1);
frame.freeReg(T2);
stubcc.leave();
stubcc.masm.move(Imm32(offset), Registers::ArgReg1);