fixed bugs in CPS.hs; see comments in #2733 at github

[sydow]

Fixed problems in the pre pass of CPS.hs for Let expressions and And/Or expressions.

[sydow]

After several trivial nmistakes I managed to put together a PR which fails "only" in the four checks listed. I believe that only the acton-yang failure is important, but this needs to be looked at carefully. The test run report says the following:

 w3cregex_to_re_match:                                       FAIL           :  140 runs in 50.131ms @ 2792.7/s
    outcomes: 140 fail
    testing.NotEqualError: Expected equal values but they are non-equal. A: ^(a||b)$ B: ^(a|b)$
    @@ -1,1 +1,1 @@
    -^(a||b)$
    +^(a|b)$
    
   w3cregex_to_re_search_basic:                                FAIL           :  148 runs in 50.035ms @ 2957.9/s
    outcomes: 148 fail
    testing.NotEqualError: Expected equal values but they are non-equal. A: a||b B: a|b
    @@ -1,1 +1,1 @@
    -a||b
    +a|b

@mzagozen, can you look at these tests and say something about the results? And @nordlander, can you help by looking at my friend's code for And/Or expressions in the CPS pre pass?

If you want/need to run the compiler, it is branch fix-2733-again.

[sydow]

I talked to my friend about the acton-yang problem, and got the following reply:

I found the core issue: this one is in the source logic, not in CPS. In parse_regex, once one branch has been parsed, every later loop iteration with a non-empty elems appends |, even if that iteration did not just consume a separator. That produces exactly a||b. I’m checking for any intended empty-branch behavior before I patch it.

He suggests the following patch to parse_regex

    312 -            if not ielems and self.try_get_char_in('|') is None:
    313 -                break
    312 +            if not ielems:
    313 +                if self.try_get_char_in('|') is None:
    314 +                    break
    315 +                elems.append('|')
    316 +                continue
    317              if ibp < bp:
    318                  bp = ibp
    316 -            if elems:
    317 -                elems.append('|')

leading to the modified version

    def parse_regex(self) -> (list[str], int):
        elems = []
        bp = BP_ATOMIC
        while True:
            ielems, ibp = self.parse_branch()
            if not ielems:
                if self.try_get_char_in('|') is None:
                    break
                elems.append('|')
                continue
            if ibp < bp:
                bp = ibp
            elems.extend(ielems)
        if len(elems) > 1:
            if BP_OR < bp:
                bp = BP_OR
        return elems, bp

So it would be good if @mzagozen could verify my friend's finding and if so update acton-yang. After that we make another try with this PR.

[mzagozen]

@sydow yes, the fix is in and now this compiler branch passes the test.