Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:99039
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.223.174 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAEZPtU5i5+LEmQkHgh1MAJ4XQi3uzaoeiQRJrNvp5Z26sE0kZA@mail.gmail.com>
References: <CAOWwgpm3Sq0nLeVcgXdGAjtcaQGuLgK=oTJRWFvQay4dDUWiXw@mail.gmail.com>
 <CAEZPtU5i5+LEmQkHgh1MAJ4XQi3uzaoeiQRJrNvp5Z26sE0kZA@mail.gmail.com>
Date: Mon, 15 May 2017 13:36:34 +0200
Message-ID: <CAOWwgpkcDqWUX=KAYXC73bB_4k66rG+AjPpCX8kEnUaWnJ69vA@mail.gmail.com>
To: Pierre Joye <pierre.php@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="94eb2c11bf86daeed1054f8e78e8"
Subject: Re: [PHP-DEV] BC break in master (7.2) - Distinguish between
 unmatched subpatterns and empty matches in preg_*()
From: nicolas.grekas@gmail.com (Nicolas Grekas)

--94eb2c11bf86daeed1054f8e78e8
Content-Type: text/plain; charset="UTF-8"

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

--94eb2c11bf86daeed1054f8e78e8--