Re: [boost] Ann: Floating Point Utilities Review starts today

From: Johan Råde (rade_at_[hidden])
Date: 2008-02-27 08:32:12

John Maddock wrote:
> It's been suggested that isinf should indicate the sign of the infinity. 
> Personally I would prefer isinf etc to return a bool as per TR1 than return 
> +1 or -1 etc to indicate the sign as well (as per glibc) - we have other 
> functions for that purpose, and IMO a function name should indicate it's 
> purpose clearly - relying on the sign of the result of isinf seems like a 
> recipe for hard to maintain code to me... just my 2c worth.

I agree.

> 
> Sign Manipulation
> ~~~~~~~~~~~~~~~~~
> 
> As these use the same core code as the fpclassify code, I'm happy with the 
> implementation.  We've discussed changesign previously, so I won't labour 
> the point again :-)

I will remove changesign from the library.

> There is one other useful function worth having IMO:
> 
> template <class T>
> int sign(T x);
> 
> returns 0 (x is zero), +1 (x is > 0) or -1 (x is < 0).  This is trivial to 
> implement (lot's of old C code - for example the Numerical Recipies stuff - 
> implement this as a helper macro), and there's an undocumented version 
> currently in Boost.Math.  Is this worth integrating with Johan's code?  I've 
> found it quite useful in Boost.Math from time to time.

I'd be happy to add that.

> Number Facets
> ~~~~~~~~~~~~~
> 
> I think these are a good idea, a few comments about the interface follow:
> 
> 1) I assume that these:
> 
> const int legacy;
> const int signed_zero;
> const int trap_infinity;
> const int trap_nan;
> 
> Should be declared "static" as well?

Stupid me.

> 2) I'm not completely sure about the "legacy" flag: shouldn't the facet just 
> read in everything that it recognises by default?  Or maybe we should flip 
> the meanings and "legacy" should become "strict_read" or "nolegacy" or 
> something - I'm not that keen on those names, but Boosters can usually come 
> up with something snappy when asked :-)

There are cases where the legacy flag could change the meaning of a file.
For instance, if you set the legacy flag, and parse the string "1#2", then
"1#" will be interpreted as the beginning of "1#INF" or some other Visual Studio nonsense.
Once the parser gets to '2', it will realize its mistake,
but when you read using stream iterators it is impossible to backtrack,
so the parser will set the failbit of the stream.

So the legacy flag can be a little dangerous.
This should of course be pointed out in the docs.

> 3) I think someone else mentioned that there is a lot of boilerplate code 
> that gets repeated when using these facets, so I think I'd like to see 
> something like:
> 
> std::locale get_nonfinite_num_locale(int flags = 0, std::locale base_locale 
> = std::locale());
> 
> which returns a locale containing the same instantiations of these facets 
> that a default locale would have for num_put and num_get, which is to say:
> 
> nonfinite_num_get<char>,
> nonfinite_num_get<wchar_t>
> nonfinite_num_put<char>
> nonfinite_num_get<wchar_t>
> 
> Then we could simply write:
> 
> my_iostream.imbue(get_nonfinite_num_locale());
> 
> or
> 
> std::locale::global(get_nonfinite_num_locale());
> 
> and be done with it.

Since I now have received two requests for this feature, I will add it.

> 4) I think the flags are useful, but would be a lot nicer as iostream 
> manipulators - although I accept this may not work with lexical_cast :-(
> 
> BTW what was the issue that would require separate file compilation for 
> manipulators?

It is about four years since last time I defined my own manipulators,
but this is how I remember it:

When you define a manipulator you need to store some state information with the stream.
Streams contain an integer array that should be used for that purpose.
The nonfinite_num_facets would need to reserve a position in those arrays.
That is done by calling std::xalloc, exactly once,
and storing the returned integer value in a global integer variable.
You must make sure that there is exactly one instance of that variable.
If you pass stream objects between different DLL's,
then you must make sure that all the DLL's share the same instance of that variable.
Can that be guaranteed without having a cpp file,
and compiling that file as a DLL that is used by the other DLL's?

Any comments?

> Overall nice job.
> 
> Regards, John. 

Thank you,
Johan