Edit report at https://bugs.php.net/bug.php?id=73948&edit=1
ID: 73948
Comment by: tomasyorke at hotmail dot com
Reported by: tomasyorke at hotmail dot com
Summary: Preg_match_all should return NULLs on trailing
optional capture groups.
Status: Analyzed
Type: Bug
Package: PCRE related
Operating System: Windows 7
PHP Version: 7.0.14
Block user comment: N
Private report: N
New Comment:
If you wish to delay this change until 7.2, that sounds fine.
But why would you need an RFC for a bugfix?
Previous Comments:
------------------------------------------------------------------------
[2017-01-23 17:43:39] tomasyorke at hotmail dot com
I'm glad we didn't go the unset route. Having fixed sized arrays is useful for looping by
groups/matches independently of the Flag used.
You also can very easily know how many groups/matches the regex returned/contained.
By making the output array dynamicly sized. You only allow for an easy way to determine one of these
things. While making the other more complex.
------------------------------------------------------------------------
[2017-01-23 17:17:20] cmb@php.net
Regarding the BC break: when fixing <https://bugs.php.net/61780> I
would have preferred to unset unmatched captures, but that
appeared too much of a BC break (consider somebody is count()ing
the $matches). Therefore I changed the empty strings to NULL.
Adding unmatched trailing captures as NULL would still have the
same BC concerns â I don't think we can do that before PHP 7.2,
and even that might require the RFC process.
------------------------------------------------------------------------
[2017-01-23 15:59:31] tomasyorke at hotmail dot com
Regarding BC
The proposed change would be to change the representation of missed capture groups from 2
representations to 1 representation.
The only code that would break is code that supported only the more obscure representation that only
appears with trailing groups. And if that code exists, then it will still break on non trailing
missing groups even without the change.
------------------------------------------------------------------------
[2017-01-23 15:48:19] tomasyorke at hotmail dot com
As noted by cmb, this bug does not appear with the PREG_PATTERN_ORDER flag.
Thus I switched the flag and my code to use PREG_PATTERN_ORDER and implemented my own
preg_set_order() function.
But I found a nearly identical bug with the PREG_OFFSET_CAPTURE flag.
Consider the code
preg_match_all("#(a)?(b)(c)?#","b",$matches,PREG_OFFSET_CAPTURE);
var_dump($matches);
Returns
array(4) {
[0] => array(1) {
[0] => array(2) {
[0] => string(1) "b" [1] => int(0)
}
} [1] => array(1) {
[0] => array(2) {
[0] => string(0) "" [1] => int(-1)
}
} [2] => array(1) {
[0] => array(2) {
[0] => string(1) "b" [1] => int(0)
}
} [3] => array(1) {
[0] => string(0) ""
}
}
As you can see, the third optional capture group returns something different than the first optional
capture group.
This is too a bug. Again, I don't care what representation is chosen. But it must be the same
for any optional capture no matter if trailing or not.
------------------------------------------------------------------------
[2017-01-17 13:28:22] cmb@php.net
For PREG_PATTERN_ORDER unmatched subpatterns are already added[1];
something like that would also have to be done for PREG_SET_ORDER.
However, that would cause BC issues, and adding another flag isn't
appealing to me. I also think that NULL and unset are close enough
to stick with the current behavior for now; checking isset()
would suffice.
[1] <https://github.com/php/php-src/blob/PHP-7.1.0/ext/pcre/php_pcre.c#L862-L871>
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=73948
--
Edit this bug report at https://bugs.php.net/bug.php?id=73948&edit=1