Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105490
MIME-Version: 1.0
References: <CAP=4hCLPkQ64eKn0OoAB31qMiFs=S19fvBB0HLNQMqdCMM_XqQ@mail.gmail.com>
In-Reply-To: <CAP=4hCLPkQ64eKn0OoAB31qMiFs=S19fvBB0HLNQMqdCMM_XqQ@mail.gmail.com>
Date: Sun, 28 Apr 2019 20:43:28 -0500
Message-ID: <CAESVnVpE+jH50sMtxZVZfFdOdDps-9Kdd9MBTE1Sf4itkLu26A@mail.gmail.com>
To: Gabriel Caruso <carusogabriel34@gmail.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000de74200587a169bc"
Subject: Re: [PHP-DEV] Don't silence chr function arguments error
From: pollita@php.net (Sara Golemon)

--000000000000de74200587a169bc
Content-Type: text/plain; charset="UTF-8"

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.

-Sara

--000000000000de74200587a169bc--