Hi internals,
I would like to propose making the use of arithmetic/bitwise operators on
arrays, resources and (non-overloaded) objects a TypeError exception:
https://wiki.php.net/rfc/arithmetic_operator_type_checks
This is inspired by some of the recent discussions, in particular the
operator overloading RFC (where the fact that operations on non-overloaded
objects still succeed with just a notice was criticized) and the
increment/decrement RFC, which handles the array case of this proposal for
inc/dec only.
I think as-is, this RFC should be completely uncontroversial. However,
there is an open question on whether we want to make this slightly more
strict, in order to align the semantics with the rules for weak parameter
type checks for ints/floats.
If we do that, then this RFC could be a stepping stone towards making
"implicit" internal casts use the (weak) parameter type checking semantics
more generally, which I think would be a good idea. The current explicit
cast semantics we use everywhere are too forgiving for most circumstances
(e.g. an array is almost always not a reasonable input where an integer is
expected).
Regards,
Nikita
I would like to propose making the use of arithmetic/bitwise operators on
arrays, resources and (non-overloaded) objects a TypeError exception:https://wiki.php.net/rfc/arithmetic_operator_type_checks
This is inspired by some of the recent discussions, in particular the
operator overloading RFC (where the fact that operations on non-overloaded
objects still succeed with just a notice was criticized) and the
increment/decrement RFC, which handles the array case of this proposal for
inc/dec only.I think as-is, this RFC should be completely uncontroversial. However,
there is an open question on whether we want to make this slightly more
strict, in order to align the semantics with the rules for weak parameter
type checks for ints/floats.If we do that, then this RFC could be a stepping stone towards making
"implicit" internal casts use the (weak) parameter type checking semantics
more generally, which I think would be a good idea. The current explicit
cast semantics we use everywhere are too forgiving for most circumstances
(e.g. an array is almost always not a reasonable input where an integer is
expected).
Thanks! Generally I like that very much. I'm not quite sure, though,
what to do with resource to int conversions. These are documented to be
valid[1], and are sometimes deliberately used[2], and as such it might
be reasonable not to throw on these conversions (maybe even change the
current behavior of the ~ operator). After all, I hope we will be able
to port all resources to objects sometime, and till then we could stick
with more lax semantics.
[1] https://www.php.net/manual/en/language.types.integer.php
[2]
https://www.php.net/manual/en/function.ssh2-sftp.php#refsect1-function.ssh2-sftp-examples
--
Christoph M. Becker
On Thu, Apr 2, 2020 at 10:35 AM Christoph M. Becker cmbecker69@gmx.de
wrote:
I would like to propose making the use of arithmetic/bitwise operators on
arrays, resources and (non-overloaded) objects a TypeError exception:https://wiki.php.net/rfc/arithmetic_operator_type_checks
This is inspired by some of the recent discussions, in particular the
operator overloading RFC (where the fact that operations on
non-overloaded
objects still succeed with just a notice was criticized) and the
increment/decrement RFC, which handles the array case of this proposal
for
inc/dec only.I think as-is, this RFC should be completely uncontroversial. However,
there is an open question on whether we want to make this slightly more
strict, in order to align the semantics with the rules for weak parameter
type checks for ints/floats.If we do that, then this RFC could be a stepping stone towards making
"implicit" internal casts use the (weak) parameter type checking
semantics
more generally, which I think would be a good idea. The current explicit
cast semantics we use everywhere are too forgiving for most circumstances
(e.g. an array is almost always not a reasonable input where an integer
is
expected).Thanks! Generally I like that very much. I'm not quite sure, though,
what to do with resource to int conversions. These are documented to be
valid[1], and are sometimes deliberately used[2], and as such it might
be reasonable not to throw on these conversions (maybe even change the
current behavior of the ~ operator).
While resource to int conversions are indeed valid, they are not exactly
commonly used, and if they are used, I would expect an explicit (int) cast
to be involved. I very seriously doubt that someone writing "$x = $y / 2"
really intended this to mean "$x = resource_id($y) / 2" -- it is much more
likely that a variable go mixed up. In the unlikely case that that they
really did intend that, the option of writing "$resourceId = (int) $y; $x =
$resourceId / 2" remains, and is much clearer.
On that note, I do wonder whether we should introduce a function like
get_resource_id(), that makes it more explicit that an (int) cast is
supposed to fetch a resource ID. This is not a very common operation, and I
suspect that many PHP programmers may not be familiar with the semantics of
integer casts on resources. Using get_resource_id($file) would make the
intent clearer relative to (int) $file. This is similar to the recently
added get_mangled_object_vars() function, which essentially does the same
as (array) $object, but makes it obvious that you really are interested in
mangled properties.
After all, I hope we will be able
to port all resources to objects sometime, and till then we could stick
with more lax semantics.
I think that's really an argument to align the behavior of resources and
objects, as it means one less BC break when resources are converted to
objects. Historically that's something we did in minor versions, so it's
good to reduce the amount of semantics that change when such a conversion
is done.
Regards,
Nikita
On Thu, Apr 2, 2020 at 10:35 AM Christoph M. Becker cmbecker69@gmx.de
wrote:I would like to propose making the use of arithmetic/bitwise operators on
arrays, resources and (non-overloaded) objects a TypeError exception:Thanks! Generally I like that very much. I'm not quite sure, though,
what to do with resource to int conversions. These are documented to be
valid[1], and are sometimes deliberately used[2], and as such it might
be reasonable not to throw on these conversions (maybe even change the
current behavior of the ~ operator).While resource to int conversions are indeed valid, they are not exactly
commonly used, and if they are used, I would expect an explicit (int) cast
to be involved. I very seriously doubt that someone writing "$x = $y / 2"
really intended this to mean "$x = resource_id($y) / 2" -- it is much more
likely that a variable go mixed up. In the unlikely case that that they
really did intend that, the option of writing "$resourceId = (int) $y; $x =
$resourceId / 2" remains, and is much clearer.On that note, I do wonder whether we should introduce a function like
get_resource_id(), that makes it more explicit that an (int) cast is
supposed to fetch a resource ID. This is not a very common operation, and I
suspect that many PHP programmers may not be familiar with the semantics of
integer casts on resources. Using get_resource_id($file) would make the
intent clearer relative to (int) $file. This is similar to the recently
added get_mangled_object_vars() function, which essentially does the same
as (array) $object, but makes it obvious that you really are interested in
mangled properties.
I think something like get_resource_id() would be a good addition.
After all, I hope we will be able
to port all resources to objects sometime, and till then we could stick
with more lax semantics.I think that's really an argument to align the behavior of resources and
objects, as it means one less BC break when resources are converted to
objects. Historically that's something we did in minor versions, so it's
good to reduce the amount of semantics that change when such a conversion
is done.
Good point indeed!
So +1 from me on the RFC. Not absolutely sure about the open questing
regarding going one step further, but tentatively also +1 on that.
Thanks,
Christoph
Hi internals,
I would like to propose making the use of arithmetic/bitwise operators on
arrays, resources and (non-overloaded) objects a TypeError exception:https://wiki.php.net/rfc/arithmetic_operator_type_checks
This is inspired by some of the recent discussions, in particular the
operator overloading RFC (where the fact that operations on non-overloaded
objects still succeed with just a notice was criticized) and the
increment/decrement RFC, which handles the array case of this proposal for
inc/dec only.I think as-is, this RFC should be completely uncontroversial. However,
there is an open question on whether we want to make this slightly more
strict, in order to align the semantics with the rules for weak parameter
type checks for ints/floats.
On this topic maybe it would be good time to bring back again the Saner
string to number comparison RFC, at least in some form?
https://wiki.php.net/rfc/string_to_number_comparison
Or is this open question meant to replace it?
And maybe in some other form revive this RFC about allowing trailing
white-spaces in numeric string, because IMHO only accepting those
which have white-spaces in front is kinda weird.
If we do that, then this RFC could be a stepping stone towards making
"implicit" internal casts use the (weak) parameter type checking semantics
more generally, which I think would be a good idea. The current explicit
cast semantics we use everywhere are too forgiving for most circumstances
(e.g. an array is almost always not a reasonable input where an integer is
expected).Regards,
Nikita
Other than this a solid +1 from me.
George P. Banyard
Hi Nikita,
Nikita Popov wrote:
Hi internals,
I would like to propose making the use of arithmetic/bitwise operators on
arrays, resources and (non-overloaded) objects a TypeError exception:https://wiki.php.net/rfc/arithmetic_operator_type_checks
This is inspired by some of the recent discussions, in particular the
operator overloading RFC (where the fact that operations on non-overloaded
objects still succeed with just a notice was criticized) and the
increment/decrement RFC, which handles the array case of this proposal for
inc/dec only.
Sounds good!
I think as-is, this RFC should be completely uncontroversial. However,
there is an open question on whether we want to make this slightly more
strict, in order to align the semantics with the rules for weak parameter
type checks for ints/floats.
Regarding that, the RFC says we could “Make non-numeric string operands
throwing. Non-numeric here means not starting with a digit (optionally
preceded by whitespace). This would not apply to operators that have
special behavior for strings, such as string increment.”
I know you used “such as” there, but maybe you should specifically point
out the bitwise case. I think increment and the bitwise operators are
the only special cases for strings?
If we do that, then this RFC could be a stepping stone towards making
"implicit" internal casts use the (weak) parameter type checking semantics
more generally, which I think would be a good idea. The current explicit
cast semantics we use everywhere are too forgiving for most circumstances
(e.g. an array is almost always not a reasonable input where an integer is
expected).
That would be nice! We currently have essentially three (actually more
but these are the big ones) different “weak” behaviours: function
arguments and return values, non-comparison operators, and comparisons,
and I doubt people who don't follow internals are fully aware of the
differences, so it would be good to eventually have just one set of rules.
I guess https://wiki.php.net/rfc/invalid_strings_in_arithmetic was
already a step in this direction and your RFC is a further development
of it.
Thanks,
Andrea
Hi Nikita,
I would like to propose making the use of arithmetic/bitwise operators on
arrays, resources and (non-overloaded) objects a TypeError exception:
Thanks for writing this up; one of the conclusions when revising my inc/dec
RFC was that this should be proposed, but I've not had the energy to follow
through.
Discovering that objects become int(1) was a big WTF for me. I'd happily
see that throw an error even under an explicit cast - "(string)new class{}"
is currently an Error, but "(int)new class{}" and "(float)new class{}" are
only a Notice.Would it be possible to throw an Error in this case without
fixing the comparison operator quirk you noted in rfc/engine_warnings?
I initially thought resources made sense as they are, but like you I
concluded that the only real use is to get the ID itself, so explicit casts
are enough. There's a possibility that someone used to JS might write
$resource+0 instead of (int)$resource out of habit, but it doesn't seem
particularly likely, and is easy to fix. get_resource_id() is a good idea,
too; for similar reasons, I've often wished objects with __toString()
aliased it to a more specific method, rather than it being the only way to
get a certain representation.
While the behaviour of other types such as strings would be nice to
revisit, I think it's worth keeping this RFC to arrays, objects, and
resources, because other cases have a lot more to consider in terms of
detail and backward compatibility impact.
Regards,
Rowan Tommins
[IMSoP]
On Thu, Apr 2, 2020 at 3:11 PM Rowan Tommins rowan.collins@gmail.com
wrote:
Hi Nikita,
I would like to propose making the use of arithmetic/bitwise operators on
arrays, resources and (non-overloaded) objects a TypeError exception:Thanks for writing this up; one of the conclusions when revising my inc/dec
RFC was that this should be proposed, but I've not had the energy to follow
through.Discovering that objects become int(1) was a big WTF for me. I'd happily
see that throw an error even under an explicit cast - "(string)new class{}"
is currently an Error, but "(int)new class{}" and "(float)new class{}" are
only a Notice.Would it be possible to throw an Error in this case without
fixing the comparison operator quirk you noted in rfc/engine_warnings?
In preparation for this change, I've decoupled the error reporting from the
cast logic, so it is now possible to make just the cast throw, without
influencing comparison. However, there might be exception safety concerns,
along the same lines as https://wiki.php.net/rfc/tostring_exceptions for
the string cast case.
Nikita
I initially thought resources made sense as they are, but like you I
concluded that the only real use is to get the ID itself, so explicit casts
are enough. There's a possibility that someone used to JS might write
$resource+0 instead of (int)$resource out of habit, but it doesn't seem
particularly likely, and is easy to fix. get_resource_id() is a good idea,
too; for similar reasons, I've often wished objects with __toString()
aliased it to a more specific method, rather than it being the only way to
get a certain representation.While the behaviour of other types such as strings would be nice to
revisit, I think it's worth keeping this RFC to arrays, objects, and
resources, because other cases have a lot more to consider in terms of
detail and backward compatibility impact.Regards,
Rowan Tommins
[IMSoP]
On Thu, Apr 2, 2020 at 3:11 PM Rowan Tommins rowan.collins@gmail.com
wrote:
Hi Nikita,
I would like to propose making the use of arithmetic/bitwise operators on
arrays, resources and (non-overloaded) objects a TypeError exception:Thanks for writing this up; one of the conclusions when revising my inc/dec
RFC was that this should be proposed, but I've not had the energy to follow
through.Discovering that objects become int(1) was a big WTF for me. I'd happily
see that throw an error even under an explicit cast - "(string)new class{}"
is currently an Error, but "(int)new class{}" and "(float)new class{}" are
only a Notice.Would it be possible to throw an Error in this case without
fixing the comparison operator quirk you noted in rfc/engine_warnings?I initially thought resources made sense as they are, but like you I
concluded that the only real use is to get the ID itself, so explicit casts
are enough. There's a possibility that someone used to JS might write
$resource+0 instead of (int)$resource out of habit, but it doesn't seem
particularly likely, and is easy to fix. get_resource_id() is a good idea,
too; for similar reasons, I've often wished objects with __toString()
aliased it to a more specific method, rather than it being the only way to
get a certain representation.While the behaviour of other types such as strings would be nice to
revisit, I think it's worth keeping this RFC to arrays, objects, and
resources, because other cases have a lot more to consider in terms of
detail and backward compatibility impact.
I've now update the RFC to move the open question into future scope, as I
agree that this is a larger topic that should be discussed separately.
I plan to open voting on this RFC in a couple of days, if there are no
further concerns.
Nikita
Nikita Popov wrote:
I would like to propose making the use of arithmetic/bitwise operators
on arrays, resources and (non-overloaded) objects a TypeError
exception:https://wiki.php.net/rfc/arithmetic_operator_type_checks
This is inspired by some of the recent discussions, in particular the
operator overloading RFC (where the fact that operations on
non-overloaded objects still succeed with just a notice was
criticized) and the increment/decrement RFC, which handles the array
case of this proposal for inc/dec only.I think as-is, this RFC should be completely uncontroversial. However,
there is an open question on whether we want to make this slightly
more strict, in order to align the semantics with the rules for weak
parameter type checks for ints/floats.If we do that, then this RFC could be a stepping stone towards making
"implicit" internal casts use the (weak) parameter type checking
semantics more generally, which I think would be a good idea. The
current explicit cast semantics we use everywhere are too forgiving
for most circumstances (e.g. an array is almost always not a
reasonable input where an integer is expected).
I definitely support this proposal, and I'd love the "strict_operators" directive
as well, but that's a different proposal, and a different discussion.
The current proposal seems, in the words of the proposal, to be pretty uncontroversial:
It basically corrects a bunch of WTFs in PHP's type system.
Regards,
Terje