Hi Leszek, Thank you so much for the response, will try to apply your suggestions and iron out any bugs and submit a CL, will ask here if encounter anything I don't understand Thank you once again!
Yours Sincerely, Debadree On Monday, March 13, 2023 at 12:30:11 PM UTC+5:30 les...@chromium.org wrote: 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, <debad...@gmail.com> 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 v8-...@googlegroups.com 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 v8-dev+un...@googlegroups.com. 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 v8-dev@googlegroups.com 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 v8-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/fae4e20d-fcd1-489c-ac7f-6363b66592d8n%40googlegroups.com.
