On Fri, May 22, 2020 at 12:56 PM Rowan Tommins <rowan.coll...@gmail.com>
wrote:
> Personally, I'm not a fan of promoting messages to exceptions in this
> way, because APIs designed to throw exceptions generally look rather
> different from ones designed to warn and continue, so blindly converting
> seems like putting a square peg in a round hole. However, I know people
> do it already, so will give my thoughts on the principle.
>
To this (and also to George's point) - I definitely agree that this isn't a
perfect solution. I see this as more of a "stopgap" for people who prefer
exception-style handling rather than having to do both.
> My main concern is that it needs a tighter definition of "error", and
> possibly a configurable one.
>
> The snippet from the manual which you include in your draft RFC
> references the current global error_reporting setting. This makes sense
> with a global error handler, but less so for a per-file declare - it
> means the caller can choose whether exceptions are thrown, somewhat
> defeating the point. Perhaps the declare directive could take an error
> level as its argument, e.g. declare(error_exceptions=E_ALL-E_NOTICE)?
>
I hadn't thought putting the error types in the declare(). My only issue
with that is how that would interact with the error_reporting setting - for
example if I set my error_reporting to E_WARNING and error_exceptions to
E_ALL I'm not sure whether a notice would be thrown.
As far as a tighter definition of error - I've kept it loose in the draft
as I'm not sure how exactly to define errors which can and can't be
processed at runtime. For example, a compile error cannot be caught at
runtime, nor can (at least in my patch) warnings which are issued before
current_execute_data is populated (that is the object which I read to check
if the flag is set).
> Relatedly, you would need to define how this interacts with the @ (error
> suppression or "shut up") operator - would that force the code to run
> past a point that would otherwise throw an exception? Again, I think
> that choice should be taken away from the caller, otherwise the author
> needs to account for both modes, e.g.
>
> declare(error_exceptions=E_WARNING);
> try {
> $fh = fopen($blah, $mode);
> // if caller can suppress ErrorExceptions, we still need to check
> if $fh is false
> // ...
> } catch ( ErrorException $e ) {
> // the caller shouldn't actually care what happens, because we can
> catch it here
> }
>
The error suppression operator should behave as you've described it. In a
file with declare(), no warnings will ever be issued, meaning that a
suppression operator on a function defined with the flag (or, for that
matter, within such a file itself) will never have an effect. I will
clarify this in the RFC as well as add a test for that case.
On Fri, May 22, 2020 at 1:22 PM G. P. B. <george.bany...@gmail.com> wrote:
> I did a similar PoC last summer (see:
> https://github.com/php/php-src/pull/4549)
> where Nikita commented that a lot of those warnings could/should be
> promoted
> to error regardless, and this is what myself and a couple of other people
> have
> been during for the past year.
>
>From what I can see your approach is slightly different than mine - this
does not manually change any function's error behaviour but removes the
warning system entirely. As we saw in
https://wiki.php.net/rfc/engine_warnings - there is still (and may always
be) significant pushback on moving errors to exceptions. As mentioned
above, I see this as a "stopgap" for authors who do not want to deal with
the error system in any way.
