Bug #73948 [Ana]: Preg_match_all should return NULLs on trailing optional capture groups.

From: Date: Mon, 23 Jan 2017 18:31:20 +0000
Subject: Bug #73948 [Ana]: Preg_match_all should return NULLs on trailing optional capture groups.
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-206903@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=73948&edit=1 ID: 73948 Updated by: cmb@php.net 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: > But why would you need an RFC for a bugfix? An RFC might be over the top, but at least a PR to get some attention appears to be appropriate. Previous Comments: ------------------------------------------------------------------------ [2017-01-23 17:53:31] tomasyorke at hotmail dot com If you wish to delay this change until 7.2, that sounds fine. But why would you need an RFC for a bugfix? ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ 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

« previous php.bugs (#206903) next »