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,
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?
> Please always explicitly state in your review, whether you think the library
> should be accepted into Boost.
I vote yes to accept it.