On 16 September 2014 11:05, Dmitry Stogov <dmi...@zend.com> 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 <a...@ajf.me> wrote: > >> >> On 16 Sep 2014, at 10:16, Dmitry Stogov <dmi...@zend.com> 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/ >> >> >> >> >> -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
