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! |
Hi Patrick, IMO copy should copy deep enough without the need to invoke veryDeepCopy.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: |
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.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: |
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! > > > > > |
In reply to this post by Patrick R.
2016-05-19 23:21 GMT+02:00 Rein, Patrick <[hidden email]>:
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.
|
Free forum by Nabble | Edit this page |