[boost] Re: Numeric Conversion Formal Review

From: Guillaume Melquiond (guillaume.melquiond_at_[hidden])
Date: 2003-12-12 12:47:40

Le lun 08/12/2003 à 13:04, Thorsten Ottosen a écrit :

[...]

>   a.. What is your evaluation of the design?

I think the design is good. The number of parameters of the converter
template seems a bit high. In particular, was it really necessary to
provide both a Float2IntRounder and a RawConverter parameters?

I also don't really understand the interest of having both an
OverflowHandler and a UserRangeChecker::validate_range. I would simply
remove this member since its only purpose is to use OverflowHandler (the
converter could do that itself, no need for the user to provide it).

And bounds::smallest could return 1 rather than 0 for integers.

>   b.. What is your evaluation of the implementation?

As I explained in other mails, there are a few shortcomings with the
rounding from a floating-point value to an integer when rounding to
nearest.

Other than that it seems alright to me. But it is bit hard to verify a
code with a design thought to be fully meta-programing-compliant :-).

Could it be possible for the library to use another directory tree? The
root directory boost/ is already a dumping ground, it's not a reason for
the subdirectory boost/numeric/ to become one also :-). I would let only
cast.hpp in this directory (so we can #include <boost/numeric/cast.hpp>)
but the other files should be put in their own subdirectory.

>   c.. What is your evaluation of the documentation?

The documentation may benefit from a bit of tidying, in particular in
the definitions. However, it is great to have such a big documentation
for such a "small" library. That's a good thing.

>   d.. What is your evaluation of the potential usefulness of the library?

This library is really useful. A few languages (e.g. Ada) always check
that conversions are meaningful but C++ doesn't. Now it will be possible
in a "standardized" way. I'm a bit surprised that lexical_cast was born
before numeric_cast.

>   e.. Did you try to use the library?  With what compiler?  Did you have any
> problems?

I only tried with GCC 3.3. I didn't encounter any problems.

On an architecture with an extended precision floating-point unit
(namely x86), the converter_test will fail on d->f because of compiler
optimizations (the use of a 10-byte floating-point temporary). It may be
better to add a volatile keyword to neutralize it (I only tested with
GCC but I suppose other compilers would probably behave similarly).

There was also a problem with udt_support_test at runtime.

>   f.. How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?

A few hours. The study was in-depth concerning floating-point behavior
since it's my field.

>   g.. Are you knowledgeable about the problem domain?

I'm knowledgeable about computer arithmetic. I'm not that knowledgeable
when it comes to advanced C++ design.

>   h.. Do you think the library should be accepted as a Boost library?  Be
> sure to say this explicitly so that your other comments don't obscure your
> overall opinion.

I think the library should be accepted: the design seems good to me and
it will be a really useful addition. However the implementation needs to
be corrected first (or the documentation should explain its
shortcomings).

Regards,

Guillaume