mono/mono — pull request #332 by robwilkens (2012-06-18 11:50:19+00:00)
Title: Bug 2663: Regular Expression Bug Fixes

What this fixes:
IF you were checking a regex which contained something like "(A|AB)C" and then you
checked something like "ABC" against it, it would fail previously, because its
first trip thru the regex it would match "A" first and short circuit out of the
"or" (|) then next try to match "C" but find "B" and fail. In this case, it should
be obvious that it needs to, after it fails, go back and try "AB" rather than "A"
and that's where this patch comes in.

Brief summary of each change in order that "git diff --staged" is giving them to
me:

This first one summarizes the background of goal of project:
New Function: EmitJumpTest
For when we are evaluating Alternatives (or's) we now want to be able to go back
and try or paths we haven't already tried if and only if we fail. This alternate to
EmitJump instead of jumping to target if we have a match immediately, will
on first attempt do the jump, but later if we have already tried this jump
combination, it will jump to ElseTarget, which is the equivalent of this test
failing on subsequent attempts with the same combination of 'or's passing.

RxCompiler:
EmitJumpTest isn't implemented for RxCompiler because i didn't have sample
code to test it on, so it falls back to a regular jump, nothing changes in
RxCompiler in other words other than having this required fucntion front.

arch.cs:
New OpCode added for JumpTest (see EmitJumpTest above)

compiler.cs:
EmitJumpTest added here for regular compiler, it now takes two arguments (two
items on stack) first was the standard where to jump to address, the second
argument is where to jump to in case where we have already tried this
combination)

interpreter.cs:

JumpTestEntry - What we keep track of to know if we've been here before, for
now i presumed just 'pc' (program counter') and 'ptr' (pointer - where we are)
were enough to tell us if we've been to this particular spot before.

Eval has been moved into Eval_Real

Eval if it gets called with parameters indicating we've already been here
(i.e we're being called recursively) just passes the call directly to eval_real
as-was.

Otherwise eval will loop on eval_real while we fail and haven't tried every
possible path through all the or's. (Note: The special case of where we're
running a 'test' (i.e. checking !(a|b|c) rather than just matching (a|b|c)),
and if we're running a test, we turn off JumpTestTracking while it's in its
recursive section. The reason for this is because while we can rightly check
all the options of (a|ab|abc) first for a then try again with ab then try again
with abc, we cannot do the same for a not (i.e. !(something or something else)
is really (not something AND not somtehing else), and it is not (not something
or not something else))

JumpTestList tracks our current run's JumpTest calls...
TriedCombo's tracks all the combos (all the JumpTestList results) that we've
already tried.

All calls to anything which calls Eval were modified to call eval passing
the optional new parameters that we now need, and remember if we call
eval with those parameters having a value, it will just forward the call
to Eval_Real.

This included modifying TryMatch because it calls Eval.

OpCode.JumpTest handler was added, what it does is first check if we need
to ByPass to jump to 'else' right away, which again happens when we're in
a test recursive call (Checking not, for example).
Next, it will create and populate a new JumpTestEntry with the current values.
It then calls CheckComboMatch() which sees if we match a combo already on file.
If we don't have a Match we jump to the first label (at pc+1) which is the
regular jump call. If we have a match, though, it will jump to the else label
which is now at (pc+2).

CheckComboMatch had to be broken up into two functions because it was going
too deep in terms of nesting. What it does by now should be somewhat understood
it will see if we have a match between our current run and previous runs, and
if we have a match, it returns true. If we don't have a match, it returns
false. I have some doubts about whether this function is 100% correct, there
may remain a bug or two, but it does pass all the "System" unit tests which
includes all the regular expression tests, plus it works for all the tests
in the bug report, so i have little reason to mess with it.

syntax.cs:
Here's where i modified so that when evaluating alternatives ("or"s or "|"s)
we instead call EmitJumpTest rather than just a regular EmitJump.

RegExBugs.cs:
This is the unit test (a bunch in one) which fails on mono without this patch
but succeeds with it
commit 48027ac2bdd9b60d18985ec2537eb188607f8164 1 parent 91c4180d55

This patch was originally submitted under another branch which had a bunch of
unrelated commits in it. I'm hoping if i did this right, these changes stand
alone.