Everything

php

[VOTE] throw expression

php.internals

Ilija Tovilo

6 years ago

Hi internals,

I hope you're doing well.

I have opened voting for the throw expression RFC.
https://wiki.php.net/rfc/throw_expression

Voting closes on April 19th, 2020.

Regards,
Ilija

Dan Ackroyd

6 years ago

Hi Iija,

During the discussion of the precedence, you wrote:

> There is a reasonable way to recognize this, namely to check if the expression
> after the throw keyword is of type ZEND_AST_OR or ZEND_AST_AND. The
> expression above will fail with this message (given that $condition is false):

Did the RFC get updated to address this?

As the current behaviour listed* in the RFC seems wrong and surprising to me.

cheers
Dan
Ack


* relevant part:

Ilija Tovilo

6 years ago

Hi Dan

> Did the RFC get updated to address this?
>
> As the current behaviour listed* in the RFC seems wrong and surprising to me.

I did respond to that e-mail but probably got classified as spam as
were the other e-mails I sent with that address. This was my response:

> I've thought about this some more. There is a reasonable way to recognize this, namely to check if the expression after the throw keyword is of type ZEND_AST_OR or ZEND_AST_AND. The expression above will fail with this message (given that $condition is false):
>
> > Fatal error: Uncaught Error: Can only throw objects in ...
>
> IMO this is clear enough that a message for this edge case is not necessary.

We could improve the error message by printing the type that got
passed to throw:

> Fatal error: Uncaught Error: Can only throw objects, bool given in ...

Regards,
Ilija

Dan Ackroyd

6 years ago

Hi Ilija,

On Sun, 5 Apr 2020 at 11:53, Ilija Tovilo <tovilo.ilija@gmail.com> wrote:

>
>
> I did respond to that e-mail but probably got classified as spam as
> were the other e-mails I sent with that address. This was my response:
>
> > I've thought about this some more. There is a reasonable way to recognize this, namely to check if the expression after the throw keyword is of type ZEND_AST_OR or ZEND_AST_AND. The expression above will fail with this message (given that $condition is false):

fyi your emails are not marked as spam for me any more.

The issue is that as your words are written, it is not possible for me
to understand exactly what you mean. "There is a reasonable way to
recognize this" could mean:

* This has been done and is part of the implementation people are voting on.
* This could be done and you think it can be done trivially so doesn't
need to be part of the RFC.
* This could be done, but you're not sure how it could be done and
people are voting on it without being part of the implementation.

Please could you say whether this issue has been fixed or whether it
still needs fixing.

The RFC as currently stands hasn't been updated since it was initially
put to discussion. Which means that anyone just reading the RFC won't
be aware of this issue that was brought up during the discussion.

cheers
Dan
Ack

Ilija Tovilo

6 years ago

Sorry Dan, at the time I sent this e-mail I wasn't aware you wouldn't
receive it. After not hearing from you I assumed you were ok with my
response.

> The issue is that as your words are written, it is not possible for me
> to understand exactly what you mean.

To be fair, you omitted the next sentence which does answer your
question, admittedly not as concretely as it should have:

> IMO this is clear enough that a message for this edge case is not necessary.

So to answer your question more directly, no error message was added.

> The RFC as currently stands hasn't been updated since it was initially
> put to discussion. Which means that anyone just reading the RFC won't
> be aware of this issue that was brought up during the discussion.

Again, to be fair, yours was literally the only criticism I received.
And that example was taken from the RFC so it was already documented.

Either way, I'm not opposed to a warning. What are the rules here, can
I add this small change after voting has started?

Regards,
Ilija

Dan Ackroyd

6 years ago

On Sun, 5 Apr 2020 at 13:05, Ilija Tovilo <tovilo.ilija@gmail.com> wrote:

>
> "What are the rules here, can"

"... the rules are more what you'd call "guidelines" than actual rules."

As a project we should clarify exactly what is meant to happen when
issues are found in an RFC that affect how people are going to vote.

But for this case; there is plenty of time until the deadline for RFCs
that target PHP 8.

As it's a small but non-trivial change, I'd recommend closing the
vote, updating the RFC to either have the correct behaviour around
precedence, or if it's not possible to do that, disallow code that has
the ambiguous/surprising behaviour. Having what people are voting on
change, when votes are currently being cast is a thing that leads to
drama.

Once that issue is fixed, then wait two weeks and put the RFC to a
vote and it should sail through imo.

> yours was literally the only criticism I received.

In an attempt to set expectations; if a point is already being
addressed as part of the discussion, it's unlikely that other people
are going to "+1" that*. Extra email messages take up people's time
and don't add much value. Currently it's the responsibility of the RFC
author to make sure all points are addressed. Arguably that's
something that could be split off to be a role done by someone else.

cheers
Dan
Ack

* moving to a discussion platform that did allow indicating agreement
with an argument in a way that didn't take up people's time would be
nice...

Nikita Popov

6 years ago

On Sun, Apr 5, 2020 at 2:05 PM Ilija Tovilo <tovilo.ilija@gmail.com> wrote:

> Sorry Dan, at the time I sent this e-mail I wasn't aware you wouldn't
> receive it. After not hearing from you I assumed you were ok with my
> response.
>
> > The issue is that as your words are written, it is not possible for me
> > to understand exactly what you mean.
>
> To be fair, you omitted the next sentence which does answer your
> question, admittedly not as concretely as it should have:
>
> > IMO this is clear enough that a message for this edge case is not
> necessary.
>
> So to answer your question more directly, no error message was added.
>
> > The RFC as currently stands hasn't been updated since it was initially
> > put to discussion. Which means that anyone just reading the RFC won't
> > be aware of this issue that was brought up during the discussion.
>
> Again, to be fair, yours was literally the only criticism I received.
> And that example was taken from the RFC so it was already documented.
>
> Either way, I'm not opposed to a warning. What are the rules here, can
> I add this small change after voting has started?
>

FWIW I (strongly) don't think a warning should be added for this.

The only thing wrong here is calling this an "exception to the rule" in the
RFC. This is completely standard and expected behavior given the specified
precedence. It is in the nature of precedence that there will always be
cases where the use of () is required, and this is one of them.

Regards,
Nikita

Ilija Tovilo

6 years ago

Hi Nikita

> The only thing wrong here is calling this an "exception to the rule" in the RFC. This is completely standard and expected behavior given the specified precedence. It is in the nature of precedence that there will always be cases where the use of () is required, and this is one of them.

I can see how that statement could be confusing. What I meant was this:

> This isn't a problem because generally throw should be the last operator ...
> ...
> One exception to this rule ...

As in, this is the only example I could think of where throw would be
used between two other expressions. Not that the precedence behaves
differently than in any other case.

I'll rephrase that sentence.

Regards,
Ilija

Girgias

6 years ago

On Sun, 5 Apr 2020 at 12:03, Ilija Tovilo <tovilo.ilija@gmail.com> wrote:

> Hi internals,
>
> I hope you're doing well.
>
> I have opened voting for the throw expression RFC.
> https://wiki.php.net/rfc/throw_expression
>
> Voting closes on April 19th, 2020.
>
> Regards,
> Ilija
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Maybe I'm out of the loop but I haven't seen this before,
possibly due to the high volume on the list currently.

But I must say the last behaviour is pretty surprising.
What is the limitation leading to this behaviour? How the parser works?

Moreover, this specific case (and maybe some other exotic ones)
isn't covered by a test case provided in the implementation.

As suggested by Dan I think closing the vote for now and amending
the implementation. Is the best way forward as we still have a long
time before feature freeze and it makes it clearer for what we are voting.

Best regards

George P. Banyard

Ilija Tovilo

6 years ago

Hi George

> Maybe I'm out of the loop but I haven't seen this before

It's my fault, my e-mails went to the spam folder for many people. I'm
sending my e-mails from a different address now which seems to have
resolved the problem.

> What is the limitation leading to this behaviour?

It's to maintain backwards compatibility (chapter "Operator
precedence"). To make sure the examples in this chapter continue
working the precedence needs to be very low.

> Moreover, this specific case (and maybe some other exotic ones)
> isn't covered by a test case provided in the implementation.

Yes, I need to update the tests with all the examples in the RFC. I
just haven't done that yet.

Regards,
Ilija