OK, finally(!), here comes my review of this library - purely as a potential user, not as review manager. First off the headline: I think the library should be accepted into Boost. fpclassify ~~~~~~~~~~ Having already demonstated my ignorance of the aliasing rules, I'm quite happy to accept this as is :-) There's little to say design wise, as it follows existing practice, so as long as Johan is happy to see this merged with my code that handles non-builtin floating point numbers (class types like NTL::RR or mpfr_class etc) then I'm happy. 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. 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 :-) 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. 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? 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 :-) 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. 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? Overall nice job. Regards, John.