Hi Nicolas,


thanks for looking into this, as this is kind of tricky to get right on my own without any discussions. What I did is to look into the value of anRmxLink in #makeQuantified:min:max: before and after a copy. What happens is that the backward references in RxmBranch nodes back to other RxmLink nodes break as the references get only copied separately and result in two different objects. As a result the copied structure does not include any back references. The postCopy callbacks do not solve that issue but do, to my understanding at that point, only implement the recursion of the copying. veryDeepCopy however produces the same node structure which is what is actually needed for the quantified structures. This way the termiateWith: method works correctly and the generating of broken matcher structures is fixed. However, it changes the behavior slightly (as in the Regex-Tests-Core commit in the inbox) so I am not yet sure whether this is only a technical fix or also a correct fix of the behavior.


Does that sound somewhat reasonable? :)


Bests

Patrick



________________________________
From: squeak-dev-bounces at lists.squeakfoundation.org <squeak-dev-bounces at lists.squeakfoundation.org> on behalf of Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>
Sent: Thursday, May 19, 2016 22:18
To: The general-purpose Squeak developers list
Subject: Re: [squeak-dev] The Trunk: Regex-Core-pre.51.mcz

Hi Patrick,
IMO copy should copy deep enough without the need to invoke veryDeepCopy.
Wouldn't it be a missing postCopy in RxmLookahead?

postCopy
    super postCopy.
    lookahead := lookahead copy

2016-05-19 20:49 GMT+02:00 <commits at source.squeak.org<mailto:commits at source.squeak.org>>:
Patrick Rein uploaded a new version of Regex-Core to project The Trunk:
http://source.squeak.org/trunk/Regex-Core-pre.51.mcz

==================== Summary ====================

Name: Regex-Core-pre.51
Author: pre
Time: 19 May 2016, 8:49:05.994548 pm
UUID: 49e3e29a-cf96-4f42-a8f1-4af75dbb9bca
Ancestors: Regex-Core-ul.50

Fixes an erroring test of RxMatcher by replacing a copy with a veryDeepCopy to sustain the integrity of the RxmLink tree.

=============== Diff against Regex-Core-ul.50 ===============

Item was changed:
  ----- Method: RxMatcher>>makeQuantified:min:max: (in category 'private') -----
  makeQuantified: anRxmLink min: min max: max
        "Perform recursive poor-man's transformation of the {<min>,<max>} quantifiers."
        | aMatcher |

        "<atom>{,<max>}       ==>  (<atom>{1,<max>})?"
        min = 0 ifTrue: [
                ^ self makeOptional: (self makeQuantified: anRxmLink min: 1 max: max) ].

        "<atom>{<min>,}       ==>  <atom>{<min>-1, <min>-1}<atom>+"
        max ifNil: [
+               ^ (self makeQuantified: anRxmLink min: 1 max: min-1) pointTailTo: (self makePlus: anRxmLink veryDeepCopy) ].
-               ^ (self makeQuantified: anRxmLink min: 1 max: min-1) pointTailTo: (self makePlus: anRxmLink copy) ].

        "<atom>{<max>,<max>}  ==>  <atom><atom> ... <atom>"
        min = max
                ifTrue: [
+                       aMatcher := anRxmLink veryDeepCopy.
+                       (min-1) timesRepeat: [ aMatcher pointTailTo: anRxmLink veryDeepCopy ].
-                       aMatcher := anRxmLink copy.
-                       (min-1) timesRepeat: [ aMatcher pointTailTo: anRxmLink copy ].
                        ^ aMatcher ].

        "<atom>{<min>,<max>}  ==>  <atom>{<min>,<min>}(<atom>{1,<max>-1})?"
+       aMatcher := self makeOptional: anRxmLink veryDeepCopy.
-       aMatcher := self makeOptional: anRxmLink copy.
        (max - min - 1) timesRepeat: [
+                aMatcher := self makeOptional: (anRxmLink veryDeepCopy pointTailTo: aMatcher) ].
-                aMatcher := self makeOptional: (anRxmLink copy pointTailTo: aMatcher) ].
        ^ (self makeQuantified: anRxmLink min: min max: min) pointTailTo: aMatcher!



-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20160519/539541fc/attachment-0001.htm