Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:126249 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by qa.php.net (Postfix) with ESMTPS id A1DA61A00BC for ; Thu, 30 Jan 2025 19:59:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1738267005; bh=OLEfwsG+YFJ0w4d7kcchNxWU0bL4RDGqGSBbO3VrSuI=; h=Date:From:To:Cc:In-Reply-To:References:Subject:From; b=AZLLK0icd33pB1wO9vzoX/kwVbYjpXY8vhZwRgvB3sIHeAhLzs+Q6z2k9cS6neVXg MR1XgaIzc2D7gPI2lISX1PHXxLAHsr7D6t9wZz0xlpzti4RpzOfWe6rUuJeazJsaih yXuzXOHIlU53m31s/NzI7gcNUsQ/4Mx5Vm0nI0bKgsyPI4G48GZNL4yKjCVvaVeF6d sCD0pvestbtEjlQMsSjqTvzZPsc6Kbv3WIWKuItpOdAOrnMs7+QMbxvZ7kT16ekItE v8IeumdQFBcW27guiybv1o/7NnnZYZTXI/UNDHxAeGfJHMEy8rM85JBo4YwCiYINHV 9pTNaNtZakJPA== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id AB75E180088 for ; Thu, 30 Jan 2025 19:56:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-0.1 required=5.0 tests=BAYES_50,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,DMARC_MISSING,HTML_MESSAGE, RCVD_IN_DNSWL_LOW,SPF_HELO_PASS,SPF_PASS autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: No X-Envelope-From: Received: from fout-b2-smtp.messagingengine.com (fout-b2-smtp.messagingengine.com [202.12.124.145]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Thu, 30 Jan 2025 19:56:44 +0000 (UTC) Received: from phl-compute-01.internal (phl-compute-01.phl.internal [10.202.2.41]) by mailfout.stl.internal (Postfix) with ESMTP id 61DDA114011A; Thu, 30 Jan 2025 14:59:31 -0500 (EST) Received: from phl-imap-09 ([10.202.2.99]) by phl-compute-01.internal (MEProxy); Thu, 30 Jan 2025 14:59:31 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bottled.codes; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm1; t=1738267171; x= 1738353571; bh=OLEfwsG+YFJ0w4d7kcchNxWU0bL4RDGqGSBbO3VrSuI=; b=T fDMC1kVGX6DlgDO2mnE3bqUP/c/e0XHOywWkBPmmHwx3SFo8/OEe9jsas6C2DHjC HFRpiDfQ/7Lfz0SMSCuSyoLVC11rXdbaZeFwXwIlu0RFdl6gSE96aVCQoZeTCVuB 7r1jVQEqBwRFhso9ppiZ9wUD2Qwwq90jzYs7WAL0xLqQvmlZfWsRAVRCrNHaU1Eg 1SI+8L5FDW6LdH6AtJXgJ9BRiuIMyxcIjFXMfE3T81AvkQy1mIlT+tSn2UjaNVGO dnq0pRmoBbXoqQhXSCU6ABCgr77YaRfL2HiiZfyatEL8+OVFbCLK5R5eplb9THFT Q5ABeI9CDHIGgxF7EuY2Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1738267171; x=1738353571; bh=OLEfwsG+YFJ0w4d7kcchNxWU0bL4RDGqGSB bO3VrSuI=; b=E+NJlFYOw3IgNGkYLQxIZ0Z7rCVt6qrz5xg4/FjOIGgftmmVEJ0 Zij7As/lf0bDVVaWfdECLUKlQoymv8t7SuUUGUAVvZDlAn6154mZF7gi++NiC7tS VMiLIu75eDti90eNr3r33wjr2HSEV74STExBW+aYMTMEEhpQrufhDc9U8QTj+6JT FR+1Os5slf/nPDeO4BLTWiBTj7CHV7ZxA4bYlqgJCJAmyLZSrk2+t5nJTgQLrmbD ALailfGlFH7NkESi8qAga1OC5Wa7V5fuBBJETDnWobRyaY0nsZDdfgyu+GMe6+3Z PCDa3x7J6pCDmFqhhMMpO88OY6GjI8bBLPQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdeijeegucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepofggfffhvfevkfgjfhfutgesrgdtreerredtjeen ucfhrhhomhepfdftohgsucfnrghnuggvrhhsfdcuoehrohgssegsohhtthhlvggurdgtoh guvghsqeenucggtffrrghtthgvrhhnpeeiueethedvvdefjefhgfeiheelheehtdfhfeek jefflefgvedvkeduteejjedttdenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehrohgssegsohhtthhlvggurdgtohguvghspdhnsggprhgtphht thhopeefpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopehtihhmsegsrghsthgvlh hsthhurdgsvgdprhgtphhtthhopehinhhtvghrnhgrlhhssehlihhsthhsrdhphhhprdhn vghtpdhrtghpthhtohepvhholhhkvghrsehtihguvgifrgihshdqghhmsghhrdgtohhm X-ME-Proxy: Feedback-ID: ifab94697:Fastmail Received: by mailuser.phl.internal (Postfix, from userid 501) id B388F780068; Thu, 30 Jan 2025 14:59:30 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net x-ms-reactions: disallow MIME-Version: 1.0 Date: Thu, 30 Jan 2025 20:59:10 +0100 To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= Cc: "Volker Dusch" , "php internals" Message-ID: In-Reply-To: <412ea440edbfbfc10be2e39497648ad6@bastelstu.be> References: <3f170549-cfd6-441e-b892-cf51d726dbe8@bastelstu.be> <3b4d9be8-9255-44ab-95c5-ea3045212cf4@app.fastmail.com> <412ea440edbfbfc10be2e39497648ad6@bastelstu.be> Subject: Re: [PHP-DEV] RFC: Marking return values as important (#[\NoDiscard]) Content-Type: multipart/alternative; boundary=92920c684b564dba91263521604bf0c4 From: rob@bottled.codes ("Rob Landers") --92920c684b564dba91263521604bf0c4 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Thu, Jan 30, 2025, at 16:50, Tim D=C3=BCsterhus wrote: > Hi >=20 > Am 2025-01-29 21:16, schrieb Rob Landers: > > I understand what you are saying, but I can also just remove the=20 > > warning via: > >=20 > > $_ =3D outer; >=20 > Please note that `$_` is a regular variable and thus will increase the=20 > lifetimes of returned objects (and arrays that can contain objects),=20 > which might or might not be observable when implementing `__destruct()= `.=20 > Also note that OPcache will actually optimize away dead stores if it c= an=20 > guarantee that the difference is not observable (i.e. if it knows that=20 > the function never returns an object and if `get_defined_vars()` is no= t=20 > 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 obser= vable). 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 (wheth= er opcache is used or not) may not be ideal. For example: $lock =3D flock($fp); -if ($lock) { +if(true) { // do stuff This diff is rather obvious something went wrong here; but use your imag= ination to imagine a diff where this is not so clear. In this case $lock= accidentally becomes unused. If opcache is enabled and optimizes this c= ode, we get the warning we probably desire, but without opcache, we do n= ot get the warning (tests usually have opcache disabled)! >=20 > Of course it would be possible to exclude `$_` from this dead-store=20 > optimization with PHP 8.5+ to allow for a smoother migration for=20 > libraries that require cross-version compatibility. I believe the draft pattern matching RFC uses this variable name. It is = probably good to align there. >=20 > We nevertheless wanted to offer an alternative that might be less=20 > confusing than a =E2=80=9Cspecial variable=E2=80=9D that might also co= nflict with=20 > diagnostics like unused variable detection. >=20 > > [=E2=80=A6] It also happens to make the diffs more natural looking w= hen and if=20 > > the return value gets used during a code review. [=E2=80=A6] >=20 > Can you clarify what you mean by =E2=80=9Cmore natural looking=E2=80=9D? Here are two diffs: -(void)doSomething(); +$var =3D doSomething(); +somethingElse($var); or -$_ =3D doSomething(); +$var =3D 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 gi= thub. It is immediately clear what has changed: only the variable name i= s 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 Rus= t, Swift and Zig). For someone coming from more C/Typescript backgrounds= , they may disagree. =E2=80=94 Rob --92920c684b564dba91263521604bf0c4 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable
On Thu, Jan 30,= 2025, at 16:50, Tim D=C3=BCsterhus wrote:
Hi

Am 2025-= 01-29 21:16, schrieb Rob Landers:
> I understand what y= ou are saying, but I can also just remove the 
> w= arning via:
> 
> $_ =3D outer;

Please note that `$_` is a regular variable a= nd thus will increase the 
lifetimes of returned obje= cts (and arrays that can contain objects), 
which mig= ht or might not be observable when implementing `__destruct()`. 
Also note that OPcache will actually optimize away dead stor= es if it can 
guarantee that the difference is not ob= servable (i.e. if it knows that 
the function never r= eturns an object and if `get_defined_vars()` is not 
= called).

If $_ gets optim= ized away and causes the warning, then this seems like a (future) bug wi= th 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 th= e inconsistency (whether opcache is used or not) may not be ideal.

For example:

 = $lock =3D flock($fp);
-if ($lock) {
<= /span>
+if(true) {
// do stuff=

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


Of course it would be po= ssible to exclude `$_` from this dead-store 
optimiza= tion 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.
<= br>

<= div>We nevertheless wanted to offer an alternative that might be less&nb= sp;
confusing than a =E2=80=9Cspecial variable=E2=80=9D th= at might also conflict with 
diagnostics like unused = variable detection.

> [=E2=80=A6] It als= o happens to make the diffs more natural looking when and if 
> the return value gets used during a code review. [=E2=80=A6= ]

Can you clarify what you mean by =E2=80=9C= more natural looking=E2=80=9D?

Here are two diffs:

-(void)doSomething();
+$var =3D doSomething();
+som= ethingElse($var);

or

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

At the expense of adding some html... I= 've added syntax highlighting for those using html readers to show how i= t 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 f= rom more C/Typescript backgrounds, they may disagree.

=
=E2=80=94 Rob
--92920c684b564dba91263521604bf0c4--