Bug 46893 – The parenthetical assertion checking isn't working in some cases with YARR Interpreter

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

[Peter Varga]

If a minimum matching size (minimumSize) of parenthetical assertion is equal to the following terms width in the pattern then 
the matching of the assertion fails. The calculation of the number of potentially matching characters (countToCheck) should be
changed in case of parenthetical assertions.

Eg.:
"abad".match(/a(?=d)./);
result: null
expected: ad

[Peter Varga]

Created attachment 69330 [details]
proposed patch

[WebKit Review Bot]

Attachment 69330 [details] did not build on win:
Build output: http://queues.webkit.org/results/4193030

[Andras Becsi]

Comment on attachment 69330 [details]
proposed patch

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

> JavaScriptCore/yarr/RegexInterpreter.cpp:1588
> +                    emitDisjunction(term.parentheses.disjunction, currentCountAlreadyChecked, -delegateEndInputOffset, true);

emitDisjunction avaits an unsigned value in the third parameter, MSVC warns because of the unary minus operator, and warnings are treated as errors.

[Peter Varga]

Created attachment 69345 [details]
proposed patch

Windows build fixed.

[Gavin Barraclough]

Comment on attachment 69345 [details]
proposed patch

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

I know it's only a small thing, but I'll r-, since I don't like the unnecessary *-1.  Otherwise, looks great!

> JavaScriptCore/yarr/RegexInterpreter.cpp:1599
> +                    int positiveInputOffset = -1 * (term.inputPosition - currentCountAlreadyChecked);

I think this could be a little more clear as:
+    ASSERT(currentCountAlreadyChecked > term.inputPosition);
+    int positiveInputOffset = currentCountAlreadyChecked - term.inputPosition;

[Peter Varga]

Created attachment 69910 [details]
proposed patch v3

[Gavin Barraclough]

Comment on attachment 69910 [details]
proposed patch v3

Looks great, thank you!

[WebKit Commit Bot]

Comment on attachment 69910 [details]
proposed patch v3

Rejecting patch 69910 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'build', '--no-clean', '--no-update', '--build-style=both', '--quiet']" exit_code: 2
Building WebKit
Failed to run "['WebKitTools/Scripts/build-webkit', '--debug']" exit_code: 1

Full output: http://queues.webkit.org/results/4470019

[Zoltan Horvath]

Comment on attachment 69910 [details]
proposed patch v3

Little bot, try it again!

[WebKit Commit Bot]

Comment on attachment 69910 [details]
proposed patch v3

Rejecting patch 69910 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'build', '--no-clean', '--no-update', '--build-style=both', '--quiet']" exit_code: 2
Building WebKit
Failed to run "['WebKitTools/Scripts/build-webkit', '--debug']" exit_code: 1

Full output: http://queues.webkit.org/results/4460029

[Zoltan Horvath]

Committed revision 69842.