Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114231
MIME-Version: 1.0
References: <CA+p1Faa1sg4pKLixrzXo1F8D8zRx1GoMCqCDDgh9GhCyhEuSOw@mail.gmail.com>
 <CAFPFaMJhU02akee_Rw-F6X8cCp=s0gmyGBECtyB+FFqmYgS0NQ@mail.gmail.com>
In-Reply-To: <CAFPFaMJhU02akee_Rw-F6X8cCp=s0gmyGBECtyB+FFqmYgS0NQ@mail.gmail.com>
Date: Wed, 28 Apr 2021 12:24:29 +0100
Message-ID: <CA+p1FaaAFDnKy5xE_bUBch-5cOWFa9YSLcXAkL-vtNLym7Sdhg@mail.gmail.com>
To: "G. P. B." <george.banyard@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000ece12305c1069f0f"
Subject: Re: [PHP-DEV] PR for minor bugfix in compact()
From: davidgebler@gmail.com (David Gebler)

--000000000000ece12305c1069f0f
Content-Type: text/plain; charset="UTF-8"

Ah I see, thanks for the clarification! I only did it this way because when
I did my RFC for fsync, I recall one or two people mentioned they weren't
keen on the bit where it could raise a warning rather than a TypeError -
but I appreciate that one was a completely new function and not a change to
an existing one (and it went through with the warning condition in the end,
anyway).

On Wed, 28 Apr 2021, 12:18 G. P. B., <george.banyard@gmail.com> wrote:

> On Wed, 28 Apr 2021 at 12:12, David Gebler <davidgebler@gmail.com> wrote:
>
>> Hi internals,
>> I've opened a PR to cause compact() to throw a TypeError if its parameters
>> are not valid, which I consider to be a fix for what is effectively a bug
>> whereby logical errors in user code can be silently swallowed.
>>
>> GPB has done an initial review and left a comment
>> https://github.com/php/php-src/pull/6921#pullrequestreview-646848902 in
>> which he suggests I open this up to the floor, so here it is, seeking
>> your feedback kindly. Also if anyone can clarify what is meant by a
>> warning
>> "will be promoted in PHP 9", I am not familiar with what changes are
>> planned for the next major version?
>>
>> Regards
>> David
>>
>
> Hey David,
>
> What I meant is that we usually don't introduce an exception without prior
> warning to existing functionality.
> So making this a Warning in the PHP 8 series which gets promoted to a
> TypeError in PHP 9,
> similarly to how most of the internal functions went from returning null +
> warning to throwing a TypeError,
> or many of the warnings that got promoted to ValueErrors in 8.0 because
> it's a major release.
>
> Personally I don't mind introducing the TypeError immediately in PHP 8.1,
> because compact() should be rather rare
> and mostly used on an array, but others might feel differently about this.
>
> Best regards,
>
> George P. Banyard
>

--000000000000ece12305c1069f0f--