During upgrade of Map/Set/WeakMap constructor interface, I've found that WeakSet implementation is missing.
Since MapData change becomes a little bit large, at first, I'll create the separate patch to integrate MapData into JSMap / JSSet. https://bugs.webkit.org/show_bug.cgi?id=142556
OK, MapData is done. So I'll perform the same thing on WeakMapData :D
At first, implement ES6 WeakSet and after that, optimize WeakMapData.
Since WeakMap / WeakSet doesn't have iterators, optimization is a little bit easier than MapData.
Created attachment 251103 [details] Patch
Comment on attachment 251103 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251103&action=review > Source/JavaScriptCore/runtime/JSWeakSet.cpp:2 > + * Copyright (C) 2013 Apple, Inc. All rights reserved. Copyright is based on date of publication (the time we publish in the open source repository). So it should be 2015 unless this code was published in 2013. > Source/JavaScriptCore/runtime/JSWeakSet.h:2 > + * Copyright (C) 2013 Apple, Inc. All rights reserved. Same issue with copyright date. > Source/JavaScriptCore/runtime/WeakMapPrototype.cpp:88 > if (!key.isObject()) > - return JSValue::encode(throwTypeError(callFrame, WTF::ASCIILiteral("A WeakMap cannot have a non-object key"))); > + return JSValue::encode(jsBoolean(false)); > return JSValue::encode(jsBoolean(map->remove(asObject(key)))); Now that this is always a boolean, I think we can write it more cleanly: return JSValue::encode(jsBoolean(key.isObject() && map->remove(asObject(key))); No need for the three-line version with the separate early return. > Source/JavaScriptCore/runtime/WeakMapPrototype.cpp:98 > + return JSValue::encode(jsUndefined()); Should this be undefined, or null? The change log comment mentions only "false". > Source/JavaScriptCore/runtime/WeakMapPrototype.cpp:110 > if (!key.isObject()) > - return JSValue::encode(throwTypeError(callFrame, WTF::ASCIILiteral("A WeakMap cannot have a non-object key"))); > + return JSValue::encode(jsBoolean(false)); > return JSValue::encode(jsBoolean(map->contains(asObject(key)))); Now that this is always a boolean, I think we can write it more cleanly: return JSValue::encode(jsBoolean(key.isObject() && map->contains(asObject(key))); No need for the three-line version with the separate early return. > Source/JavaScriptCore/runtime/WeakSetConstructor.cpp:2 > + * Copyright (C) 2013 Apple, Inc. All rights reserved. Same issue with copyright. > Source/JavaScriptCore/runtime/WeakSetConstructor.h:2 > + * Copyright (C) 2013 Apple, Inc. All rights reserved. Same issue with copyright. > Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:2 > + * Copyright (C) 2013 Apple, Inc. All rights reserved. Same issue with copyright. > Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:57 > + return 0; nullptr > Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:64 > + return 0; nullptr > Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:75 > + JSValue key = callFrame->argument(0); > + if (!key.isObject()) > + return JSValue::encode(jsBoolean(false)); > + return JSValue::encode(jsBoolean(map->remove(asObject(key)))); Now that this is always a boolean, I think we can write it more cleanly: return JSValue::encode(jsBoolean(key.isObject() && map->remove(asObject(key))); No need for the three-line version with the separate early return. > Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:86 > + if (!key.isObject()) > + return JSValue::encode(jsBoolean(false)); > + return JSValue::encode(jsBoolean(map->contains(asObject(key)))); Now that this is always a boolean, I think we can write it more cleanly: return JSValue::encode(jsBoolean(key.isObject() && map->contains(asObject(key))); No need for the three-line version with the separate early return. > Source/JavaScriptCore/runtime/WeakSetPrototype.h:2 > + * Copyright (C) 2013 Apple, Inc. All rights reserved. Same issue with copyright date.
Comment on attachment 251103 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251103&action=review Thank you for your review and detailed comments! >> Source/JavaScriptCore/runtime/JSWeakSet.cpp:2 >> + * Copyright (C) 2013 Apple, Inc. All rights reserved. > > Copyright is based on date of publication (the time we publish in the open source repository). So it should be 2015 unless this code was published in 2013. Thanks! I'll fix it. >> Source/JavaScriptCore/runtime/JSWeakSet.h:2 >> + * Copyright (C) 2013 Apple, Inc. All rights reserved. > > Same issue with copyright date. Done >> Source/JavaScriptCore/runtime/WeakMapPrototype.cpp:88 >> return JSValue::encode(jsBoolean(map->remove(asObject(key)))); > > Now that this is always a boolean, I think we can write it more cleanly: > > return JSValue::encode(jsBoolean(key.isObject() && map->remove(asObject(key))); > > No need for the three-line version with the separate early return. OK. I'll follow it :) >> Source/JavaScriptCore/runtime/WeakMapPrototype.cpp:98 >> + return JSValue::encode(jsUndefined()); > > Should this be undefined, or null? The change log comment mentions only "false". Oops! Thank you. The code is correct. We need to fix change log comment. http://people.mozilla.org/~jorendorff/es6-draft.html#sec-weakmap.prototype.get >> Source/JavaScriptCore/runtime/WeakMapPrototype.cpp:110 >> return JSValue::encode(jsBoolean(map->contains(asObject(key)))); > > Now that this is always a boolean, I think we can write it more cleanly: > > return JSValue::encode(jsBoolean(key.isObject() && map->contains(asObject(key))); > > No need for the three-line version with the separate early return. Done. >> Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:57 >> + return 0; > > nullptr Nice! I also fixed them in WeakMapPrototype.cpp. >> Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:64 >> + return 0; > > nullptr Done. >> Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:75 >> + return JSValue::encode(jsBoolean(map->remove(asObject(key)))); > > Now that this is always a boolean, I think we can write it more cleanly: > > return JSValue::encode(jsBoolean(key.isObject() && map->remove(asObject(key))); > > No need for the three-line version with the separate early return. Done. >> Source/JavaScriptCore/runtime/WeakSetPrototype.cpp:86 >> + return JSValue::encode(jsBoolean(map->contains(asObject(key)))); > > Now that this is always a boolean, I think we can write it more cleanly: > > return JSValue::encode(jsBoolean(key.isObject() && map->contains(asObject(key))); > > No need for the three-line version with the separate early return. Done.
Committed r182994: <http://trac.webkit.org/changeset/182994>