Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:43662
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 209.151.69.1 cause and error)
Message-ID: <B653F7D59B1D47A6A303C65F48B9714D@pc1>
To: <internals@lists.php.net>,
	"Dmitry Stogov" <dmitry@zend.com>
Cc: "Stas Malyshev" <stas@zend.com>,
	"Lukas Kahwe Smith" <mls@pooteeweet.org>,
	=?iso-8859-1?Q?Johannes_Schl=FCter?= <johannes@php.net>
References: <EB466381915442CFBBA6D6C9BCCB5392@pc1> <49D468CA.5050200@zend.com> <F29D36280ED8458AA3E2B3FA6BA94164@pc1> <49D65752.6020207@zend.com>
Date: Fri, 10 Apr 2009 14:45:37 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=response
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] double to long conversion change
From: php_lists@realplain.com ("Matt Wilmas")

Hi Dmitry,

I finally updated the patches:
http://realplain.com/php/dval_to_lval.diff
http://realplain.com/php/dval_to_lval_5_3.diff

After seeing how things work on Windows and [new for me] 32- and 64-bit 
Linux, there's no longer all that magic stuff I had before. :-)  Now the 
conversion method is basically just like what 5.2 has, and its behavior 
that's been there for years (I assume, for the majority of users, at least 
for positive numbers).  The difference is that on 32-bit, it now uses a 
(zend_long64) cast, which should fix the inconsistency with the Mac that Zoe 
reported in Bug #42868, which lead to the change in 5.3.  (It appears that 
the (unsigned long) cast was limiting to ULONG_MAX, instead of overflowing 
and preserving least significant bits like other platforms.)

NOTE: The (zend_long64) part is exactly the same thing you did (to ensure 
overflow?), for positive values, in April '07, zend_operators.c v1.269 for 
"WIN64 support" (I don't think Win64 is any different than Win32 -- a 
difference in compilers between VC6 and newer? Yes).

This should handle the 64-bit range of values.  From what I've been able to 
check, once a double's exponent is above 64 (or is it 63? whatever), the 
conversion doesn't work (no different than PHP 5.2), without fmod... 
Outside of -2^63 and 2^63 on 32-bit Linux/Windows, you get 0; on 64-bit 
Linux (*like 5.2*), above 2^64 is 0 and below -2^63 is LONG_MIN.

I also added a configure check and flag if a simple (long) cast is 
sufficient to preserve LSB.  Don't know if I put it in the "right" place 
(Zend.m4) or if it should be kept, but it's there.

BTW, I used a function so it's easier to use than the current macro.  With 
zend_always_inline, it should usually be the same...?  I would have used a 
macro with ?: but that was used previously before removal from 
zend_operators.c in Sep '06, v1.256: "use if() instead of ?: and avoid 
possible optimization problems."

More below...

----- Original Message -----
From: "Dmitry Stogov"
Sent: Friday, April 03, 2009

> Hi Matt,
>
> I don't really see why we should "preserve the least significant bits"

Because that's presumably what most users have seen, at least for positive 
numbers, in previous versions for years?  And I think it makes sense, 
effectively truncating the number in that way.  Obviously no out-of-range 
number can be correct, but at least preserving LSB has a *chance* of giving 
the correct result in the case of bitwise ANDing a large number with a small 
one, for example.

You asked Stas to take a look as well, and I see that he was the one who 
introduced the 5.2 and prior behavior to overflow/preserve LSB (again, for 
most users, etc.) by adding the (unsigned long) cast in Jul '01, 
zend_operators.c v1.105: "fix double->long conversion" (though it doesn't 
say what was broken :-)).

> and I don't think we should support bitwise operations with doubles.

But we do. :-)  (I don't know if you're talking about keeping LSB or my 
other "64-bit operators" patch for doubles on 32-bit platforms.)

It's not an error ("Invalid operand types"), even if out of long range.  As 
I said above, the 5.2 and prior method has a chance of working correctly in 
some cases, unlike a limit.

Anyway, I don't see why bitwise operations with doubles wouldn't be 
supported, as much as is possible.  With the PHP way of "do what we can to 
make it work" and all. :-)  That was my thinking with the "64-bit operators" 
patch -- it's possible, simple, and brings consistency with 64-bit 
platforms.

> Stas, could you please look into this too.
>
> Thanks. Dmitry.

Thanks,
Matt