Bug 46893 - The parenthetical assertion checking isn't working in some cases with YARR Interpreter
Summary: The parenthetical assertion checking isn't working in some cases with YARR In...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Peter Varga
URL:
Keywords:
Depends on:
Blocks: 46719
  Show dependency treegraph
 
Reported: 2010-09-30 06:17 PDT by Peter Varga
Modified: 2010-10-15 00:59 PDT (History)
7 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Varga 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
Comment 1 Peter Varga 2010-09-30 06:22:27 PDT
Created attachment 69330 [details]
proposed patch
Comment 2 WebKit Review Bot 2010-09-30 09:06:06 PDT
Attachment 69330 [details] did not build on win:
Build output: http://queues.webkit.org/results/4193030
Comment 3 Andras Becsi 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.
Comment 4 Peter Varga 2010-09-30 10:31:54 PDT
Created attachment 69345 [details]
proposed patch

Windows build fixed.
Comment 5 Gavin Barraclough 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;
Comment 6 Peter Varga 2010-10-06 02:45:44 PDT
Created attachment 69910 [details]
proposed patch v3
Comment 7 Gavin Barraclough 2010-10-14 10:38:53 PDT
Comment on attachment 69910 [details]
proposed patch v3

Looks great, thank you!
Comment 8 WebKit Commit Bot 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
Comment 9 Zoltan Horvath 2010-10-14 23:53:36 PDT
Comment on attachment 69910 [details]
proposed patch v3

Little bot, try it again!
Comment 10 WebKit Commit Bot 2010-10-14 23:57:28 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/4460029
Comment 11 Zoltan Horvath 2010-10-15 00:59:40 PDT
Committed revision 69842.