https://codereview.chromium.org/203243004/diff/20001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/203243004/diff/20001/src/runtime.cc#newcode640
src/runtime.cc:640: HandleScope scope(isolate);
On 2014/03/19 10:15:25, Michael Starzinger wrote:
nit: Not that it makes much of a difference, but this can be within a
SealHandleScope here.
Done.
https://codereview.chromium.org/203243004/diff/20001/src/symbol.js
File src/symbol.js (right):
https://codereview.chromium.org/203243004/diff/20001/src/symbol.js#newcode66
src/symbol.js:66: function SymbolRegistry() {
On 2014/03/19 10:15:25, Michael Starzinger wrote:
nit: This naming makes it look like this is the implementation of
Symbol.registry, which doesn't exist. Could we instead rename it to
GetSymbolRegistry and move it up to around line 37 to clarify that
it's an
internal helper?
Done.
https://codereview.chromium.org/203243004/diff/20001/src/symbol.js#newcode77
src/symbol.js:77: function SymbolHook(key) {
On 2014/03/19 10:15:25, Michael Starzinger wrote:
nit: Likewise for GetSymbolHook here.
Renamed to InternalSymbol.
https://codereview.chromium.org/203243004/diff/20001/src/symbol.js#newcode98
src/symbol.js:98: function SymbolKeyFor(symbol) {
On 2014/03/19 10:15:25, Michael Starzinger wrote:
Is it specified what "Symbol.keyFor(Symbol.create)" should return?
Simply undefined, since it's not in the (public) registry. I added a
test.
https://codereview.chromium.org/203243004/diff/20001/src/symbol.js#newcode100
src/symbol.js:100: throw MakeTypeError("not_a_symbol", ["Symbol.keyFor",
symbol]);
On 2014/03/19 09:06:47, sof wrote:
Looks like 'symbol' (%1) isn't used in the not_a_symbol format string.
Done.
https://codereview.chromium.org/203243004/diff/20001/src/symbol.js#newcode121
src/symbol.js:121: var symbolCreate = SymbolHook("@@create");
On 2014/03/19 09:06:47, sof wrote:
Just trying to understand.. where is the implicit addition to the
global
registry for the well-known symbols given?
Hm, I'm not sure what you are asking. The well-known symbols are not
supposed to be in the (public) registry. Or did I miss something?
https://codereview.chromium.org/203243004/diff/20001/test/mjsunit/harmony/symbols.js
File test/mjsunit/harmony/symbols.js (right):
https://codereview.chromium.org/203243004/diff/20001/test/mjsunit/harmony/symbols.js#newcode414
test/mjsunit/harmony/symbols.js:414: assertFalse(Symbol.for("@@create")
=== Symbol.create)
On 2014/03/19 10:15:25, Michael Starzinger wrote:
On 2014/03/19 09:06:47, sof wrote:
> Worth adding an initial test for keyFor() over well known symbols?
+1
Done. (But note that it just returns undefined.)
https://codereview.chromium.org/203243004/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
