Hi Debadree, Sorry for not replying to your original email - often with bugs like this, the reason we haven't fixed them is because no one thought about it too much, and thinking about the solution is 90% of the time needed to actually fix it.
In your case, I haven't looked at the details of the issue whether your overall approach is correct - at a skim it seems right but you never know with weird edge cases. To answer your question about IsNumber, you likely want to also check for index strings, like "123". You can detect these with String::AsArrayIndex, which returns false if the conversion to a numeric index fails. Hope that's helpful, Leszek On Sat, 11 Mar 2023, 21:13 Debadree Chatterjee, <[email protected]> wrote: > Anu guidance toward any relevant documentation would also be helpful > > On Thursday, March 2, 2023 at 12:16:36 AM UTC+5:30 Debadree Chatterjee > wrote: > >> Hello v8-dev, >> >> I am trying to work on the issue >> https://bugs.chromium.org/p/v8/issues/detail?id=13728 which is a bug >> noticed from nodejs, as a first time contributor the docs suggest that I >> reach out here first before working on a issue. Already some of the >> debugging of the issue has been done on the original thread on nodejs repo >> @ https://github.com/nodejs/node/issues/41714, the crux of the matter >> being that the function `FilterProxyKeys` seems to ignore the >> `v8::IndexFilter::kSkipIndices` property passed to `GetPropertyNames`, I >> attempted to solve this with the following diff >> >> ```diff >> diff --git a/deps/v8/src/objects/keys.cc b/deps/v8/src/objects/keys.cc >> index a0796864f1..4f84cd9094 100644 >> --- a/deps/v8/src/objects/keys.cc >> +++ b/deps/v8/src/objects/keys.cc >> @@ -182,8 +182,9 @@ ExceptionStatus >> KeyAccumulator::AddKeys(Handle<JSObject> array_like, >> MaybeHandle<FixedArray> FilterProxyKeys(KeyAccumulator* accumulator, >> Handle<JSProxy> owner, >> Handle<FixedArray> keys, >> - PropertyFilter filter) { >> - if (filter == ALL_PROPERTIES) { >> + PropertyFilter filter, >> + bool skip_indices) { >> + if (filter == ALL_PROPERTIES && !skip_indices) { >> // Nothing to do. >> return keys; >> } >> @@ -191,7 +192,7 @@ MaybeHandle<FixedArray> >> FilterProxyKeys(KeyAccumulator* accumulator, >> int store_position = 0; >> for (int i = 0; i < keys->length(); ++i) { >> Handle<Name> key(Name::cast(keys->get(i)), isolate); >> - if (key->FilterKey(filter)) continue; // Skip this key. >> + if (key->FilterKey(filter) || (skip_indices && key->IsNumber())) >> continue; // Skip this key. >> if (filter & ONLY_ENUMERABLE) { >> PropertyDescriptor desc; >> Maybe<bool> found = >> @@ -218,7 +219,7 @@ Maybe<bool> >> KeyAccumulator::AddKeysFromJSProxy(Handle<JSProxy> proxy, >> // Postpone the enumerable check for for-in to the ForInFilter step. >> if (!is_for_in_) { >> ASSIGN_RETURN_ON_EXCEPTION_VALUE( >> - isolate_, keys, FilterProxyKeys(this, proxy, keys, filter_), >> + isolate_, keys, FilterProxyKeys(this, proxy, keys, filter_, >> skip_indices_), >> Nothing<bool>()); >> } >> // >> https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys >> ``` >> >> but this doesn't seem to work mainly because I think I am using >> `key->IsNumber()` wrong. I am not sure what would be the correct way to >> check if the given key is indeed an index hence I ask for any guidance >> here, I understand this is a very beginner question but any help would be >> greatly appreciated. >> >> Your Sincerely, >> Debadree >> > -- > -- > 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]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/v8-dev/9307c0b4-fd2c-4774-9614-71d0f7799091n%40googlegroups.com > <https://groups.google.com/d/msgid/v8-dev/9307c0b4-fd2c-4774-9614-71d0f7799091n%40googlegroups.com?utm_medium=email&utm_source=footer> > . > -- -- 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]. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAGRskv_SAzDTQrZuTXEQ-9Ljb%3DnntTauQj%2BGuqZCDMSPMoAaOQ%40mail.gmail.com.
