The Trunk: Regex-Core-pre.51.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Regex-Core-pre.51.mcz

commits-2
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!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Regex-Core-pre.51.mcz

Nicolas Cellier
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 <[hidden email]>:
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!





Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Regex-Core-pre.51.mcz

Patrick R.

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: [hidden email] <[hidden email]> on behalf of Nicolas Cellier <[hidden email]>
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 <[hidden email]>:
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!





Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Regex-Core-pre.51.mcz

Levente Uzonyi
Hi All,

The reason why #veryDeepCopy works, and #copy doesn't (besides some
missing #postCopy methods) is that some nodes in the matcher graph are
referenced from more than one place, so #copy will create two copies of
these, while #veryDeepCopy will only create one.
Still, #veryDeepCopy does make unnecessary object duplications, so we
should find a way to work this around.
One example for a double reference is in RxMatcher >> #makeOptional:,
where the dummy link is added to the branch and the end of the matcher
chain as well.

Levente

On Thu, 19 May 2016, Rein, Patrick wrote:

>
> 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: [hidden email] <[hidden email]> on behalf of Nicolas Cellier
> <[hidden email]>
> 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 <[hidden email]>:
>       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!
>
>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Regex-Core-pre.51.mcz

Nicolas Cellier
In reply to this post by Patrick R.


2016-05-19 23:21 GMT+02:00 Rein, Patrick <[hidden email]>:

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


Perfectly, i missed that point, thanks for explanations.
Still, copy should do something correct and i prefer the solution posted by Levente, even if it may sound more heavy than a clever veryDeepCopy.



From: [hidden email] <[hidden email]> on behalf of Nicolas Cellier <[hidden email]>
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 <[hidden email]>:
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!