Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105499
MIME-Version: 1.0
References: <CAP=4hCLPkQ64eKn0OoAB31qMiFs=S19fvBB0HLNQMqdCMM_XqQ@mail.gmail.com>
 <CAESVnVpE+jH50sMtxZVZfFdOdDps-9Kdd9MBTE1Sf4itkLu26A@mail.gmail.com>
In-Reply-To: <CAESVnVpE+jH50sMtxZVZfFdOdDps-9Kdd9MBTE1Sf4itkLu26A@mail.gmail.com>
Date: Mon, 29 Apr 2019 09:37:28 +0200
Message-ID: <CAF+90c8Gq3mmOoKzN8gNOi-bE66n6q+OY6JcDZUYH_FHVZWJgQ@mail.gmail.com>
To: Sara Golemon <pollita@php.net>
Cc: Gabriel Caruso <carusogabriel34@gmail.com>, PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000039f0690587a65cd6"
Subject: Re: [PHP-DEV] Don't silence chr function arguments error
From: nikita.ppv@gmail.com (Nikita Popov)

--00000000000039f0690587a65cd6
Content-Type: text/plain; charset="UTF-8"

On Mon, Apr 29, 2019 at 3:44 AM Sara Golemon <pollita@php.net> wrote:

> On Sun, Apr 28, 2019 at 6:05 PM Gabriel Caruso <carusogabriel34@gmail.com>
> wrote:
>
> > Currently, if you pass an argument that is not an integer, it will simply
> > return an empty string: https://3v4l.org/FF2nA.
>
>
> Not entirely accurate. It outputs a one-character string (the null byte).
>
>
> > In case you pass more than
> > 1 argument, it will complain about "Wrong number of arguments":
> > https://3v4l.org/Yrr43.
> >
> >
>
>
> > The proposal of https://github.com/php/php-src/pull/4080 is to relly
> these
> > checks to ZPP, making the first argument mandatory to an integer, and
> make
> > the error message for extra arguments consistent with the rest of the
> core
> > functions, e.g. "Warning: chr() expects parameter 1 to be int, string
> > given".
> >
> > I'm actually perfectly in favor of raising a proper warning here.  I
> noticed the odd behavior when I was porting this function to HHVM and IIRC
> I raised a discussion about it and the bottom line was "Let's not do this
> because X", and I couldn't possibly tell you what X was. :(
> The thread is out there somewhere though.
>
>
> > The idea is to merge in PHP-7.4, but if there's a consensus that this
> > should go into PHP 8 only, so it is :)
> >
> > Technically a BC break, so 8.0 would be the preferred branch IMO.
>

As this would be a TypeError on 8.0, I think it might make sense to
introduce it in 7.4 where this is still a warning (and should keep the
current "\0" return value in that case -- just make it throw a warning).

Nikita

--00000000000039f0690587a65cd6--