Bug 39289 – Wrong result in case of non-iterative matching of subpatterns in non-greedy cases in YARR Interpreter

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Peter Varga]

The start of the non-iterative matching of subpatterns isn't stored 
in case of non-greedy matching.

Eg.:
  print("ab".match(/(a)??b/));
 
Results:
  YARR Interpreter: ab,
  Expected: ab,a

[Peter Varga]

Created attachment 56366 [details]
proposed patch

[Darin Adler]

Comment on attachment 56366 [details]
proposed patch

Looks like a good code change. But needs a test case to demonstrate at least one regular expression that malfunctions without this and works with it. We don't take patches without test cases unless there's a reason a test case can't be created.

[Darin Adler]

I see the test case here in the bug report. Now it needs to be added to existing tests in LayoutTests. I suggest putting it into one of the existing tests in the fast/js directory that tests other regular expression cases. And add other similar test cases that are not affected.

[Peter Varga]

Created attachment 56489 [details]
proposed patch v2

I fixed the previous change and added a layout test. My previous change caused problems in some test cases and these cases have been added to the layout test.

[Darin Adler]

Comment on attachment 56489 [details]
proposed patch v2

> -                    output[(subpatternId << 1) + 1] = input.getPos() + term.inputPosition;
> +                    output[(subpatternId << 1)] = input.getPos() + term.inputPosition;

I suggest omitting the parentheses here. output[subpatternId << 1] would be easier to read.

> +        The backtrackParentesesOnceEnd function should store the start position

Typo here in the name of the function.

[Peter Varga]

Comment on attachment 56489 [details]
proposed patch v2

cq+ removed to fix the mentioned issues.

[Andras Becsi]

(In reply to comment #6)
> (From update of attachment 56489 [details])
> cq+ removed to fix the mentioned issues.
Sending        JavaScriptCore/ChangeLog

Sending        JavaScriptCore/yarr/RegexInterpreter.cpp
Sending        LayoutTests/ChangeLog
Adding         LayoutTests/fast/js/regexp-non-greedy-parentheses-expected.txt
Adding         LayoutTests/fast/js/regexp-non-greedy-parentheses.html
Adding         LayoutTests/fast/js/script-tests/regexp-non-greedy-parentheses.js
Transmitting file data ......
Committed revision 59766.

Patch landed. Closing bug.