Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114033
MIME-Version: 1.0
References: <CAF+90c9crhPpSMcTx-ozGo0LGjNuBU43e8fiK_=PJMU6iTHNmw@mail.gmail.com>
 <28be190c-21ea-b796-cec7-b8db21d14eca@gmail.com>
In-Reply-To: <28be190c-21ea-b796-cec7-b8db21d14eca@gmail.com>
Date: Mon, 12 Apr 2021 14:55:47 +0200
Message-ID: <CAF+90c8n_Ny5h8JyF6jKc0i4Rg_=msKEa1Eum6rQJmcV2cGQvw@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000003f123005bfc6092c"
Subject: Re: [PHP-DEV] [RFC] Deprecations for PHP 8.1
From: nikita.ppv@gmail.com (Nikita Popov)

--0000000000003f123005bfc6092c
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 23, 2021 at 6:05 AM Stanislav Malyshev <smalyshev@gmail.com>
wrote:

> Hi!
>
>  > date_sunrise() and date_sunset()
>
> Do we have any information on usage? I am generally not a fan of
> deprecating functions that work - even if they are odd and have quirky
> APIs - but if the usage is essentially zero than it might be ok.
>

In my corpus of top 2k composer packages, the only real (not stub, metadata
etc) reference to these functions is in the DateTime wrapper from Zend
Framework 1:
https://github.com/zendframework/zf1/blob/136735e776f520b081cd374012852cb88=
cef9a88/library/Zend/Date/DateObject.php#L913
That's not quite zero, but close...

 > key(), current(), next(), prev(), and reset() on objects
>
> I'd be happier if those worked with iterators. Except for prev() which I
> don't think many people need.
>

Some thoughts on this topic:
 * These functions currently don't work with Iterators, so switching them
to use the Iterator interface rather than object properties will be a BC
break. Of course, we're looking at a BC break here in either case, but it's
usually preferable to make something stop working entirely, rather than
make it behave differently.
 * As these functions currently don't work on Iterators, the usual way to
work with arbitrary iterables is to normalize everything to Iterator, that
is convert arrays to Iterators via ArrayIterator. This works today and is
essentially the only way to handle all iterables if you need something more
than a simple foreach. Supporting Iterator in key() etc would add an
alternative way to do this that works only in PHP 8.1. Why would I choose
the new way over the old one?
 * There is a long-term goal to remove the "internal array pointer" concept
from arrays. In PHP 5 this was used for all iteration, but as of PHP 8 the
key() family of functions is the only part of PHP still using this concept.
With this in mind, I'm generally inclined to encourage people to migrate
away from using these functions, and make use of ArrayIterator instead, if
they need to iterate arrays in complex ways.

  > mb_check_encoding() without argument
>
> No objection.
>
>  > get_class(), get_parent_class() and get_called_class() without argumen=
t
>
> I'm not sure why. I mean if we want to make them return the same as
> self::class etc. - up to the point of actually compiling them as such -
> no problem, but I don't see why they need to be deprecated. And I
> vaguely remember seeing get_class() at least a bunch of times in old
> code, when ::class didn't even exist. I don't see a good reason why that
> code should be broken.
>

I agree with you on this one. From my perspective, there should be some
technical motivation for deprecations, and this one seems to be more about
coding style -- it removes an old way to write something, which has been
subsumed by a different construct in the meantime. I think M=C3=A1t=C3=A9 s=
uggested
this one, maybe he can provide additional reasoning.


>  > FILE_BINARY and FILE_TEXT constants
>
> No objection.
>
>  > t fopen mode
>
> I'm afraid there's - despite the warning - a bunch of code for Windows
> that relies on "t" and I don't think we should be breaking it. Is there
> a good reason to drop this mode?
>
>  > Passing bool for $amountOrUpOrDown argument of IntlCalendar::roll()
>  > Accessing static members on traits
>
> No objection.
>
>  > strptime()
>  > strftime() and gmtstrftime()
>
> We have to remember many applications do not need to be portable, as
> they will ever be only run on one setup - the one that the company
> running it has. So non-portability itself should not be a fatal problem.
> I am worried by musl thing mentioned - what exactly happens on musl,
> they don't have strptime()? If it's just different outputs or some
> options not supported, I think it's ok - warning in the docs should be
> enough.
>

musl does have both strftime() and strptime(), but not with quite the same
behavior as glibc. It's been a while, but for strftime() I believe the main
issue was that %Z does not work. It's theoretically supported by musl, but
the implementation is "maliciously compliant". At the time, I implemented
some manual expansion for %Z to paper over the difference, but we
ultimately decided that we shouldn't be trying to fix platform behavior in
this way. Based on
https://github.com/php/doc-en/commit/6936064e733f39045f471aad492abdeed7c4d2=
c6
the %P format also doesn't work.

Based on my dataset of composer packages, there is not a single usage of
strptime(), but strftime() does have around 150 references, including in
Symfony and Drupal. I think my inclination here is to drop strftime() from
the RFC and only keep strptime(). After all, strftime() is relatively
portable if you avoid certain formats. strptime() is the one that's really
non-portable, because it doesn't even have Windows support.


>  > mhash*() function family
>
> No objection.
>
>  > ctype_*() function family accepts int parameters
>
> Here I think adding notice for int arguments would be appropriate, but
> changing the behavior is not - we could cause some very nasty breaks in
> the code if we suddenly change such a basic thing.
>

Hm, yes, I can understand your concern here. I think it's worth pointing
out though that we have already gone through this process for quite a few
other functions already. E.g. passing a codepoint to strpos() was
deprecated in PHP 7.3 and then the behavior changed in PHP 8.0 to always
treat the input as a string. The current proposal is to do the same thing
here.


>  > Return by reference with void type
>  > NIL constant defined by the IMAP extension
>
> No objection.
>
>  > Calling overloaded pgsql functions without the connection argument
>
> I hate global state, but there are a lot of old quick-n-dirty scripts
> relying on stuff like that. Can we maybe check how common is usage of
> this pattern?
>

I checked my composer dataset for uses of pg_query() and pg_prepare()
without connection, and the only hits were the function wrappers in the
safe library, which just mirror whatever PHP does (
https://github.com/thecodingmachine/safe/blob/4cc05ada622d30746f573695f748d=
44585981efa/generated/pgsql.php#L1446).
Of course, this is a case where it's more likely that such usage is found
in applications rather than libraries (as libraries cannot assume no other
connections exist), so this result should be taken with a grain of salt.

Nikita

--0000000000003f123005bfc6092c--