[Webkit-unassigned] [Bug 186694] [ESNExt] String.prototype.matchAll

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=186694

--- Comment #30 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 372055
  --> https://bugs.webkit.org/attachment.cgi?id=372055
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372055&action=review

>>> Source/JavaScriptCore/builtins/RegExpPrototype.js:28
>>> +function RegExpStringIteratorConstructor(R, S, global, fullUnicode)
>> 
>> Rename this to createRegExpStringIterator.
> 
> Using `new` with `create*` function looks kinda weird.
> Idiomatic JS is `new` with title-cased constructor, just like we already have with AsyncFromSyncIterator:
> 
> ```
> return new @AsyncFromSyncIteratorConstructor(syncIterator, nextMethod);
> ```
> 
> AsyncFromSyncIterator does indeed expose `createAsyncFromSyncIterator` helper for BytecodeGenerator,
> but I not sure it is something we need for RegExpStringIterator.
> 
> ===
> 
> On performance: we can't use any of already defined fast RegExp methods.
> We can avoid extra sets and property lookups in two places:
> 1. After @speciesConstructor in RegExp.prototype[@@matchAll]
> 2. After last @getByIdDirectPrivate in %RegExpStringIteratorPrototype%.next.
> 
> Given that we will have to call @hasObservableSideEffects in `next` every time, does it worth it?

But instead, you introduced "RegExpStringIteratorConstructor" variable in JSGlobalObject.cpp. In WebKit, variables start with lower case.
If you want to use this, can you do the following things too?

1. Rename RegExpStringIteratorConstructor to RegExpStringIterator
2. Fix the script not to expose RegExpStringIterator as a variable. Instead, we should expose regExpStringIterator
3. And fix all the existing ones too.

For performance perspective, I think this is OK for the first implementation.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190617/56e94e86/attachment-0001.html>