Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126465
Precedence: bulk
MIME-Version: 1.0
Date: Thu, 20 Feb 2025 16:44:19 +0100
To: Jakob Givoni <jakob@givoni.dk>
Cc: volker@tideways-gmbh.com, internals@lists.php.net
Subject: Re: [PHP-DEV] Re: RFC: Marking return values as important
 (#[\NoDiscard])
In-Reply-To: <CAJmjfvXx5wvN5rHEHjEXdV+2U0RFL-tN7NsSQMVWjhhLPNo=3g@mail.gmail.com>
References: <a995ac24905030e168e7f6006b996cd5@bastelstu.be>
 <CAMzRQTcTrZR2=vnjvVZRt=fEJpiBci9HN36BQAwG8+FK60Gc1A@mail.gmail.com>
 <5c142df3-94f8-45ba-b5c6-af3b4f7caa8b@varteg.nz>
 <CAJmjfvUQEEnEVpjFTr_Kkzsjq5f1bfwtPDkKPhg=hXq-0EpzsQ@mail.gmail.com>
 <fabda1c573ff3c605c2b8da3e74eabf5@bastelstu.be>
 <CAJmjfvXx5wvN5rHEHjEXdV+2U0RFL-tN7NsSQMVWjhhLPNo=3g@mail.gmail.com>
Message-ID: <1e29b3153978bbb01357e2c41a375be3@bastelstu.be>
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

Am 2025-02-14 10:10, schrieb Jakob Givoni:
> I think all the native attributes so far, with the exception of
> #[\AllowDynamicProperties],
> only affect the program at compile-time, or by emitting errors.
> They don't affect the program otherwise by changing behavior,
> throwing exceptions etc. (As long as you don't use reflection, nor 
> promote
> errors to exceptions.)
> 
> AllowDynamicProperties is the only one that may change the program 
> behavior
> at run-time by throwing an exception if the property is removed from a
> class while running PHP versions 9.0+.

This is false. The `#[\SensitiveParameter]` attribute changes the 
behavior of argument capturing whenever PHP creates a backtrace. This 
includes the backtrace added to Exception objects, the 
`debug_backtrace()` function and the fatal error backtraces added in PHP 
8.5 with https://wiki.php.net/rfc/error_backtraces_v2. This is also 
listed in the backwards compatible changes section of the 
`#[\SensitiveParameter]` RFC:

https://wiki.php.net/rfc/redact_parameters_in_back_traces#backward_incompatible_changes

> 2. Custom exception handlers might see objects of class 
> \SensitiveParameterValue, despite the parameter having a different type 
> within the method's signature.

You mentioned “by emitting errors”, but I don't think that this is 
applicable here, since `#[\SensitiveParameter]` does not emit an error 
by itself and because it also affects the `debug_backtrace()` function, 
which is sometimes used for meta-programming.

-

> I don't mean to be rude and use your own words against you, but I found
> this quote in the "Why an attribute and not a keyword?" section of
> your #[\Override] RFC:
> 
>> This RFC proposes an attribute instead of a keyword, because contrary 
>> to
>> other modifiers (e.g. visibility) that are part of the method 
>> signature,
>> the attribute does not affect behavior or compatibility for users that
>> further extend a given class and neither does it affect users that 
>> call the
>> method. It is purely an assistance to the author of a given class.
> 
>  (and I apologize if I misunderstand your intention or take it out of
> context completely), but
> I like the sentiment that "the attribute does not affect behavior or
> compatibility for users"
> and that it's part of the argument for an attribute instead of a 
> keyword
> (language construct).
> 
> I feel like #[NoDiscard] is getting dangerously close to being part of 
> a
> method signature,
> especially if it throws exceptions at run-time (which I know it doesn't 
> do
> in your RFC, so I'm less against it).

The situation with `#[\NoDiscard]` is indeed different to 
`#[\Override]`, because it affects the *user* of the API rather than the 
*creator* of it. But as you said, it doesn't throw an Exception by 
itself, but emits a warning which like other warnings only affects 
behavior if you specifically decide to do so via a custom error handler. 
And even if it would throw an Exception, having the Attribute would be 
“more visible” than an Exception that the function itself throws, since 
PHP does not implement “checked Exceptions” as in Java and unless the 
author adds a `/** @throws SomeException */` PHPDoc it's not clear what 
Exceptions might be thrown (or if any Exceptions are thrown at all).

Given that other languages also implement it as Attribute + Warning, I'd 
say we're in good company here. In case of C, using the `-Werror' flag 
for your compiler would then be equivalent to a throwing error handler 
:-)

Best regards
Tim Düsterhus