[VOTE] Reclassifying engine warnings

4 years ago by Olumide Samson — view source

unread

Thanks to those who can vote, all in all I hope for a better language where
we can proudly post jobs(even intern) for on our company's website without
been looked down on as inferior.

Since I can't vote, I can only hope for the best.

<3

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

4 years ago by Chase Peeler — view source

unread

Thanks to those who can vote, all in all I hope for a better language where
we can proudly post jobs(even intern) for on our company's website without
been looked down on as inferior.

If you're so embarrassed by the language, then why not use something else,
instead of trying to force such massive changes on the entire user base?

Also, if you really think this is going to change how non-PHP developers
view PHP, you're sorely mistaken. The few I've talked to about this, in
order to gauge how the languages they work in handle BC breaks, were
appalled that such a major breakage would be forced on users, even though
they personally don't like the fact that PHP supports uninitialized
variables.

Again, I implore everyone to stop trying and make everyone else like us.
Our language is awesome because of the fact that it is different and
flexible. That flexibility allows you to be strict if you want. If we just
want to turn PHP into another language that is like everything else out
there, then what's the point of even using PHP to begin with?

Since I can't vote, I can only hope for the best.

<3

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

--
Chase Peeler
chasepeeler@gmail.com

4 years ago by Olumide Samson — view source

unread

On Thu, Sep 12, 2019 at 8:33 AM Olumide Samson oludonsexy@gmail.com
wrote:

Thanks to those who can vote, all in all I hope for a better language
where
we can proudly post jobs(even intern) for on our company's website without
been looked down on as inferior.

If you're so embarrassed by the language, then why not use something
else, instead of trying to force such massive changes on the entire user
base?

Anything considered through vote is opinion based, not the way you could
call "force" and I think you can also ask those who are not embarrassed by
the current language situation to stick to whatever is their current
version. Upgrading can't be for everyone.
Only those who like major features/changes of a new version upgrade.

Also, if you really think this is going to change how non-PHP developers
view PHP, you're sorely mistaken. The few I've talked to about this, in
order to gauge how the languages they work in handle BC breaks, were
appalled that such a major breakage would be forced on users, even though
they personally don't like the fact that PHP supports uninitialized
variables.

I'm sure you are the one mistaken here, cos you don't speak for non-PHP
developers likewise I don't.

You can in all utmost speak for yourself like I did In my last email.

Again, I implore everyone to stop trying and make everyone else like us.
Our language is awesome because of the fact that it is different and
flexible. That flexibility allows you to be strict if you want. If we just
want to turn PHP into another language that is like everything else out
there, then what's the point of even using PHP to begin with?

I'm sure you are insulting the Project through those written words. If the
project still want to be what it has always been, then there's no want or
need for anything called @internals cos the language can actually still
stick to PHP 3 and become what it has always been without any updates or
changes.

Like I said, since I can't vote I can only hope for the best.

Since I can't vote, I can only hope for the best.

<3

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

--
Chase Peeler
chasepeeler@gmail.com

4 years ago by Chase Peeler — view source

unread

On Thu, Sep 12, 2019 at 10:13 AM Olumide Samson oludonsexy@gmail.com
wrote:

On Thu, Sep 12, 2019 at 8:33 AM Olumide Samson oludonsexy@gmail.com
wrote:

Thanks to those who can vote, all in all I hope for a better language
where
we can proudly post jobs(even intern) for on our company's website
without
been looked down on as inferior.

If you're so embarrassed by the language, then why not use something
else, instead of trying to force such massive changes on the entire user
base?

Anything considered through vote is opinion based, not the way you could
call "force" and I think you can also ask those who are not embarrassed by
the current language situation to stick to whatever is their current
version. Upgrading can't be for everyone.
Only those who like major features/changes of a new version upgrade.

I do like new features. There are tons of new features that can be added
without such a massive and unnecessary BC break.

Also, if you really think this is going to change how non-PHP developers
view PHP, you're sorely mistaken. The few I've talked to about this, in
order to gauge how the languages they work in handle BC breaks, were
appalled that such a major breakage would be forced on users, even though
they personally don't like the fact that PHP supports uninitialized
variables.

I'm sure you are the one mistaken here, cos you don't speak for non-PHP
developers likewise I don't.

You can in all utmost speak for yourself like I did In my last email.

I'm not claiming to speak for everyone. I did provide some anecdotal
evidence though.

Again, I implore everyone to stop trying and make everyone else like us.

Our language is awesome because of the fact that it is different and
flexible. That flexibility allows you to be strict if you want. If we just
want to turn PHP into another language that is like everything else out
there, then what's the point of even using PHP to begin with?

I'm sure you are insulting the Project through those written words. If the
project still want to be what it has always been, then there's no want or
need for anything called @internals cos the language can actually still
stick to PHP 3 and become what it has always been without any updates or
changes.

Not an insult at all. PHP has made a lot of awesome improvements, and
continues to make more. We can add things like union types, enums, etc.,
without losing the flexibility that makes the language great. This RFC
takes away something that, in the opinion of many, makes PHP better than
the other options out there.

Like I said, since I can't vote I can only hope for the best.

Since I can't vote, I can only hope for the best.

<3

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for
specific
cases that were controversial during the discussion, the last one is
for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

--
Chase Peeler
chasepeeler@gmail.com

--
Chase Peeler
chasepeeler@gmail.com

4 years ago by Nikita Popov — view source

unread

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

I just realized that I missed one notice here, because it is generated from
a different location: "Constant %s already defined" (The define/const will
be ignored and the previous value used.)

It would be great to have that as an exception for optimization reasons,
but as it's only a notice right now ... what do people think about
promoting it to a warning?

Nikita

4 years ago by guilhermeblanco@gmail.com — view source

unread

Nikita,

I'd suggest to wait until the current vote ends and then open a new
RFC to vote this one, otherwise it'll be disrupting.

Cheers,

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

I just realized that I missed one notice here, because it is generated from
a different location: "Constant %s already defined" (The define/const will
be ignored and the previous value used.)

It would be great to have that as an exception for optimization reasons,
but as it's only a notice right now ... what do people think about
promoting it to a warning?

Nikita

--
Guilherme Blanco
SVP Technology at Statflo Inc.
Mobile: +1 647 232 5599

4 years ago by Nikita Popov — view source

unread

On Wed, Sep 18, 2019 at 6:32 PM guilhermeblanco@gmail.com <
guilhermeblanco@gmail.com> wrote:

Nikita,

I'd suggest to wait until the current vote ends and then open a new
RFC to vote this one, otherwise it'll be disrupting.

Err, to be clear, I'm not suggesting to change the RFC under vote. We have
enough drama without that ;)

I certainly don't care about this enough to run it through a separate RFC
vote either. I'd like a simple consensus decision on a minor point. If
Chase & co tell me that "This is no problem for us, go ahead" or "This is
going to break all our code, leave it alone" that's good enough for me.

Context is that the current notice prevents constant propagation
optimizations. A warning won't help us either, but it would be the first
step towards making it an exception in the future, which would enable
those optimizations.

Regards,
Nikita

On Wed, Sep 18, 2019 at 12:29 PM Nikita Popov nikita.ppv@gmail.com
wrote:

On Thu, Sep 12, 2019 at 2:17 PM Nikita Popov nikita.ppv@gmail.com
wrote:

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for
specific
cases that were controversial during the discussion, the last one is
for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

I just realized that I missed one notice here, because it is generated
from
a different location: "Constant %s already defined" (The define/const
will
be ignored and the previous value used.)

It would be great to have that as an exception for optimization reasons,
but as it's only a notice right now ... what do people think about
promoting it to a warning?

Nikita

--
Guilherme Blanco
SVP Technology at Statflo Inc.
Mobile: +1 647 232 5599

4 years ago by Chase Peeler — view source

unread

On Wed, Sep 18, 2019 at 6:32 PM guilhermeblanco@gmail.com <
guilhermeblanco@gmail.com> wrote:

Nikita,

I'd suggest to wait until the current vote ends and then open a new
RFC to vote this one, otherwise it'll be disrupting.

Err, to be clear, I'm not suggesting to change the RFC under vote. We have
enough drama without that ;)

I certainly don't care about this enough to run it through a separate RFC
vote either. I'd like a simple consensus decision on a minor point. If
Chase & co tell me that "This is no problem for us, go ahead" or "This is
going to break all our code, leave it alone" that's good enough for me.

I don't know if I should be flattered or concerned that I was called out
by name :-) I also want to reiterate, for the record, most of my arguments
in regards to breaking BC isn't "This will be hard for me" as much as it
its "This will be hard for a lot of other users" - I would never ask for
anything to be done based on a use-case that only applies to me.

That being said, I don't really know what the impact to the rest of
userland would be. Since you can't actually redefine constants, I would
argue that defining a previously defined constant IS probably an error
situation. We actually have a "local_constants.php" that's included at the
top of "constants.php" file. We can then define things in the
local_constants.php, "overriding" the value in the constants.php file. All
of the defines in the constants.php are preceded with @ though. Not sure
if @ will suppress exceptions, but if not, that can always be put in
try/catch (it'll just be REALLY ugly :-)). Also, we're trying to migrate
away from the constants to using YAML configuration files, which handle
per-environment overriding much better, but I digress.

I think that's a specific exception though, where we know that we might be
trying to define an already defined constant, and don't want it redefined.
In most cases, if you are defining a constant, it's because you want it
defined to that value, and are not aware it already was defined. I find
that a totally different situation than using $i++ without having done $i=0
first.

Context is that the current notice prevents constant propagation
optimizations. A warning won't help us either, but it would be the first
step towards making it an exception in the future, which would enable
those optimizations.

So, this is a case where there is an advantage to the change beyond "We
want things to be more strict" - As many of us have said, we aren't against
BC breaks. We just feel they should provide something that makes the
language functionally better at a level that justifies the break.

Regards,
Nikita

On Wed, Sep 18, 2019 at 12:29 PM Nikita Popov nikita.ppv@gmail.com
wrote:

On Thu, Sep 12, 2019 at 2:17 PM Nikita Popov nikita.ppv@gmail.com
wrote:

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for
specific
cases that were controversial during the discussion, the last one is
for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

I just realized that I missed one notice here, because it is generated
from
a different location: "Constant %s already defined" (The define/const
will
be ignored and the previous value used.)

It would be great to have that as an exception for optimization
reasons,
but as it's only a notice right now ... what do people think about
promoting it to a warning?

Nikita

--
Guilherme Blanco
SVP Technology at Statflo Inc.
Mobile: +1 647 232 5599

--
Chase Peeler
chasepeeler@gmail.com

4 years ago by Claude Pache — view source

unread

Le 18 sept. 2019 à 18:28, Nikita Popov nikita.ppv@gmail.com a écrit :

I just realized that I missed one notice here, because it is generated from
a different location: "Constant %s already defined" (The define/const will
be ignored and the previous value used.)

It would be great to have that as an exception for optimization reasons,
but as it's only a notice right now ... what do people think about
promoting it to a warning?

Nikita

Since attempting to define an already defined constant fails to perform the expected operation (i.e., defining the constant to that value), I think that a Warning is very reasonable.

—Claude

4 years ago by bjorn.x.larsson@telia.com — view source

unread

Den 2019-09-18 kl. 21:44, skrev Claude Pache:

Le 18 sept. 2019 à 18:28, Nikita Popov nikita.ppv@gmail.com a écrit :

I just realized that I missed one notice here, because it is generated from
a different location: "Constant %s already defined" (The define/const will
be ignored and the previous value used.)

It would be great to have that as an exception for optimization reasons,
but as it's only a notice right now ... what do people think about
promoting it to a warning?

Nikita
Since attempting to define an already defined constant fails to perform the expected operation (i.e., defining the constant to that value), I think that a Warning is very reasonable.

—Claude

I also think that a warning would be appropriate here.
Have stumbled on it myself, not having notices on.

r//Björn L

4 years ago by Nikita Popov — view source

unread

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Voting has closed with the final outcome being:

Undefined variables: 36 exception, 18 warning, 10 notice. Exception
declined with 56% in favor. Warning accepted with 84% combined majority.
Undefined array index: 42 warning, 21 notice. Warning accepted with 2/3
majority.
Division by zero: 52 exception, 8 warning. Exception accepted with 87%
majority.
Remainder: 54 yes, 3 no. Accepted with 95% majority.

Regards,
Nikita

4 years ago by Christian Schneider — view source

unread

Am 26.09.2019 um 09:41 schrieb Nikita Popov nikita.ppv@gmail.com:

Remainder: 54 yes, 3 no. Accepted with 95% majority.

Just for the record:
The one I'm most concerned about here is the foreach on undefined variables throwing an exception.
While this was already promoted to a warning at an earlier stage it still allowed a program to finish with a well-defined result (looping over nothing does nothing).
Now this code will stop with an Exception and possibly won't produce its output.

I noticed this before the vote but as the mailing list was already flooded with the 'undefined variables' discussion I feared that there was no place to bring this up without getting personally attacked like I was for the undefined variables stuff.

Anyway, the vote is done, I said my piece and will shut up about it now,

Chris

4 years ago by Chase Peeler — view source

unread

On Thu, Sep 26, 2019 at 4:31 AM Christian Schneider cschneid@cschneid.com
wrote:

Am 26.09.2019 um 09:41 schrieb Nikita Popov nikita.ppv@gmail.com:

Remainder: 54 yes, 3 no. Accepted with 95% majority.

Just for the record:
The one I'm most concerned about here is the foreach on undefined
variables throwing an exception.
While this was already promoted to a warning at an earlier stage it still
allowed a program to finish with a well-defined result (looping over
nothing does nothing).
Now this code will stop with an Exception and possibly won't produce its
output.

I agree. At the very least, I'd propose that null (and ideally false) NOT
throw an exception. There are enough instances of methods that return one
of those values when there is no data (so, not an error situation) but an
array when there is.

I noticed this before the vote but as the mailing list was already flooded
with the 'undefined variables' discussion I feared that there was no place
to bring this up without getting personally attacked like I was for the
undefined variables stuff.

Thanks for bringing this up. I think this should be noted in relation to
the other RFC currently out there. Those of us speaking out against this
RFC are the ones being held up as examples of why that RFC is needed, which
this statement actually shows that it was those in favor of the RFC that
were behaving in a way that intimidated others and made them feel reluctant
to contribute.

Anyway, the vote is done, I said my piece and will shut up about it now,

Chris

--

--
Chase Peeler
chasepeeler@gmail.com

4 years ago by Peter Cowburn — view source

unread

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Voting has closed with the final outcome being:

Undefined variables: 36 exception, 18 warning, 10 notice. Exception
declined with 56% in favor. Warning accepted with 84% combined majority.

I just want to go on the record in saying that I am very, very disappointed
that a choice that only got 28% of the overall votes, and only 33% of votes
in the "we want change" scenario, is being taken as the will of the
overwhelming majority, which is the bar that is needed to be crossed for
RFC votes. This is wholly irresponsible.

Undefined array index: 42 warning, 21 notice. Warning accepted with 2/3
majority.

Division by zero: 52 exception, 8 warning. Exception accepted with 87%
majority.

Remainder: 54 yes, 3 no. Accepted with 95% majority.

Regards,
Nikita

4 years ago by Rowan Tommins — view source

unread

I just want to go on the record in saying that I am very, very disappointed
that a choice that only got 28% of the overall votes, and only 33% of votes
in the "we want change" scenario, is being taken as the will of the
overwhelming majority, which is the bar that is needed to be crossed for
RFC votes. This is wholly irresponsible.

Three-way votes are always tricky in this respect, but I think in this case
Nikita has taken a very sensible approach.

Firstly, the interpretation of the three-way vote was laid out very clearly
on the page, and I'm not aware of anyone objecting to it prior to this
point.

Secondly, it makes sense intuitively: it seems unlikely that someone who
would vote yes to the question "Should undefined variables give an Error
instead of a Notice?" would vote no to the question "Should undefined
variables give a Warning instead of a Notice?"

Thirdly, the options are not mutually exclusive in the way that, say, a
syntax decision would be. Raising the level to Warning now doesn't prevent
a future proposal to raise it to Error (e.g. on a different timescale).

Finally, and perhaps most importantly, RFC votes are intended to be
measures of consensus. Taken alongside the discussion, the result strongly
suggests that there is a consensus (but not a unanimous one) to change the
error level, but there is some concern about raising it as high as Error.

Regards,

Rowan Tommins
[IMSoP]

4 years ago by Nikita Popov — view source

unread

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Voting has closed with the final outcome being:

Undefined variables: 36 exception, 18 warning, 10 notice. Exception
declined with 56% in favor. Warning accepted with 84% combined majority.

Undefined array index: 42 warning, 21 notice. Warning accepted with 2/3
majority.

Division by zero: 52 exception, 8 warning. Exception accepted with 87%
majority.

Remainder: 54 yes, 3 no. Accepted with 95% majority.

Regards,
Nikita

This RFC is now mostly implemented. The parts that are still missing are:

Switching to DivisionByZeroError. I would like to add an fdiv() function
before doing this and have started a separate thread on the topic.
The "Invalid argument supplied for foreach()" case. Christian Schneider
has requested that this stay as a warning, and personally I'm okay with
doing that. Unlike most of the other exception promotions, this one won't
result in any VM simplifications/optimizations and I don't have a very
strong reason to make it an exception. Does anyone else feel strongly about
this?
The "Undefined array index" case. This one passed the vote with an exact
2/3 majority, so I'm a bit uncomfortable making changes here. This is also
the only case where convincing usability concerns have been brought
forward, primarily around the $array[$key]++ example. I'm not yet decided
on what to do here and whether we should pursue additional library or
language features first.

Nikita

4 years ago by Sara Golemon — view source

unread

The "Undefined array index" case. This one passed the vote with an exact
2/3 majority, so I'm a bit uncomfortable making changes here. This is also
the only case where convincing usability concerns have been brought
forward, primarily around the $array[$key]++ example. I'm not yet decided
on what to do here and whether we should pursue additional library or
language features first.

We raised margins to 2/3 to avoid having to feel bad about narrow
majorities. I say this as someone who was quite tempted to vote "Keep
Notice" which on retrospect would have changed the entire outcome based on
that one vote.

If this had been a simple majority vote and it had pass by like....
52%-48%, then obviously that's not a mandate worth making potentially
catastrophic changes over, but this is 67% to 33%, it feels like you're
okay here.

-Sara

4 years ago by Bishop Bettini — view source

unread

The "Undefined array index" case. This one passed the vote with an
exact
2/3 majority, so I'm a bit uncomfortable making changes here. This is
also
the only case where convincing usability concerns have been brought
forward, primarily around the $array[$key]++ example. I'm not yet decided
on what to do here and whether we should pursue additional library or
language features first.

We raised margins to 2/3 to avoid having to feel bad about narrow
majorities. I say this as someone who was quite tempted to vote "Keep
Notice" which on retrospect would have changed the entire outcome based on
that one vote.

If this had been a simple majority vote and it had pass by like....
52%-48%, then obviously that's not a mandate worth making potentially
catastrophic changes over, but this is 67% to 33%, it feels like you're
okay here.

I'd say it's ok, also: with 63 votes cast, I think we have a quorum and
therefore representative opinion. (Per 1, this vote is second only in
participation to typed properties 2.0, with 71 votes cast.)

I personally abstained, seeing both sides of the argument.

4 years ago by Zeev Suraski — view source

unread

The "Undefined array index" case. This one passed the vote with an exact
2/3 majority, so I'm a bit uncomfortable making changes here.

I think that making this change on the edge of a single vote is less than
ideal... Even if I’m known for my pro-BC bias ?

If you’re looking for an excuse for why we might consider not going through
with it even though it did clear the needed majority (if barely) - the vote
ended roughly 5 hours before two weeks passed from the point the vote was
announced. It’s not outside the realm of possibility that given the zero
margin - the vote might have ended differently if these 5hrs were
available... But I’d say it’s up to you.

Zeev

4 years ago by Nikita Popov — view source

unread

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

As people have expressed interest in hearing about direct technical
benefits that these kinds of changes have ... let me give you an example
that came up yesterday.

Opcache performs a bunch of optimizations, and one class of optimizations
it does are subsequent jumps on the same operand. For example:

if ($x) { A; }
if ($x) { B; }

Currently, opcache will optimize the first if($x) condition to jump
directly until after the second if($x) if the value is false, on the
expectation that it is redundant to check the same condition twice in a
row: The result is going to be the same. Basically the result is something
like this:

if ($x) { A; } else { goto end; }
if ($x) { B; }
end:

Now, it turns out that this entire class of optimizations is technically
illegal. Why? Because $x might be an undefined variable! That means that
this optimization at the least loses an "undefined variable" notice, and at
worse changes control flow:

set_error_handler(function() {
$GLOBALS['x'] = true;
});
if ($x) echo "foo\n";
if ($x) echo "bar\n";

Because it's been around for years and doesn't seem to have caused any
active issues, we're likely going to keep this, but nonetheless, it
illustrates the kind of issue we see with these notices. Either an
exception or nothing at all are fine, but notices caused problems.

Of course there are also other problems, such as
https://bugs.php.net/bug.php?id=78598, which is one of multiple
use-after-free issues related to notices thrown during write operations.
The root cause is that under the PHP memory model, it is not legal to run
arbitrary user code while holding writable references into structures -- an
invariant that is violated by some notices, such as the undefined array key
one, because those notices may invoke error handlers. Again, either
throwing nothing or throwing an exception would be unproblematic.

Generally notices thrown by the engine are a pretty big pain to deal with,
as well as something of a correctness and safety hazard. We have quite a
few bugs in this area, though most of them are thankfully not likely to be
hit by accident.

Nikita

4 years ago by Benjamin Morel — view source

unread

As people have expressed interest in hearing about direct technical
benefits that these kinds of changes have ... let me give you an example
that came up yesterday.

Too bad this example comes after the vote has been made, and failed.
This would be a very strong argument in favour of using exceptions
everywhere in the next major version: codebase cleanup, room for more
optimization.

Nikita, please fork PHP, we'll follow you ;-)

— Benjamin

4 years ago by Olumide Samson — view source

unread

On Fri, Oct 11, 2019, 9:29 AM Benjamin Morel benjamin.morel@gmail.com
wrote:

As people have expressed interest in hearing about direct technical
benefits that these kinds of changes have ... let me give you an example
that came up yesterday.

Too bad this example comes after the vote has been made, and failed.
This would be a very strong argument in favour of using exceptions
everywhere in the next major version: codebase cleanup, room for more
optimization.

Nikita, please fork PHP, we'll follow you ;-)

— Benjamin

I think I'm always available to contribute to a fork of a better PHP, coz I
love the syntax not the garbages included in the current one.

4 years ago by Claude Pache — view source

unread

Le 11 oct. 2019 à 11:12, Olumide Samson oludonsexy@gmail.com a écrit :

On Fri, Oct 11, 2019, 9:29 AM Benjamin Morel benjamin.morel@gmail.com
wrote:

As people have expressed interest in hearing about direct technical
benefits that these kinds of changes have ... let me give you an example
that came up yesterday.

Too bad this example comes after the vote has been made, and failed.
This would be a very strong argument in favour of using exceptions
everywhere in the next major version: codebase cleanup, room for more
optimization.

Nikita, please fork PHP, we'll follow you ;-)

— Benjamin

I think I'm always available to contribute to a fork of a better PHP, coz I
love the syntax not the garbages included in the current one.

If you’re seeking a fork of PHP that wilfully breaks BC for the sake of cleanup and optimisation, you should seriously consider Hack. Although I don’t know whether they’ve already removed support of the appalling implicit initialisation of variables to null, or of the dreadful backtick operator, you’ll be delighted to learn that they’re on the process of removing references, PHP arrays (in favour of Hack arrays and collections), and even that little pesky <?php tag that plagues the first line of every PHP file. ;-)

https://hhvm.com/blog/2019/02/11/hhvm-4.0.0.html https://hhvm.com/blog/2019/02/11/hhvm-4.0.0.html

Enjoy. (But not with me: our company does not have the budget to migrate 30 Mo of code without counting external libraries.)

—Claude

4 years ago by Gert — view source

unread

I may be a bit late to this party, but will the promotion to error
exception of illegal offsets also happen for using resources as array
keys? These currently produce a notice, and are then casted to an
integer: https://3v4l.org/cQ8hf
Or will the behaviour of that remain the same?

Le 11 oct. 2019 à 11:12, Olumide Samson oludonsexy@gmail.com a écrit :

On Fri, Oct 11, 2019, 9:29 AM Benjamin Morel benjamin.morel@gmail.com
wrote:

As people have expressed interest in hearing about direct technical
benefits that these kinds of changes have ... let me give you an example
that came up yesterday.

Too bad this example comes after the vote has been made, and failed.
This would be a very strong argument in favour of using exceptions
everywhere in the next major version: codebase cleanup, room for more
optimization.

Nikita, please fork PHP, we'll follow you ;-)

— Benjamin

I think I'm always available to contribute to a fork of a better PHP, coz I
love the syntax not the garbages included in the current one.

If you’re seeking a fork of PHP that wilfully breaks BC for the sake of cleanup and optimisation, you should seriously consider Hack. Although I don’t know whether they’ve already removed support of the appalling implicit initialisation of variables to null, or of the dreadful backtick operator, you’ll be delighted to learn that they’re on the process of removing references, PHP arrays (in favour of Hack arrays and collections), and even that little pesky <?php tag that plagues the first line of every PHP file. ;-)

https://hhvm.com/blog/2019/02/11/hhvm-4.0.0.html https://hhvm.com/blog/2019/02/11/hhvm-4.0.0.html

Enjoy. (But not with me: our company does not have the budget to migrate 30 Mo of code without counting external libraries.)

—Claude

4 years ago by Christoph M. Becker — view source

unread

I may be a bit late to this party, but will the promotion to error
exception of illegal offsets also happen for using resources as array
keys? These currently produce a notice, and are then casted to an
integer: https://3v4l.org/cQ8hf
Or will the behaviour of that remain the same?

Only objects and arrays will throw; resources are promoted from E_NOTICE
to E_WARNING.

--
Christoph M. Becker

4 years ago by Andreas Hennings — view source

unread

Hi internals,

I've opened the vote on //wiki.php.net/rfc/engine_warnings.

There are 4 votes, all of them independent. The first 3 are for specific
cases that were controversial during the discussion, the last one is for
the remainder of the proposal.

Voting closes 2019-09-26.

Regards,
Nikita

As people have expressed interest in hearing about direct technical
benefits that these kinds of changes have ... let me give you an example
that came up yesterday.

Opcache performs a bunch of optimizations, and one class of optimizations
it does are subsequent jumps on the same operand. For example:

if ($x) { A; }
if ($x) { B; }

Currently, opcache will optimize the first if($x) condition to jump
directly until after the second if($x) if the value is false, on the
expectation that it is redundant to check the same condition twice in a
row: The result is going to be the same. Basically the result is something
like this:

if ($x) { A; } else { goto end; }
if ($x) { B; }
end:

Now, it turns out that this entire class of optimizations is technically
illegal. Why? Because $x might be an undefined variable! That means that
this optimization at the least loses an "undefined variable" notice, and at
worse changes control flow:

set_error_handler(function() {
$GLOBALS['x'] = true;
});
if ($x) echo "foo\n";
if ($x) echo "bar\n";

To be fair, the same problem would still apply for other code that
emits notices in an if condition, right?
Or does the opcache only optimize this for simple variables?

The "correct" behavior would be to analyse the code before the if(),
and only optimize if we are sure that $x will always be defined..

Otherwise, we would need to convert it to if ($x) {..} elseif
(variable_exists($x)) {goto end;}
Sadly there is currently no variable_exists() in php, and the above
code would probably lose the optimization benefit with the extra
logic.

Because it's been around for years and doesn't seem to have caused any
active issues, we're likely going to keep this, but nonetheless, it
illustrates the kind of issue we see with these notices. Either an
exception or nothing at all are fine, but notices caused problems.

Of course there are also other problems, such as
https://bugs.php.net/bug.php?id=78598, which is one of multiple
use-after-free issues related to notices thrown during write operations.
The root cause is that under the PHP memory model, it is not legal to run
arbitrary user code while holding writable references into structures -- an
invariant that is violated by some notices, such as the undefined array key
one, because those notices may invoke error handlers. Again, either
throwing nothing or throwing an exception would be unproblematic.

Generally notices thrown by the engine are a pretty big pain to deal with,
as well as something of a correctness and safety hazard. We have quite a
few bugs in this area, though most of them are thankfully not likely to be
hit by accident.

Nikita