Closed p5pRT closed 14 years ago
I'm hoping this is an uncontroversial patch\, in preparation for the new regex modifiers coming soon. This does cause binary incompatibility.
On 26 June 2010 21:25\, karl williamson \perlbug\-followup@​perl\.org wrote:
# New Ticket Created by karl williamson # Please include the string: [perl #76122] # in the subject line of all future correspondence about this issue. # \<URL: http://rt.perl.org/rt3/Ticket/Display.html?id=76122 >
I'm hoping this is an uncontroversial patch\, in preparation for the new regex modifiers coming soon. This does cause binary incompatibility.
Just wanted to comment\, on this one...
I worked a bit on an earlier phase of reorganizing these defines\, and frankly the only reason there is the RXf_ PMf_ difference\, or that one is not explicitly defined in terms of the other\, was because of my weak header-fu.
So for instance\, if you thought you could eliminate the duplication then I personally think it would probably be a good idea.
Anyway\, cant apply/test this right now\, but it looks sane to me.
Just curious about the comment patch - is it really necessary or just good practice? Dont the rules specify that comments are stipped first? Seems like it should go in regardless\, but im curious about the fine point of the rules.
Yves
-- perl -Mre=debug -e "/just|another|perl|hacker/"
The RT System itself - Status changed from 'new' to 'open'
demerphq wrote:
On 26 June 2010 21:25\, karl williamson \perlbug\-followup@​perl\.org wrote:
# New Ticket Created by karl williamson # Please include the string: [perl #76122] # in the subject line of all future correspondence about this issue. # \<URL: http://rt.perl.org/rt3/Ticket/Display.html?id=76122 >
I'm hoping this is an uncontroversial patch\, in preparation for the new regex modifiers coming soon. This does cause binary incompatibility.
Just wanted to comment\, on this one...
I worked a bit on an earlier phase of reorganizing these defines\, and frankly the only reason there is the RXf_ PMf_ difference\, or that one is not explicitly defined in terms of the other\, was because of my weak header-fu.
So for instance\, if you thought you could eliminate the duplication then I personally think it would probably be a good idea.
I'll look into it.
Anyway\, cant apply/test this right now\, but it looks sane to me.
Just curious about the comment patch - is it really necessary or just good practice? Dont the rules specify that comments are stipped first? Seems like it should go in regardless\, but im curious about the fine point of the rules.
That it has worked without complaint from all compilers indicates it's not a compile problem. I looked at K&R 2nd edition\, and it's not clear to me what's legal from that. The reason I made the fix is that my vim doesn't like the way it was\, highlights it as being wrong. This is a distraction to me whenever I look at the code; adding extra cognitive load. So\, it's easier to change it once\, and then forget about it.
Yves
karl williamson \public@​khwilliamson\.com wrote: :demerphq wrote: :> Just curious about the comment patch - is it really necessary or just :> good practice? Dont the rules specify that comments are stipped first? :> Seems like it should go in regardless\, but im curious about the fine :> point of the rules. : :That it has worked without complaint from all compilers indicates it's :not a compile problem. I looked at K&R 2nd edition\, and it's not clear :to me what's legal from that. The reason I made the fix is that my vim :doesn't like the way it was\, highlights it as being wrong. This is a :distraction to me whenever I look at the code; adding extra cognitive :load. So\, it's easier to change it once\, and then forget about it.
I'd rather see the comments moved above the defines\, to remove the issue altogether.
Hugo
On Wed\, Jun 30\, 2010 at 12:40:14PM +0100\, hv@crypt.org wrote:
I'd rather see the comments moved above the defines\, to remove the issue altogether.
+1
-- In England there is a special word which means the last sunshine of the summer. That word is "spring".
Dave Mitchell wrote:
On Wed\, Jun 30\, 2010 at 12:40:14PM +0100\, hv@crypt.org wrote:
I'd rather see the comments moved above the defines\, to remove the issue altogether.
+1
I will do this\, as well as combine the common parts of op.h and regexp.h\, as suggested by Yves.
This series of commits is intended for 5.13.4. I imagine there will be multiple go-arounds before a version of it is accepted.
This patch is the first step in trying to add new regex modifiers. Currently the structures in op.h and regexp.h share common values\, which manually must be kept in sync. This patch creates a new header with those common values that is #included from the two affected header files. There are commits for ancillary small bug fixes and added comments I did as I went along.
However\, I left the two different symbols for each value because I wasn't sure what to do with them\, since they are both exported by B\, and I presume that means that external programs may be depending on them. I see that B has some explicit definitions for backward compatibility\, so I'm thinking it would be better to remove the duplicate symbols in the core\, and then define them there for external programs. But I don't know enough to know if that is reasonable or not.
That brings up another question that I need for future patches. We are adding mutually exclusive regex modifiers. It is more suitable to encode these into one multi-bit field instead of a single bit each. It also takes fewer bits\, and we are running out of bits\, and I can see other uses coming along down the road. For example\, traditional could be 0\, locale could be 1\, and unicode could be 2. This takes two bits for 3 values\, with room for a fourth\, instead of a bit for each value. But\, can/do external programs rely on the status quo with a bit for each thing?
It was most convenient to change the definitions to use eg.\, 1\<\<2 instead of 0x4. That meant I had to change the code that reads and evals those definitions to understand the left shift syntax.
I went out of the way to ensure binary compatibility with this patch. The actual bit structures haven't changed. Once a version of this is installed\, it will be easy to move things around to make room for the new bits needed.
This patch seems to be warnocked\, so I'm trying again\, changing the subject line to hopefully attract more interest. I had held off submitting it late in 5.13.3 to early 5.13.4 at the release manager's request because I thought it would break binary compatibility. But now we are well into the new cycle; I wouldn't want to miss this cycle too. I ended up splitting the patch so that the part I'm currently submitting doesn't break binary compatibility; that part is yet to come.
This patch is the first step in trying to add new regex modifiers\, and then fix Unicode handling in regexes.
The commits still apply cleanly. A rebased version is at github git://github.com/khwilliamson/perl.git branch match_sw
Currently the structures in op.h and regexp.h share common values\, which manually must be kept in sync. This patch creates a new header with those common values that is #included from the two affected header files. There are commits for ancillary small bug fixes and added comments I did as I went along.
I'm not sure if this common header is the best way to approach the matter\, but it works. I'm open to other ideas.
I left the two different symbols for each value because I wasn't sure what to do with them\, since they are both exported by B\, and I presume that means that external programs may be depending on them. I see that B has some explicit definitions for backward compatibility\, so I'm thinking it would be better to remove the duplicate symbols in the core\, and then define them there for external programs. But I don't know enough to know if that is reasonable or not. Please advise me.
It was most convenient to change the definitions to use eg.\, 1\<\<2 instead of 0x4. That meant I had to change the code that reads and evals those definitions to understand the left shift syntax.
I went out of the way to ensure binary compatibility with this patch. The actual bit structures haven't changed. Once a version of this is installed\, it will be easy to move things around to make room for the new bits needed.
Thanks\, applied to bleadperl. (I see that the only patch that needed some reviewing was the last one)
On 28 July 2010 18:37\, karl williamson \public@​khwilliamson\.com wrote:
This patch seems to be warnocked\, so I'm trying again\, changing the subject line to hopefully attract more interest. I had held off submitting it late in 5.13.3 to early 5.13.4 at the release manager's request because I thought it would break binary compatibility. But now we are well into the new cycle; I wouldn't want to miss this cycle too. I ended up splitting the patch so that the part I'm currently submitting doesn't break binary compatibility; that part is yet to come.
This patch is the first step in trying to add new regex modifiers\, and then fix Unicode handling in regexes.
The commits still apply cleanly. A rebased version is at github git://github.com/khwilliamson/perl.git branch match_sw
Currently the structures in op.h and regexp.h share common values\, which manually must be kept in sync. This patch creates a new header with those common values that is #included from the two affected header files. There are commits for ancillary small bug fixes and added comments I did as I went along.
I'm not sure if this common header is the best way to approach the matter\, but it works. I'm open to other ideas.
I left the two different symbols for each value because I wasn't sure what to do with them\, since they are both exported by B\, and I presume that means that external programs may be depending on them. I see that B has some explicit definitions for backward compatibility\, so I'm thinking it would be better to remove the duplicate symbols in the core\, and then define them there for external programs. But I don't know enough to know if that is reasonable or not. Please advise me.
It was most convenient to change the definitions to use eg.\, 1\<\<2 instead of 0x4. That meant I had to change the code that reads and evals those definitions to understand the left shift syntax.
I went out of the way to ensure binary compatibility with this patch. The actual bit structures haven't changed. Once a version of this is installed\, it will be easy to move things around to make room for the new bits needed.
The RT System itself - Status changed from 'new' to 'open'
@rgs - Status changed from 'open' to 'resolved'
Migrated from rt.perl.org#76586 (status was 'resolved')
Searchable as RT76586$