Bug 155177 – [ES6] Implement RegExp sticky flag and related functionality

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

[Michael Saboff]

Implement the sticky flag, 'y', and the behavior described in the ES6 spec.

[Radar WebKit Bug Importer]

<rdar://problem/25038274>

[Michael Saboff]

Created attachment 273443 [details]
Patch

[WebKit Commit Bot]

Attachment 273443 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/yarr/YarrPattern.h:305:  The parameter name "flags" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Comment on attachment 273443 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:15
> +        Changed both the Yarr interpreter and jit to no loop to the next character for sticky RegExp's.

typo: "to no loop" ==> "to not loop"?

[Mark Lam]

Comment on attachment 273443 [details]
Patch

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

> Source/JavaScriptCore/runtime/RegExpPrototype.cpp:179
> +    if (exec->hadException())
> +        return string;
> +    JSValue stickyValue = regexp->get(exec, exec->propertyNames().sticky);
> +    if (exec->hadException())
> +        return string;

Perhaps take this opportunity to cache "VM& vm = exec->vm();" up top and use "vm.exception()" in all these tests?

[Saam Barati]

Comment on attachment 273443 [details]
Patch

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

r=me

> Source/JavaScriptCore/runtime/RegExpPrototype.cpp:179
> +    if (exec->hadException())
> +        return string;

All the exception checks in this function would be better off if they cached the VM at the top and checked vm.exception()

> Source/JavaScriptCore/yarr/YarrJIT.cpp:1848
> +                            // If the pattern size is not fixed, then store the start index, for use if we match.

second comma not needed.

> Source/JavaScriptCore/yarr/YarrJIT.cpp:1898
> +                    removeCallFrame();
> +                    move(TrustedImmPtr((void*)WTF::notFound), returnRegister);
> +                    move(TrustedImm32(0), returnRegister2);
> +                    generateReturn();

This code is repeated in a couple other places. Might be worth a helper.

> LayoutTests/js/script-tests/regexp-sticky.js:56
> +    for (var iter = 0; iter < expected.length; iter++) {
> +        let lastIndexStart = re.lastIndex;
> +
> +        let result = str.match(re);
> +        let correctResult = false;
> +	if (expected[iter] === null || result === null)
> +	    correctResult = (expected[iter] === result);
> +	else if (result.length == expected[iter].length) {
> +	    correctResult = true;
> +            for (let i = 0; i < result.length; i++) {
> +                if (result[i] != expected[iter][i])
> +                    correctResult = false;
> +            }
> +        }

Style: Indentation is off here.

[Michael Saboff]

Committed r197869: <http://trac.webkit.org/changeset/197869>