On Thu, Jan 30, 2025, at 16:50, Tim Düsterhus wrote:
> Hi
> 
> Am 2025-01-29 21:16, schrieb Rob Landers:
> > I understand what you are saying, but I can also just remove the 
> > warning via:
> > 
> > $_ = outer;
> 
> Please note that `$_` is a regular variable and thus will increase the 
> lifetimes of returned objects (and arrays that can contain objects), 
> which might or might not be observable when implementing `__destruct()`. 
> Also note that OPcache will actually optimize away dead stores if it can 
> guarantee that the difference is not observable (i.e. if it knows that 
> the function never returns an object and if `get_defined_vars()` is not 
> called).

If $_ gets optimized away and causes the warning, then this seems like a 
(future) bug with opcache (it optimized something away and it was observable). 
It also shouldn't matter if you use $_, $unused, or $whatever; I just chose 
that as an example as someone who came from C# + Scala + Go lineage.

Some might say that is a feature, though... and the inconsistency (whether 
opcache is used or not) may not be ideal.

For example:

 $lock = flock($fp);
-if ($lock) {
+if(true) {
// do stuff

This diff is rather obvious something went wrong here; but use your imagination 
to imagine a diff where this is not so clear. In this case $lock accidentally 
becomes unused. If opcache is enabled and optimizes this code, we get the 
warning we probably desire, but without opcache, we do not get the warning 
(tests usually have opcache disabled)!

> 
> Of course it would be possible to exclude `$_` from this dead-store 
> optimization with PHP 8.5+ to allow for a smoother migration for 
> libraries that require cross-version compatibility.

I believe the draft pattern matching RFC uses this variable name. It is 
probably good to align there.

> 
> We nevertheless wanted to offer an alternative that might be less 
> confusing than a “special variable” that might also conflict with 
> diagnostics like unused variable detection.
> 
> > […] It also happens to make the diffs more natural looking when and if 
> > the return value gets used during a code review. […]
> 
> Can you clarify what you mean by “more natural looking”?

Here are two diffs:

-(void)doSomething();
+$var = doSomething();
+somethingElse($var);

or

-$_ = doSomething();
+$var = doSomething();
+somethingElse($var);

At the expense of adding some html... I've added syntax highlighting for those 
using html readers to show how it might look in something like github. It is 
immediately clear what has changed: only the variable name is highlighted as a 
change on that line, letting me know what to look for. Though, as I mentioned, 
I come from a lineage of languages where _ is a thing, so it looks more natural 
to me (other languages using _ are Rust, Swift and Zig). For someone coming 
from more C/Typescript backgrounds, they may disagree.

— Rob

Reply via email to