Bug 46893

Summary:

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

Product:

WebKit

Reporter:

Peter Varga <pvarga>

Component:

JavaScriptCore

Assignee:

Peter Varga <pvarga>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abecsi, barraclough, commit-queue, msaboff, webkit.review.bot, zherczeg, zoltan

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

46719

Attachments:

Description	Flags
proposed patch	none
proposed patch	barraclough: review-
proposed patch v3	barraclough: review+, commit-queue: commit-queue-

Peter Varga

Reported 2010-09-30 06:17:41 PDT

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

Attachments
proposed patch (3.93 KB, patch) 2010-09-30 06:22 PDT, Peter Varga	no flags	Details Formatted Diff Diff
proposed patch (4.07 KB, patch) 2010-09-30 10:31 PDT, Peter Varga	barraclough: review-	Details Formatted Diff Diff
proposed patch v3 (4.00 KB, patch) 2010-10-06 02:45 PDT, Peter Varga	barraclough: review+ commit-queue: commit-queue-	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Peter Varga

Comment 1 2010-09-30 06:22:27 PDT

Created attachment 69330 [details] proposed patch

WebKit Review Bot

Comment 2 2010-09-30 09:06:06 PDT

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

Andras Becsi

Comment 3 2010-09-30 09:41:44 PDT

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

Comment 4 2010-09-30 10:31:54 PDT

Created attachment 69345 [details] proposed patch Windows build fixed.

Gavin Barraclough

Comment 5 2010-10-05 14:32:14 PDT

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

Comment 6 2010-10-06 02:45:44 PDT

Created attachment 69910 [details] proposed patch v3

Gavin Barraclough

Comment 7 2010-10-14 10:38:53 PDT

Comment on attachment 69910 [details] proposed patch v3 Looks great, thank you!

WebKit Commit Bot

Comment 8 2010-10-14 10:46:57 PDT

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 9 2010-10-14 23:53:36 PDT

Comment on attachment 69910 [details] proposed patch v3 Little bot, try it again!

WebKit Commit Bot

Comment 10 2010-10-14 23:57:28 PDT

Zoltan Horvath

Comment 11 2010-10-15 00:59:40 PDT

Committed revision 69842.

Note You need to log in before you can comment on or make changes to this bug.