Just to preempt this, I think it will be more appropriate for any review discussions to take place on the developers' list (boost@lists.boost.org). On Wed, May 30, 2012 at 2:38 PM, Vicente J. Botet Escriba < vicente.botet@wanadoo.fr> wrote:

...

Hi all,

The review of the proposed Boost.Multiprecision library authored by John Maddock and Christopher Kormanyos has been scheduled for

June 8th - June 17th, 2012

and will be managed by myself.

I hope everyone interested can reserve some time to read through the documentation, try the code out, and post a formal review, either during

Le 29/05/12 23:08, Jeffrey Lee Hellrung, Jr. a écrit : the formal review window or before.

...

Hi,

glad to see that the library will be reviewed soon.

I have spent some hours reading the documentation. Here are some comments and a lot of questions.

* As all the classes are at the multi-precision namespace, why name the main class mp_number and not just number?

typedef mp::number<mp::mpfr_float_**backend<300> > my_float;

* I think that the fact that operands of different backends can not be mixed on the same operation limits some interesting operations:

I would expect the result of unary operator-() always signed? Is this operation defined for signed backends? I would expect the result of binary operator-() always signed? Is this operation defined for signed backends? what is the behavior of mp_uint128_t(0) - mp_uint128_t(1)?

It would be great if the tutorial could show that it is possible however to add a mp_uint128_t and a mp_int256_t, or isn't it possible? I guess this is possible, but a conversion is needed before adding the operands. I don't know if this behavior is not hiding some possible optimizations.

I think it should be possible to mix backends without too much complexity and that the library could provide the mechanism so that the backend developer could tell to the library about how to perform the operation and what should be the result.

* Anyway, if the library authors don't want to open to this feature, the limitation should be stated more clearly, e.g in the reference documentation "The arguments to these functions must contain at least one of the following:

An mp_number. An expression template type derived from mp_number. " there is nothing that let think mixing backend is not provided.

* What about replacing the second bool template parameter by an enum class expression_template {disabled, enabled}; which will be more explicit. That is

typedef mp::mp_number<mp::mpfr_float_**backend<300>, false> my_float;

versus

typedef mp::mp_number<mp::mpfr_float_**backend<300>, mp::expression_template::**disabled> my_float;

* As I posted in this ML already I think that allocators and precision are orthogonal concepts and the library should allow to associate one for fixed precision. What about adding a 3rd parameter to state if it is fixed or arbitrary precision?

* Why cpp_dec_float doesn't have a template parameter to give the integral digits? or as the C++ standard proposal from Lawrence Crow ( http://www.open-std.org/jtc1/**sc22/wg21/docs/papers/2012/**n3352.html<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3352.html>), take the range and resolution as template parameters?

* What about adding Throws specification on the mp_number and backend requirements operations documentation?

* Can the user define a backend for fixed int types that needs to manage with overflow?

* Why bit_set is a free function?

* I don't see nothing about overflow for cpp_dec_float backend operations. I guess it is up to the user to avoid overflow as for integers. what would be the result on overflow? Could this be added to the documentation?

* can we convert from a cpp_dec_float_100 to a cpp_dec_float_50? if yes, which rounding policy is applied? Do you plan to let the user configure the rounding policy? BTW, I see in the reference "Type mp_number is default constructible, and both copy constructible and assignable from: ... Any type that the Backend is constructible or assignable from. " I would expect to have this information in some way on the tutorial. I will appreciate also if section "Constructing and Interconverting Between Number Types" says something about convert_to<T> member function.

If not, what about a mp_number_cast function taking as parameter a rounding policy?

* Does the cpp_dec_float back end satisfies any of the Optional Requirements? The same question for the other backends?

* Is there a difference between implicit and explicit construction? * On c++11 compilers providing explicit conversion, couldn't the convert_to function be replaced by a explicit conversion operator? * Are implicit conversion possible?

* Do you plan to add constexpr and noexcept to the interface? After thinking a little bit I'm wondering if this is this possible when using 3pp libraries backends that don't provide them?

* Why do you allow the argument of left and right shift operations to be signed and throw an exception when negative? Why don't just forbid it for signed types?

* Why the "Non-member standard library function support" can be used only with floating-point Backend types? Why not with fixed-point types?

* What is the type of boost::multiprecision::number_**category<B>::type for all the provided backends? Could the specialization for boost::multiprecision::number_**category<B>::type be added in the documentation of each backend? and why not add also B::signed_types, B::unsigned_types, B::float_types, B::exponent_type?

* Why have you chosen the following requirements for the backend? - negate instead of operator-() - eval_op instead of operator op=() - eval_convert_to instead of explicit operator T() - eval_floor instead of floor

Optimization? Is this optimization valid for short types (e.g. up to 4/8 bytes)?

* As the developer needs to define a class with some constraints to be a model of backend, which are the advantages of requiring free functions instead of member functions? * Couldn't these be optional if the backend defines the usual operations? * Or could the library provide a trivial backend adaptor that requires the backend just to provide the usual operations instead of the eval_xxx? * How the performances of mp_number<this_trivial_**adaptor<float>, false> will compare with float?

* And last, I don't see anything related to rvalue references and move semantics. Have you analyzed if its use could improve the performances of the library?

Good luck for the review. A really good work. Vicente

______________________________**_________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/**mailman/listinfo.cgi/boost-**users<http://lists.boost.org/mailman/listinfo.cgi/boost-users>