Bug 1017067 - Merge deleteProperty/deleteElement ops back into a single deleteGeneric op. r=bhackett.

This commit is contained in:
Jason Orendorff
2014-06-05 13:19:23 -04:00
parent 01dce61b91
commit 201acb0e94
18 changed files with 86 additions and 179 deletions

View File

@@ -377,8 +377,7 @@ JS_NULL_OBJECT_OPS
nullptr, /* setElement */
nullptr, /* getGenericAttributes */
nullptr, /* setGenericAttributes */
nullptr, /* deleteProperty */
nullptr, /* deleteElement */
nullptr, /* deleteGeneric */
nullptr, /* watch */
nullptr, /* unwatch */
nullptr, /* slice */

View File

@@ -232,10 +232,7 @@ typedef bool
(* PropertyAttributesOp)(JSContext *cx, JS::HandleObject obj, JS::Handle<PropertyName*> name,
unsigned *attrsp);
typedef bool
(* DeletePropertyOp)(JSContext *cx, JS::HandleObject obj, JS::Handle<PropertyName*> name,
bool *succeeded);
typedef bool
(* DeleteElementOp)(JSContext *cx, JS::HandleObject obj, uint32_t index, bool *succeeded);
(* DeleteGenericOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id, bool *succeeded);
typedef bool
(* WatchOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id, JS::HandleObject callable);
@@ -342,20 +339,18 @@ struct ObjectOps
StrictElementIdOp setElement;
GenericAttributesOp getGenericAttributes;
GenericAttributesOp setGenericAttributes;
DeletePropertyOp deleteProperty;
DeleteElementOp deleteElement;
DeleteGenericOp deleteGeneric;
WatchOp watch;
UnwatchOp unwatch;
SliceOp slice; // Optimized slice, can be null.
JSNewEnumerateOp enumerate;
ObjectOp thisObject;
};
#define JS_NULL_OBJECT_OPS \
{nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr, \
nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr,nullptr, \
nullptr,nullptr}
{nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, \
nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, \
nullptr, nullptr, nullptr}
} // namespace js
@@ -366,7 +361,7 @@ typedef void (*JSClassInternal)();
struct JSClass {
JS_CLASS_MEMBERS(JSFinalizeOp);
void *reserved[32];
void *reserved[31];
};
#define JSCLASS_HAS_PRIVATE (1<<0) // objects have private slot

View File

@@ -2093,10 +2093,8 @@ TypedObject::obj_setGenericAttributes(JSContext *cx, HandleObject obj,
}
bool
TypedObject::obj_deleteProperty(JSContext *cx, HandleObject obj,
HandlePropertyName name, bool *succeeded)
TypedObject::obj_deleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded)
{
Rooted<jsid> id(cx, NameToId(name));
if (IsOwnId(cx, obj, id))
return ReportPropertyError(cx, JSMSG_CANT_DELETE, id);
@@ -2106,27 +2104,7 @@ TypedObject::obj_deleteProperty(JSContext *cx, HandleObject obj,
return true;
}
return JSObject::deleteProperty(cx, proto, name, succeeded);
}
bool
TypedObject::obj_deleteElement(JSContext *cx, HandleObject obj, uint32_t index,
bool *succeeded)
{
RootedId id(cx);
if (!IndexToId(cx, index, &id))
return false;
if (IsOwnId(cx, obj, id))
return ReportPropertyError(cx, JSMSG_CANT_DELETE, id);
RootedObject proto(cx, obj->getProto());
if (!proto) {
*succeeded = false;
return true;
}
return JSObject::deleteElement(cx, proto, index, succeeded);
return JSObject::deleteGeneric(cx, proto, id, succeeded);
}
bool
@@ -2287,8 +2265,7 @@ const Class TransparentTypedObject::class_ = {
TypedObject::obj_setElement,
TypedObject::obj_getGenericAttributes,
TypedObject::obj_setGenericAttributes,
TypedObject::obj_deleteProperty,
TypedObject::obj_deleteElement,
TypedObject::obj_deleteGeneric,
nullptr, nullptr, // watch/unwatch
nullptr, /* slice */
TypedObject::obj_enumerate,
@@ -2633,8 +2610,7 @@ const Class OpaqueTypedObject::class_ = {
TypedObject::obj_setElement,
TypedObject::obj_getGenericAttributes,
TypedObject::obj_setGenericAttributes,
TypedObject::obj_deleteProperty,
TypedObject::obj_deleteElement,
TypedObject::obj_deleteGeneric,
nullptr, nullptr, // watch/unwatch
nullptr, // slice
TypedObject::obj_enumerate,

View File

@@ -576,10 +576,7 @@ class TypedObject : public ArrayBufferViewObject
static bool obj_setGenericAttributes(JSContext *cx, HandleObject obj,
HandleId id, unsigned *attrsp);
static bool obj_deleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name,
bool *succeeded);
static bool obj_deleteElement(JSContext *cx, HandleObject obj, uint32_t index,
bool *succeeded);
static bool obj_deleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded);
static bool obj_enumerate(JSContext *cx, HandleObject obj, JSIterateOp enum_op,
MutableHandleValue statep, MutableHandleId idp);

View File

@@ -3631,7 +3631,7 @@ JS_DeletePropertyById2(JSContext *cx, HandleObject obj, HandleId id, bool *resul
CHECK_REQUEST(cx);
assertSameCompartment(cx, obj, id);
return JSObject::deleteByValue(cx, obj, IdToValue(id), result);
return JSObject::deleteGeneric(cx, obj, id, result);
}
JS_PUBLIC_API(bool)
@@ -3653,7 +3653,8 @@ JS_DeleteProperty2(JSContext *cx, HandleObject obj, const char *name, bool *resu
JSAtom *atom = Atomize(cx, name, strlen(name));
if (!atom)
return false;
return JSObject::deleteByValue(cx, obj, StringValue(atom), result);
RootedId id(cx, AtomToId(atom));
return JSObject::deleteGeneric(cx, obj, id, result);
}
JS_PUBLIC_API(bool)
@@ -3666,7 +3667,8 @@ JS_DeleteUCProperty2(JSContext *cx, HandleObject obj, const jschar *name, size_t
JSAtom *atom = AtomizeChars(cx, name, AUTO_NAMELEN(name, namelen));
if (!atom)
return false;
return JSObject::deleteByValue(cx, obj, StringValue(atom), result);
RootedId id(cx, AtomToId(atom));
return JSObject::deleteGeneric(cx, obj, id, result);
}
JS_PUBLIC_API(bool)

View File

@@ -337,10 +337,10 @@ DeleteArrayElement(JSContext *cx, HandleObject obj, double index, bool *succeede
return true;
}
if (index <= UINT32_MAX)
return JSObject::deleteElement(cx, obj, uint32_t(index), succeeded);
return JSObject::deleteByValue(cx, obj, DoubleValue(index), succeeded);
RootedId id(cx);
if (!ToId(cx, index, &id))
return false;
return JSObject::deleteGeneric(cx, obj, id, succeeded);
}
/* ES6 20130308 draft 9.3.5 */

View File

@@ -283,8 +283,7 @@ namespace js {
js::proxy_SetElement, \
js::proxy_GetGenericAttributes, \
js::proxy_SetGenericAttributes, \
js::proxy_DeleteProperty, \
js::proxy_DeleteElement, \
js::proxy_DeleteGeneric, \
js::proxy_Watch, js::proxy_Unwatch, \
js::proxy_Slice, \
nullptr, /* enumerate */ \
@@ -349,10 +348,7 @@ proxy_GetGenericAttributes(JSContext *cx, JS::HandleObject obj, JS::HandleId id,
extern JS_FRIEND_API(bool)
proxy_SetGenericAttributes(JSContext *cx, JS::HandleObject obj, JS::HandleId id, unsigned *attrsp);
extern JS_FRIEND_API(bool)
proxy_DeleteProperty(JSContext *cx, JS::HandleObject obj, JS::Handle<PropertyName*> name,
bool *succeeded);
extern JS_FRIEND_API(bool)
proxy_DeleteElement(JSContext *cx, JS::HandleObject obj, uint32_t index, bool *succeeded);
proxy_DeleteGeneric(JSContext *cx, JS::HandleObject obj, JS::HandleId id, bool *succeeded);
extern JS_FRIEND_API(void)
proxy_Trace(JSTracer *trc, JSObject *obj);

View File

@@ -1819,26 +1819,6 @@ JSObject::nonNativeSetElement(JSContext *cx, HandleObject obj,
return obj->getOps()->setElement(cx, obj, index, vp, strict);
}
/* static */ bool
JSObject::deleteByValue(JSContext *cx, HandleObject obj, const Value &property, bool *succeeded)
{
uint32_t index;
if (IsDefinitelyIndex(property, &index))
return deleteElement(cx, obj, index, succeeded);
RootedValue propval(cx, property);
JSAtom *name = ToAtom<CanGC>(cx, propval);
if (!name)
return false;
if (name->isIndex(&index))
return deleteElement(cx, obj, index, succeeded);
Rooted<PropertyName*> propname(cx, name->asPropertyName());
return deleteProperty(cx, obj, propname, succeeded);
}
JS_FRIEND_API(bool)
JS_CopyPropertyFrom(JSContext *cx, HandleId id, HandleObject target,
HandleObject obj)
@@ -2716,7 +2696,8 @@ DefineConstructorAndPrototype(JSContext *cx, HandleObject obj, JSProtoKey key, H
bad:
if (named) {
bool succeeded;
JSObject::deleteByValue(cx, obj, StringValue(atom), &succeeded);
RootedId id(cx, AtomToId(atom));
JSObject::deleteGeneric(cx, obj, id, &succeeded);
}
if (cached)
ClearClassObject(obj, key);
@@ -5312,23 +5293,6 @@ baseops::DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succe
return obj->removeProperty(cx, id) && js_SuppressDeletedProperty(cx, obj, id);
}
bool
baseops::DeleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name,
bool *succeeded)
{
Rooted<jsid> id(cx, NameToId(name));
return baseops::DeleteGeneric(cx, obj, id, succeeded);
}
bool
baseops::DeleteElement(JSContext *cx, HandleObject obj, uint32_t index, bool *succeeded)
{
RootedId id(cx);
if (!IndexToId(cx, index, &id))
return false;
return baseops::DeleteGeneric(cx, obj, id, succeeded);
}
bool
js::WatchGuts(JSContext *cx, JS::HandleObject origObj, JS::HandleId id, JS::HandleObject callable)
{

View File

@@ -148,12 +148,6 @@ GetAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp);
extern bool
SetAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned *attrsp);
extern bool
DeleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name, bool *succeeded);
extern bool
DeleteElement(JSContext *cx, HandleObject obj, uint32_t index, bool *succeeded);
extern bool
DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded);
@@ -1054,13 +1048,10 @@ class JSObject : public js::ObjectImpl
static inline bool setGenericAttributes(JSContext *cx, js::HandleObject obj,
js::HandleId id, unsigned *attrsp);
static inline bool deleteProperty(JSContext *cx, js::HandleObject obj,
js::HandlePropertyName name,
static inline bool deleteGeneric(JSContext *cx, js::HandleObject obj, js::HandleId id,
bool *succeeded);
static inline bool deleteElement(JSContext *cx, js::HandleObject obj, uint32_t index,
bool *succeeded);
static inline bool deleteElement(JSContext *cx, js::HandleObject obj,
uint32_t index, bool *succeeded);
static bool deleteByValue(JSContext *cx, js::HandleObject obj,
const js::Value &property, bool *succeeded);
static inline bool watch(JSContext *cx, JS::HandleObject obj, JS::HandleId id,
JS::HandleObject callable);

View File

@@ -41,13 +41,12 @@ JSObject::changePropertyAttributes(JSContext *cx, js::HandleObject obj,
}
/* static */ inline bool
JSObject::deleteProperty(JSContext *cx, js::HandleObject obj, js::HandlePropertyName name,
JSObject::deleteGeneric(JSContext *cx, js::HandleObject obj, js::HandleId id,
bool *succeeded)
{
JS::RootedId id(cx, js::NameToId(name));
js::types::MarkTypePropertyNonData(cx, obj, id);
js::DeletePropertyOp op = obj->getOps()->deleteProperty;
return (op ? op : js::baseops::DeleteProperty)(cx, obj, name, succeeded);
js::DeleteGenericOp op = obj->getOps()->deleteGeneric;
return (op ? op : js::baseops::DeleteGeneric)(cx, obj, id, succeeded);
}
/* static */ inline bool
@@ -56,9 +55,7 @@ JSObject::deleteElement(JSContext *cx, js::HandleObject obj, uint32_t index, boo
JS::RootedId id(cx);
if (!js::IndexToId(cx, index, &id))
return false;
js::types::MarkTypePropertyNonData(cx, obj, id);
js::DeleteElementOp op = obj->getOps()->deleteElement;
return (op ? op : js::baseops::DeleteElement)(cx, obj, index, succeeded);
return deleteGeneric(cx, obj, id, succeeded);
}
/* static */ inline bool

View File

@@ -698,7 +698,7 @@ Walk(JSContext *cx, HandleObject holder, HandleId name, HandleValue reviver, Mut
if (newElement.isUndefined()) {
/* Step 2a(iii)(2). */
bool succeeded;
if (!JSObject::deleteByValue(cx, obj, IdToValue(id), &succeeded))
if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
return false;
} else {
/* Step 2a(iii)(3). */
@@ -726,7 +726,7 @@ Walk(JSContext *cx, HandleObject holder, HandleId name, HandleValue reviver, Mut
if (newElement.isUndefined()) {
/* Step 2b(ii)(2). */
bool succeeded;
if (!JSObject::deleteByValue(cx, obj, IdToValue(id), &succeeded))
if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
return false;
} else {
/* Step 2b(ii)(3). */

View File

@@ -427,7 +427,7 @@ DirectProxyHandler::delete_(JSContext *cx, HandleObject proxy, HandleId id, bool
{
assertEnteredPolicy(cx, proxy, id, SET);
RootedObject target(cx, proxy->as<ProxyObject>().target());
return JS_DeletePropertyById2(cx, target, id, bp);
return JSObject::deleteGeneric(cx, target, id, bp);
}
bool
@@ -2920,8 +2920,8 @@ js::proxy_SetGenericAttributes(JSContext *cx, HandleObject obj, HandleId id, uns
return Proxy::defineProperty(cx, obj, id, &desc);
}
static bool
proxy_DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded)
bool
js::proxy_DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded)
{
bool deleted;
if (!Proxy::delete_(cx, obj, id, &deleted))
@@ -2930,22 +2930,6 @@ proxy_DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeede
return js_SuppressDeletedProperty(cx, obj, id);
}
bool
js::proxy_DeleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name, bool *succeeded)
{
RootedId id(cx, NameToId(name));
return proxy_DeleteGeneric(cx, obj, id, succeeded);
}
bool
js::proxy_DeleteElement(JSContext *cx, HandleObject obj, uint32_t index, bool *succeeded)
{
RootedId id(cx);
if (!IndexToId(cx, index, &id))
return false;
return proxy_DeleteGeneric(cx, obj, id, succeeded);
}
void
js::proxy_Trace(JSTracer *trc, JSObject *obj)
{

View File

@@ -0,0 +1,21 @@
/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/*
* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/licenses/publicdomain/
*/
// delete o[p] only performs ToString(p) once, even if there's a strict error.
var hits = 0;
var p = {
toString: function () {
hits++;
return "noconfig";
}
};
assertEq(testLenientAndStrict('var o = Object.freeze({noconfig: "ow"}); delete o[p]',
returns(false), raisesException(TypeError)),
true);
assertEq(hits, 2);
reportCompare(true, true);

View File

@@ -5417,16 +5417,18 @@ static bool
DebuggerObject_deleteProperty(JSContext *cx, unsigned argc, Value *vp)
{
THIS_DEBUGOBJECT_OWNER_REFERENT(cx, argc, vp, "deleteProperty", args, dbg, obj);
RootedValue nameArg(cx, args.get(0));
RootedId id(cx);
if (!ValueToId<CanGC>(cx, args.get(0), &id))
return false;
Maybe<AutoCompartment> ac;
ac.construct(cx, obj);
if (!cx->compartment()->wrap(cx, &nameArg))
if (!cx->compartment()->wrapId(cx, id.address()))
return false;
bool succeeded;
ErrorCopier ec(ac, dbg->toJSObject());
if (!JSObject::deleteByValue(cx, obj, nameArg, &succeeded))
if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
return false;
args.rval().setBoolean(succeeded);
return true;

View File

@@ -2312,17 +2312,17 @@ END_CASE(JSOP_DELNAME)
CASE(JSOP_DELPROP)
{
RootedPropertyName &name = rootName0;
name = script->getName(REGS.pc);
RootedId &id = rootId0;
id = NameToId(script->getName(REGS.pc));
RootedObject &obj = rootObject0;
FETCH_OBJECT(cx, -1, obj);
bool succeeded;
if (!JSObject::deleteProperty(cx, obj, name, &succeeded))
if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
goto error;
if (!succeeded && script->strict()) {
obj->reportNotConfigurable(cx, NameToId(name));
obj->reportNotConfigurable(cx, id);
goto error;
}
MutableHandleValue res = REGS.stackHandleAt(-1);
@@ -2340,15 +2340,12 @@ CASE(JSOP_DELELEM)
propval = REGS.sp[-1];
bool succeeded;
if (!JSObject::deleteByValue(cx, obj, propval, &succeeded))
goto error;
if (!succeeded && script->strict()) {
// XXX This observably calls ToString(propval). We should convert to
// PropertyKey and use that to delete, and to report an error if
// necessary!
RootedId id(cx);
RootedId &id = rootId0;
if (!ValueToId<CanGC>(cx, propval, &id))
goto error;
if (!JSObject::deleteGeneric(cx, obj, id, &succeeded))
goto error;
if (!succeeded && script->strict()) {
obj->reportNotConfigurable(cx, id);
goto error;
}
@@ -3684,7 +3681,8 @@ js::DeleteProperty(JSContext *cx, HandleValue v, HandlePropertyName name, bool *
if (!obj)
return false;
if (!JSObject::deleteProperty(cx, obj, name, bp))
RootedId id(cx, NameToId(name));
if (!JSObject::deleteGeneric(cx, obj, id, bp))
return false;
if (strict && !*bp) {
@@ -3705,16 +3703,13 @@ js::DeleteElement(JSContext *cx, HandleValue val, HandleValue index, bool *bp)
if (!obj)
return false;
if (!JSObject::deleteByValue(cx, obj, index, bp))
return false;
if (strict && !*bp) {
// XXX This observably calls ToString(propval). We should convert to
// PropertyKey and use that to delete, and to report an error if
// necessary!
RootedId id(cx);
if (!ValueToId<CanGC>(cx, index, &id))
return false;
if (!JSObject::deleteGeneric(cx, obj, id, bp))
return false;
if (strict && !*bp) {
obj->reportNotConfigurable(cx, id);
return false;
}
@@ -3815,7 +3810,8 @@ js::DeleteNameOperation(JSContext *cx, HandlePropertyName name, HandleObject sco
}
bool succeeded;
if (!JSObject::deleteProperty(cx, scope, name, &succeeded))
RootedId id(cx, NameToId(name));
if (!JSObject::deleteGeneric(cx, scope, id, &succeeded))
return false;
res.setBoolean(succeeded);
return true;

View File

@@ -534,19 +534,10 @@ with_SetGenericAttributes(JSContext *cx, HandleObject obj, HandleId id, unsigned
}
static bool
with_DeleteProperty(JSContext *cx, HandleObject obj, HandlePropertyName name,
bool *succeeded)
with_DeleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded)
{
RootedObject actual(cx, &obj->as<DynamicWithObject>().object());
return JSObject::deleteProperty(cx, actual, name, succeeded);
}
static bool
with_DeleteElement(JSContext *cx, HandleObject obj, uint32_t index,
bool *succeeded)
{
RootedObject actual(cx, &obj->as<DynamicWithObject>().object());
return JSObject::deleteElement(cx, actual, index, succeeded);
return JSObject::deleteGeneric(cx, actual, id, succeeded);
}
static JSObject *
@@ -602,8 +593,7 @@ const Class DynamicWithObject::class_ = {
with_SetElement,
with_GetGenericAttributes,
with_SetGenericAttributes,
with_DeleteProperty,
with_DeleteElement,
with_DeleteGeneric,
nullptr, nullptr, /* watch/unwatch */
nullptr, /* slice */
nullptr, /* enumerate (native enumeration of target doesn't work) */

View File

@@ -697,8 +697,7 @@ const XPCWrappedNativeJSClass XPC_WN_NoHelper_JSClass = {
nullptr, // setElement
nullptr, // getGenericAttributes
nullptr, // setGenericAttributes
nullptr, // deleteProperty
nullptr, // deleteElement
nullptr, // deleteGeneric
nullptr, nullptr, // watch/unwatch
nullptr, // slice
XPC_WN_JSOp_Enumerate,

View File

@@ -912,8 +912,7 @@ XPC_WN_JSOp_ThisObject(JSContext *cx, JS::HandleObject obj);
nullptr, /* setElement */ \
nullptr, /* getGenericAttributes */ \
nullptr, /* setGenericAttributes */ \
nullptr, /* deleteProperty */ \
nullptr, /* deleteElement */ \
nullptr, /* deleteGeneric */ \
nullptr, nullptr, /* watch/unwatch */ \
nullptr, /* slice */ \
XPC_WN_JSOp_Enumerate, \
@@ -936,8 +935,7 @@ XPC_WN_JSOp_ThisObject(JSContext *cx, JS::HandleObject obj);
nullptr, /* setElement */ \
nullptr, /* getGenericAttributes */ \
nullptr, /* setGenericAttributes */ \
nullptr, /* deleteProperty */ \
nullptr, /* deleteElement */ \
nullptr, /* deleteGeneric */ \
nullptr, nullptr, /* watch/unwatch */ \
nullptr, /* slice */ \
XPC_WN_JSOp_Enumerate, \