Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126386
Precedence: bulk
MIME-Version: 1.0
References: <a995ac24905030e168e7f6006b996cd5@bastelstu.be>
 <CAMzRQTcTrZR2=vnjvVZRt=fEJpiBci9HN36BQAwG8+FK60Gc1A@mail.gmail.com> <5c142df3-94f8-45ba-b5c6-af3b4f7caa8b@varteg.nz>
In-Reply-To: <5c142df3-94f8-45ba-b5c6-af3b4f7caa8b@varteg.nz>
Date: Thu, 13 Feb 2025 09:16:49 +0100
Message-ID: <CAJmjfvUQEEnEVpjFTr_Kkzsjq5f1bfwtPDkKPhg=hXq-0EpzsQ@mail.gmail.com>
Subject: Re: [PHP-DEV] Re: RFC: Marking return values as important (#[\NoDiscard])
To: tim@bastelstu.be, volker@tideways-gmbh.com
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary="000000000000a92124062e01ae42"
From: jakob@givoni.dk (Jakob Givoni)

--000000000000a92124062e01ae42
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

My thoughts overall on this:

0. I'm not against introducing the attribute, only how it's used/enforced
etc. (And, incidentally, what it actually MEANS, see if you can spot that
from the rest of my comments....)

1. Static analysers and IDEs: I agree with everyone who's said that whether
or not the return value is "used" should be a concern for tooling like
static analysers and the IDE, and ideally not for PHP runtime.
Attributes were added as a structured replacement for docblock props and I
don't like it when they affect how a program actually runs (as long as
you're not using reflection).

2. If it must be a concern for PHP at runtime, a warning is preferred over
an exception. It only makes sense =E2=80=93 it's in the name: Exceptions ar=
e for
exceptional situations, warnings are for warning you.

3. Naming of the attribute: I think the most precise name (if I understand
the purpose correctly) is: #[ReturnValueMayContainCriticalInformation]

4. Should all internal functions, such as fopen() (which may return false
on error instead of throwing an exception), have this attribute?
I understand that ideally we only want to add it where the failure to
inspect the return value could result in something outright dangerous, but
I think that's a really hard call to make.
If we scale it back to mean "Look, this function may not behave the way
you'd expect it to, - the return value may actually contain some
information you might be interested in, so you better check it man", then I
think it could be a small but useful addition.

Best,
Jakob

--000000000000a92124062e01ae42
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>My thoughts overall on this:</div><div><br></div><div=
>0. I&#39;m not against introducing the attribute, only how it&#39;s used/e=
nforced etc. (And, incidentally, what it actually MEANS, see if you can spo=
t that from the rest of my comments....)<br class=3D"gmail-Apple-interchang=
e-newline"></div><div><br></div><div>1. Static analysers and IDEs: I agree =
with everyone who&#39;s said that whether or not the return value is &quot;=
used&quot; should be a concern for tooling like static analysers and the ID=
E, and ideally not for PHP runtime.<br>Attributes were added as a structure=
d replacement for docblock props and I don&#39;t like it when they affect h=
ow a program actually runs (as long as you&#39;re not using reflection).</d=
iv><div><br></div><div>2. If it must be a concern for PHP at runtime, a war=
ning is preferred over an exception. It only makes sense =E2=80=93 it&#39;s=
 in the name: Exceptions are for exceptional situations, warnings are for w=
arning you.<br><br></div><div>3. Naming of the attribute: I think the most =
precise name (if I understand the purpose correctly) is: #[ReturnValueMayCo=
ntainCriticalInformation]</div><div><br></div><div>4. Should all internal f=
unctions, such as fopen() (which may return false on error instead of throw=
ing an exception), have=C2=A0this attribute?</div><div>I understand that id=
eally we only want to add it where the failure to inspect the return value =
could result in something outright dangerous, but I think that&#39;s a real=
ly hard call to make.<br>If we scale it back to mean &quot;Look, this funct=
ion may not behave the way you&#39;d expect it to, - the return value may a=
ctually contain some information you might be interested in, so you better =
check it man&quot;, then I think it could be a small but useful addition.</=
div><div class=3D"gmail_quote gmail_quote_container"><div><br></div><div>Be=
st,</div><div>Jakob</div><div>=C2=A0</div></div></div>

--000000000000a92124062e01ae42--