Re: [boost] [review] promotion traits

From: Andy Little (andy_at_[hidden])
Date: 2006-04-04 19:32:37

Tobias Schwinger wrote:

> - What is your evaluation of the design?

I wonder if the three functions are necessary together?

Either there should only be promote or there should only be integral_promotion,
float_promotion.
The main rationale for integral_promotion is for enums. float promotion and
promotion usage are already handled by Boost.Typeof.

ints and floats are very different entities and I wonder if its approporiate to
apply the same behaviours. An example of the difference is the
boost::integral_constant<T,Tvalue>, where the type is naturally constrained to
be an int or enum but floats arent valid.

I dont really like the fact types unrelated to int or floats can silently pass
through as a general rule, but I think I see the reasoning given the use cases.
It could be documented though.

> - What is your evaluation of the implementation?

I havent looked at the implementation.

> - What is your evaluation of the documentation?

I think it needs some more rationale and examples. I understand its meant to fit
with type_traits, but the documentation for other type_traits does include
examples and there is room for a bit of rationale too.

> - What is your evaluation of the potential usefulness of the library?

The main use seems to be limited to enums. If the type of promotion_traits can
determine the number of bits an enum requires to be stored in then that is
useful. That seems to be a feature unique to this library, but I havent tested
it out to see if it works as it failed in VC7.1

> - Did you try to use the library?  With what compiler?

Tried <libs/type_traits/promote_enum_test.cpp> in VC7_1 and gcc4.0.
 succeeded in gcc4.0

>  Did you have any problems?

<libs/type_traits/promote_enum_test.cpp> Failed in VC7.1 (Extensions disabled)

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

A quick reading.

> - Are you knowledgeable about the problem domain?

Yes.

> Please always explicitly state in your review, whether you think the library
> should be accepted into Boost.

I vote yes to accept it.

regards
Andy Little