Re: [pcre-dev] Handle PCRE_JCHANGED more exactly

Startseite
Nachricht löschen
Autor: Philip Hazel
Datum:  
To: Stepan Kasal
CC: pcre-dev
Betreff: Re: [pcre-dev] Handle PCRE_JCHANGED more exactly
On Fri, 23 Nov 2007, Stepan Kasal wrote:

> I noticed that PCRE_JCHANGED can be set even though the option 'J'
> does not actually change.


The name "CHANGED" is perhaps wrong. Maybe it should have been "USED"
instead.

> Actually, I think that ``J changed'' is not exactly the information
> which is needed. Function get_first_set can perform its
> optimization, unless a subpattern name was used more than once.
> Let's call this bit PCRE_DUPNAMEUSED; it's easy to trace this info.
>
> So I wrote a patch which replaces PCRE_JCHANGED by PCRE_DUPNAMEUSED.


I don't like replacing things. You always catch somebody. I see your
point, but I'm not sure if it is actually worth doing anything about it.
(I wasn't entire sure it was worth introducing the optimization in the
first place, if I am entirely honest; but I did it so that the
performance of previously-existing patterns would not alter. Perhaps I
should have done some timing to see if it actually made any difference
in practice.)

> But then I discovered that PCRE_INFO_JCHANGED is part of the
> documented interface. That's unfortunate,


I do not like having undocumented ("hidden") features in software. It
always leads to problems.

> Wouldn't it be possible to admit that PCRE_INFO_JCHANGED was a bad
> idea and remove it from the documented interface? It's not that long
> since it was published, after all.


The change that was made for 7.4 was made because a user requested it.
So I think we can assume that somebody is making use of this feature.
Therefore, removing it will cause at least one person pain, and this
seems like a bad idea. I don't think that fact that it is relatively new
(June 2007) is relevant

I should perhaps improve the documentation to point out that PCRE_INFO_
JCHANGED really means "(?J) or (?-J) was used somewhere in the pattern".

Philip

--
Philip Hazel