https://codereview.chromium.org/108083005/diff/40001/test/mjsunit/harmony/symbols.js
File test/mjsunit/harmony/symbols.js (right):
https://codereview.chromium.org/108083005/diff/40001/test/mjsunit/harmony/symbols.js#newcode305
test/mjsunit/harmony/symbols.js:305: assertEquals("symbol", typeof
syms[i])
On 2013/12/11 00:16:26, arv wrote:
On 2013/12/10 10:39:53, rossberg wrote:
> Also assert that insertion order is maintained (which implies that
they are
all
> different).
You sent me off on goose chase there. I finally figured out that we
only use
insertion order when %HasFastProperties is true
Ah, sorry about that. I should have known.
https://codereview.chromium.org/108083005/diff/90001/src/macros.py
File src/macros.py (right):
https://codereview.chromium.org/108083005/diff/90001/src/macros.py#newcode162
src/macros.py:162: macro IS_PRIVATE(arg) = (%SymbolIsPrivate(arg));
Nit: s/arg/sym/
https://codereview.chromium.org/108083005/diff/90001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/108083005/diff/90001/src/runtime.cc#newcode5750
src/runtime.cc:5750: if (name->IsSymbol() &&
!static_cast<Symbol*>(name)->is_private()) {
Use Symbol::cast(name) here (and below).
But I had actually hoped that this whole function would be unnecessary.
See below.
https://codereview.chromium.org/108083005/diff/90001/src/runtime.cc#newcode5861
src/runtime.cc:5861: if ((key_type & Runtime::PROPERTY_KEY_SYMBOL) &&
How about splitting PropertyAttributes SYMBOLIC filter into
SYMBOLIC_PUBLIC and SYMBOLIC_PRIVATE, so that you can avoid the filter
post-processing altogether? (Except for interceptors of course, which
you have to post-process anyway, but see below.)
You would only need to refine a couple of places in objects.cc
accordingly, namely Map::NumberOfDescribedProperties and
JSObject::GetLocalPropertyNames, and Dictionary<Shape,
Key>::NumberOfElementsFilterAttributes.
https://codereview.chromium.org/108083005/diff/90001/src/runtime.cc#newcode5917
src/runtime.cc:5917: if ((key_type & Runtime::PROPERTY_KEY_SYMBOL) &&
I'm not sure I understand this. If somebody passed type =
PROPERTY_KEY_STRING, wouldn't you also need to filter all symbols?
I think it's better to leave this function alone and for the interceptor
case, do the filtering in v8natives' ObjectGetOwnPropertyKeys -- for
interceptors, that has to do a copy anyway, where it fits in nicely.
https://codereview.chromium.org/108083005/diff/90001/src/runtime.cc#newcode5919
src/runtime.cc:5919: Handle<FixedArray> res =
isolate->factory()->NewFixedArray(result->Length());
Nit: line length
https://codereview.chromium.org/108083005/diff/90001/src/runtime.cc#newcode5920
src/runtime.cc:5920: for (int i = 0; i < res->length(); i++) {
On 2013/12/11 23:51:12, arv wrote:
This is ugly. What is the correct way to do this?
You mean the copying? Only if you allow the Filter function to operate
on different array types (e.g. by templatising it).
https://codereview.chromium.org/108083005/diff/90001/test/mjsunit/harmony/symbols.js
File test/mjsunit/harmony/symbols.js (right):
https://codereview.chromium.org/108083005/diff/90001/test/mjsunit/harmony/symbols.js#newcode276
test/mjsunit/harmony/symbols.js:276: function TestKeyHas(obj) {
Doh, how t.f. did this ever work???
https://codereview.chromium.org/108083005/
--
--
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/groups/opt_out.
