Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:106163
MIME-Version: 1.0
References: <CAJW__o0GLjcagzF0ZpEK54yyhkFpgV6tm4p+ihnZvv-dC3YFUQ@mail.gmail.com>
 <09a8bba3-d17a-a8e5-f9b7-cc3d65f1e66c@gmail.com> <e67ff0b8-faba-9423-8ef1-69910eea6520@gmail.com>
 <CAF+90c-6ssx7ZXS34D4mUjE98PSnOqd6RvGuQes+xLhN0r-2rA@mail.gmail.com>
In-Reply-To: <CAF+90c-6ssx7ZXS34D4mUjE98PSnOqd6RvGuQes+xLhN0r-2rA@mail.gmail.com>
Date: Mon, 8 Jul 2019 12:27:49 +0200
Message-ID: <CAF+90c9pRX5LWpo=HOyXaHPapsWJn4vB5DCjOXx8JXOF-vmWkA@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: Kalle Sommer Nielsen <kalle@php.net>, Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000475b7b058d28e6cf"
Subject: Re: [PHP-DEV] [RFC] Deprecations for 7.4 - specifics
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000475b7b058d28e6cf
Content-Type: text/plain; charset="UTF-8"

On Fri, Jul 5, 2019 at 4:37 PM Nikita Popov <nikita.ppv@gmail.com> wrote:

> On Sat, Jun 22, 2019 at 10:21 PM Stanislav Malyshev <smalyshev@gmail.com>
> wrote:
>
>> Hi!
>>
>> enable_dl - I am not clear what is proposed - to remove dl()
>> functionality in PHP 8 completely, even in CLI? Or to keep it in CLI
>> always on and remove the directive?
>> My opinion: drop the directive, let the SAPI decide if dl() is possible,
>> with CLI keeping it. Not sure about embed though. But I guess if
>> somebody needs it in embed, they probably could patch it so it enables
>> the dl(). Not sure how easy it would be to make this per-SAPI decision
>> though...
>>
>
> This is indeed the plan and I've clarified the RFC to say so. While doing
> this I double-checked the implementation and found that currently dl() is
> available not only in CLI and Embed, but also in CGI.
>
> I think in light of this, dropping enable_dl may make less sense, as the
> premise here was more of less that dl() is not available in server SAPIs
> anyway. Or maybe the argument has to be shifted towards "use
> disable_functions=dl instead, like with all other functions".
>
> Kalle, what do you think?
>
>
>> The 'real' type - see no reason to touch it, it doesn't hurt anybody.
>>
>> hebrev, hebrevc, apache_request_headers, is_writeable,
>> convert_cyr_string, money_format, restore_include_path - leave them in
>> peace (argued why in another thread). Maybe legacy extension as Zeev
>> suggested, I'm fine with that.
>>
>
> money_format should really go away due to platform support reasons.
> Windows has no implementation and the macos implementation appears to be
> buggy (memory corruption). It nearly seems like PHP is the only major
> remaining user of this function.
>
> Magic quotes and all related - burn with fire. Magic quotes was a
>> mistake, and here my BC inner voice is not able to say anything in its
>> favor.
>>
>> array_key_exists(object) - fine with dropping object behavior. It does
>> say "array".
>> And btw, can we actually make it work with ArrayAccess properly?
>>
>
> Not really. Despite the name ArrayAccess::offsetExists() really has
> isset() semantics, so there isn't anything that exposes pure existence
> (without null check) in the interface.
>
>
>> INPUT_SESSION & INPUT_REQUEST - if they weren't implemented, drop them.
>> No point in keeping stuff that doesn't work.
>>
>> register_argc_argv - support making $argc/$argv always available in CLI
>> but not sure about the definition of CLI. Does CGI qualify? Does embed?
>> This worries me a bit. Same situation as in dl() - it's like we need
>> some ini setting that is only accessible to SAPIs... Maybe we should
>> introduce new INI level that is only settable by SAPI but not changeable
>> later? Just brainstorming here.
>>
>
> Good question. Reading the code, I think the current behavior is that
> anything that specifies SG(request_info).argc gets proper $argv/$argc
> (these will be preserved and always available), while anything else gets
> fake $argv/$argc based on GET parameters (which will be removed and never
> available).
>
> A quick grep shows these SAPIs as setting SG(request_info).argc: CLI, CGI,
> Embed, Litespeed, phpdbg. So it's not even really a matter of CLI vs server
> SAPIs, just whether command line args are provided by the SAPI or not.
>
> I feel like overall this change is not really a great fit for this
> deprecation RFC, and we should probably just land these changes in PHP 8.
>
>
>> Reflection export() - check if any real code is using it. Reflection
>> printing is kinda-sorta is the area that BC breaks may be acceptable,
>> since it's unlikely production code paths would depend on it, but we
>> need to check it. Undecided so far.
>>
>> mb_strrpos - if we promised to deprecate, we may as well do it.
>>
>> implode() - leave it alone, yes I know it's weird order, but messing
>> with it helps nobody. Yes, I know, "inconsistent". Making it
>> "consistent" will improve the life of exactly 0 people. Please let it be.
>>
>
> I have a related question on this one: Apparently (TIL) implode() also
> accepts just an array, in which case glue="". Kalle, will this form also be
> deprecated, or does it remain?
>
> ezmlm_hash - undecided... it's really the niche of the niche. It also
>> doesn't hurt anyone, but not sure if anybody actually using it anymore.
>>
>> allow_url_include - as I said, needs separate RFC. I am leaning towards
>> removing it, but maybe there's some use case for it...
>>
>> Unbinding $this - already wrote about it, doesn't look like we have a
>> choice there given that what it is trying to do seems not to be
>> supported in PHP 8. So there may not be a point in even voting on it,
>> unless I misunderstand the explanation. If we need a vote, may be a good
>> idea to separate this one either.
>>
>
> To be clear: We can (and currently do) support this in PHP 8. It's just a
> question of performance. Thanks to the removal of calling static methods
> non-statically, we can now remove the NULL check that occurs on every
> single property acces, method call, etc. If we do, it will currently come
> at the cost of a separate opcode for each $this access in a closure. If we
> remove the ability to unbind $this, we could do this without imposing a
> cost in any common scenario (the only one would be when using $this outside
> a class, e.g. in templates, which is not important).
>
> Nikita
>

I have now made the following changes to the RFC:

 * Removed enable_dl deprecation. The fact that dl() is currently available
by default on CGI, which is a server SAPI, makes this more dicey and needs
more careful consideration. As this RFC needs to go to vote today, I'm
going with the conservative option of dropping it from the RFC.
 * Removed register_argc_argv change from the RFC. This was not really a
deprecation, so does not belong here. We'll likely just want to make the
necessary changes in PHP 8.
 * Removed the INPUT_SESSION + INPUT_REQUEST deprecations. These have been
warning since forever, so going through an additional deprecation phase
makes no sense. I went ahead and removed them in PHP 8.
 * For the deprecation of implode() with inverted parameter order,
explicitly point out that the single-argument form is not deprecated.
 * Various text improvements that do not change the content of the RFC.

I will start voting on this RFC later today so we make feature freeze.

Nikita

--000000000000475b7b058d28e6cf--