Re: BC break in master (7.2) - Distinguish between unmatched subpatterns and empty matches in preg_*()

Thanks Pierre, 2017-05-15 12:12 GMT+02:00 Pierre Joye <pierre.php@gmail.com>: > hi Nicolas, > > On Mon, May 15, 2017 at 1:57 PM, Nicolas Grekas > <nicolas.grekas@gmail.com> wrote: > > Hi, > > > > in master, feature request https://bugs.php.net/61780 has > > been > implemented > > and merged into master thanks to > > https://github.com/php/php- > src/pull/1303 > > > > But as noted in the PR and even in the UPGRADING file, this is a BC > break: > > . preg_match() and other PCRE functions now distinguish between > unmatched > > subpatterns and empty matches by reporting NULL and "" (empty > string), > > respectively. Formerly, either was reported as empty string. > > > > While trying to run Symfony's test suite against 7.2, we noticed that > this > > BC break is hitting several components badly. If Symfony is hit, there > will > > be many more userland code impacted for sure. > > > > As written explicitly in the releasing policy of php-src, BC breaks must > > not happen in minor versions. Therefore, I'm kindly but strongly asking > for > > this BC break to be reconsidered and removed. > > I agree that a minimum 5 years old possible bug, quite small, causing > BC breaks is not good. As far as the fix is critical or justified, I > think it is sometimes ok to break BC for edge cases. However I do not > see this is not the case here. > > @RMs revert for 7.2 and re evaluate for later? Another possibility would be to make this opt-in, with a new flag. FYI, I quickly spotted at least two places in the code base where we check the matches with something like: if ('' !== $matches[2]) {...} else {...} This should be a pretty common check so the BC break will impact more userland code when released. Cheers, Nicolas