Re: [PHP-DEV] [VOTE][RFC] Integer Semantics

On 16 September 2014 11:05, Dmitry Stogov <[email protected]> wrote:
> you already made silent break for N << 64 and N >> 64, but it may be
> explained as more consistent behaviour.
> I don't see a big difference with negative shifts.
>
> The real thing that I don't like - is a "boolean" result. Warning is not a
> big problem.

I'm inclined to agree with this. The warning makes sense as it's
almost certainly a userland bug, and if the expected old behaviour is
desired it can easily be modified in userland, but returning FALSE
doesn't make a huge amount of sense. This will almost certainly be
immediately cast to int(0) by the next operation - almost no-one is
going to actually check for a return value of FALSE - so it makes more
sense to just return 0 and avoid the implicit cast. This would also
produce possibly-unexpected results if a future operation was to
stringify the result, as FALSE would cast to the empty string.

I have voted in favour of the RFC as it stands as I believe that
overall the changes are positive, but ideally this particular case
would be addressed.

>
> Thanks. Dmitry.
>
>
>
> On Tue, Sep 16, 2014 at 1:23 PM, Andrea Faulds <[email protected]> wrote:
>
>>
>> On 16 Sep 2014, at 10:16, Dmitry Stogov <[email protected]> wrote:
>>
>> > Shifts by negative number may make sense. (N << -1) => (N >> 1)
>> > At least receiving "false" from shift is not very pleasant.
>>
>> The problem is that changing from the current behaviour (undefined in C,
>> but typically a shift by (PHP_INT_MAX - $bits)) to do what you’d expect
>> (shift in the opposite direction) would be a silent BC break. I think it’s
>> better to just stop it altogether and raise an E_WARNING, where it’s at
>> least obvious something went wrong, than change this behaviour silently.
>>
>> > In the patch you use SIZEOF_LONG. It probably should be changed to
>> > SIZEOF_ZEND_LONG.
>>
>> Will fix, thanks for spotting that.
>>
>> --
>> Andrea Faulds
>> http://ajf.me/
>>
>>
>>
>>
>>