Travis Vitek wrote:
Thanks for the careful review!
Eric Lemings wrote:
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]
Author: faridz
Date: Wed Apr 23 08:20:07 2008
New Revision: 650902
URL: http://svn.apache.org/viewvc?rev=650902&view=rev
Log:
2008-04-23 Farid Zaripov <[EMAIL PROTECTED]>
STDCXX-880
* src/num_put.cpp: Added simple overloads of the
__rw_isfinite(), __rw_signbit(), __rw_isinf(),
__rw_isnan(), __rw_isqnan(), __rw_issnan()
for float and long double types.
Modified:
stdcxx/trunk/src/num_put.cpp
With just a quick review, this change looks a bit suspicious. The
respective overloads for type double are all defined within
conditional-compile directives. It follows that float and long double
overloads would be defined similarly unless I'm missing something.
The overloads on double are in platform specific #ifdef blocks because
they have platform specific code in them (_finite, finite, isfinite). At
least that is the only reason I see for it. So I don't really see the
overloads on float and double outside of platform specific blocks as a
problem as there is no platform specific code in them.
I guess I don't understand why the float overloads are all
unconditionally hardcoded:
http://fisheye6.cenqua.com/browse/stdcxx/trunk/src/num_put.cpp?r=trunk#l184
I'm more worried about why the overloads on float and long double are
added at all. If they are only being called from __rw_fmat_infinite(),
and that function only works with a double, then I see no motivation for
the overloads.
Hmm. I thought Farid was saying __rw_fmat_infinite() was only being
called with a double argument. It looks like it's potentially called
with all three floating point arguments except the __rw_isfinite()
function is hardcoded for float and long double to return false so
__rw_fmat_infinite() is only called for doubles. So he was right,
but the way it's done seems very confusing. Why do we bother to call
__rw_isfinite(float) and __rw_isfinite(long double) at all when we
know that they're hardcoded to return true?
I'm also wondering why we provide default implementations of these
functions if we know good and well that the results are wrong (for
platforms that don't define _WIN32, _RWSTD_OS_SUNOS, or fpclassify).
Shouldn't we fail to compile in this case?
My understanding is that the defaults will make the facet fall
back on using sprintf(). Farid, can you confirm that? (I don't
think we should ever fail to compile this code as long we have
sprintf to fall back on).
The other thing that freaks me out is the implementation of __rw_signbit
on platforms defining _RWSTD_OS_SUNOS. It assumes something similar to
IEEE-745 double representation. This may be a safe assumption for the
time being, but what happens if the user compiles with -fnonstd? Does
this still work?
It should although I haven't tried it. The bit pattern doesn't
change. (FWIW, I'm responsible for the function on Solaris).
Here's the documentation for reference:
http://docs.sun.com/app/docs/doc/819-5265/bjapp?a=view#bjarb
Martin
