Bug 173867 – RegExp.prototype[@@replace] relies on globals and doesn't perform ToLength

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

...

[Saam Barati]

I think this is observable.

[Alexey Shvayka]

Created attachment 394532 [details]
Patch

[Alexey Shvayka]

Created attachment 394535 [details]
Patch

Add a few UNUSED_PARAM, drop extra JSC_HOST_CALL. I wonder if we need to ALWAYS_INLINE string*Impl()?

[Ross Kirsling]

Comment on attachment 394535 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:14
> +        b) Removes `lastPosition` checks/updates, as there are none in the spec, and it was
> +           equivalent to checking `nextSourcePosition`.

Nice catch. I wondered about this in my earlier patch but evidently didn't think about it long enough to realize it wasn't helping at all.

[Alexey Shvayka]

Created attachment 394541 [details]
Patch

Set correct 'length' of builtinStringSubstringInternal's JSFunction.

[Ross Kirsling]

Comment on attachment 394541 [details]
Patch

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

> Source/JavaScriptCore/runtime/StringPrototype.cpp:1136
> +    JSValue position = callFrame->argument(1);
> +    UNUSED_PARAM(position);
> +    ASSERT(position.isUndefined() || position.isNumber());

Here and below, if this is just for a non-release ASSERT, I suppose it'd be okay get the argument twice instead?

[Alexey Shvayka]

Created attachment 394561 [details]
Patch

Drop UNUSED_PARAM.

[EWS]

Committed r259029: <https://trac.webkit.org/changeset/259029>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394561 [details].

[Radar WebKit Bug Importer]

<rdar://problem/60900492>