Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126566
Message-ID: <e2f31f38-1f8f-407f-8552-824a8f171d66@bastelstu.be>
Date: Tue, 4 Mar 2025 23:22:42 +0100
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PHP-DEV] Re: RFC: Marking return values as important
 (#[\NoDiscard])
To: =?UTF-8?Q?Alexandru_P=C4=83tr=C4=83nescu?= <drealecs@gmail.com>
Cc: "Marc B." <marc@mabe.berlin>, Volker Dusch <volker@tideways-gmbh.com>,
 php internals <internals@lists.php.net>
References: <55b798d4-dcd7-4d81-852b-b78294cdc39d@email.android.com>
 <1a980468-c5f3-485b-9e8b-8c97d6ac4802@bastelstu.be>
 <CAAwdEzD7x=HccDQ9ckz+v=1nBm9NCY1i4GPizLoZsUZQ7FDFEA@mail.gmail.com>
Content-Language: en-US
In-Reply-To: <CAAwdEzD7x=HccDQ9ckz+v=1nBm9NCY1i4GPizLoZsUZQ7FDFEA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=C3=BCsterhus?=)

Hi

On 3/4/25 23:08, Alexandru Pătrănescu wrote:
> Just asking, as my engine knowledge is a bit limited,
> Wouldn't it be possible that when OPcache would optimize away the unused
> variable assigned to a function, it could detect that that function have
> the NoDiscard attribute and also remove the attribute effect (warning
> triggering).

Not with how the implementation currently works: It's checking whether 
or not the FCALL opcode’s result is used or not. If the assignment is 
removed, the FCALL result will be unused.

> Or identify that the function has the NoDiscard attribute and based on that
> do not optimize away the variable?

First things first: It's not a super important optimization to have, the 
assignment to a variable is one of the cheaper operations - and given 
that OPcache needs to know about the function it's also pretty limited 
in application. Of course you can also always just remove the assignment 
yourself (or ask your favorite IDE or static analyzer to point it out 
for you).

That said: Checking for the attribute and performing the optimization is 
likely possible, though likely of limited cost/benefit. In any case that 
would be an implementation detail that does not change observable 
behavior. The RFC defines that `(void)` or alternatively `$_` will be 
the solution that guarantees that the warning will be suppressed (which 
will then also be part of the documentation), how exactly that will work 
internally is something that can and will change when refactoring and 
evolving the engine and OPcache.

Best regards
Tim Düsterhus