Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:106158
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>
In-Reply-To: <e67ff0b8-faba-9423-8ef1-69910eea6520@gmail.com>
Date: Fri, 5 Jul 2019 16:37:09 +0200
Message-ID: <CAF+90c-6ssx7ZXS34D4mUjE98PSnOqd6RvGuQes+xLhN0r-2rA@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="0000000000007565f7058cf008c6"
Subject: Re: [PHP-DEV] [RFC] Deprecations for 7.4 - specifics
From: nikita.ppv@gmail.com (Nikita Popov)

--0000000000007565f7058cf008c6
Content-Type: text/plain; charset="UTF-8"

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

--0000000000007565f7058cf008c6--