Hi
Am 2025-02-12 15:07, schrieb Kamil Tekiela:
I'd be honest, I have a very negative attitude towards this proposal
and I'd be voting against it. It seems to me like it's creating a
problem and then trying to find a solution for it.
Given that even Rust with a modern stdlib and ergonomic language
features has such an attribute, we do not believe it's an invented
problem. With the `bulk_process()` example, we also tried to provide
coherent user-story with a use-case that the readers might've already
encountered themselves in the same or a similar fashion. Personally I
encountered situations where I wished such an attribute would've warned
me both in PHP and JavaScript/TypeScript. Also in C of course, but given
that C doesn't have Exceptions, the situation is somewhat different.
For an additional example use-case, consider a function returning a
resource object with side-effects which will automatically clean up the
resource when being destructed. Specifically a function to spawn a new
process or thread:
class Process {
public function __destruct() { /* kill the process */ }
}
#[NoDiscard("the process will be terminated when the Process object
goes out of scope")]
function start_process(string $executable): Process {
return new Process($executable);
}
start_process('/my/background/process');
Depending on timing, the process might or might not run to completion
before PHP gets around to kill it, for example when a step debugger is
attached to PHP, making the bug disappear when debugging.
Related to the above example and also to the `flock()` function: A
developer might want to write a locking function that returns a lock
resource that will automatically unlock when it goes out of scope,
preventing the unlock from being forgotten. It might not be immediately
obvious that the locking function returns a lock resource, especially
when it's associated with another object:
$cacheDriver->lock('someCacheEntry');
Is there a `$cacheDriver->unlock('someCacheEntry')` or will `->lock()`
return a lock resource? Having the attribute on the `lock()` method can
help.
Searching for `language:rust "must_use ="` on GitHub reveals that a
common use-case in Rust is lazy iterators, where you can call
higher-order functions (e.g. map() or filter()), but they will not do
anything unless you explicitly “poll” them at least once. As far as I
know, Java has lazy streams that behave identically.
A return value is always supposed to be used. If some API is returning
a value that can be safely ignored, it's a badly designed API. If a
We agree that in a greenfield ecosystem, we would make ignoring the
return value of a function a warning by default (with an opt-out
mechanism), as done in Swift. However in PHP we do not have a greenfield
situation, there a number of functions where the return value is useless
and only exists for historical reasons. This includes all functions
which nowadays have a return-type of just `true`, which was only added
for this singular purpose.
There's cases where ignoring the return value is safe and reasonable,
without being a badly designed API. `array_pop()` would come to mind: If
one is only interested in the side-effect of removing the last element
in the array, one does not need the return value without necessarily
doing something unsafe.
doesn't feel like the solution. In fact, it's creating a problem for
users who want to ignore the value, which you then propose to solve
with (void) cast.
Ignoring the return value of functions having the attribute should be a
rare occurrence given the intended use-case of pointing out unsafe
situations. But as with static analyzers there needs to solution to
suppress warnings, after carefully verifying that the warning is not
applicable in a given situation.
the compilation phase. In PHP warnings are runtime errors. The code
should emit an exception instead of a warning. It would also make it
See Volker's reply to Derick.
much easier to handle and you wouldn't need any special construct to
allow users to ignore the new attribute. And I am really not a fan of
Even if an Exception would be thrown, there would be a mechanism to
suppress the Exception. A catch-block wouldn't work, because if the
Exception is thrown, the function is not executed / control flow is
interrupted.
the PHP engine generating E_USER_WARNING which should be reserved only
for warnings triggered by trigger_error.
This follows the lead of the `#[\Deprecated]` attribute, which emits
`E_USER_DEPRECATED` for userland functions and `E_DEPRECATED` for native
functions, despite being triggered by the engine.
The examples you used don't support the need for the new attribute.
Regarding the DateTimeImmutable methods, you said yourself: "The
methods are badly named, because they do not actually set the updated
value". So your proposal suggests adding a brand new thing to PHP to
deal with bad method names?
`DateTimeImmutable` is not part of the “Use Cases” section: Our intended
use-case is the kind of `bulk_process()` functionality that is used for
all the code snippets. But given that the attribute is also useful for
`DateTimeImmutable`, we made it part of the proposal, without it being
part of the intended “user-story”.
This problem should be solved using static analysers, IDE, and proper
code refactoring.
See Volker's reply to Matteo.
Best regards
Tim Düsterhus
