Boost logo

Boost :

From: Steven Watanabe (steven_at_[hidden])
Date: 2007-12-20 11:13:21


Ion Gaztañaga <igaztanaga <at>> writes:

> Hi all,
> The formal review of the Unordered library started December 7, we have a
> few nice reviews, but they are not enough. If you are interested in the
> library (and I *know* you are), please take some time to review it.

Sorry I didn't do a full review earlier. I just have a few
implementation comments.

line 118:
            typedef typename Allocator::value_type value_type;
Is there a reason you're not using allocator_value_type?

lines 165 and 217:
I don't think you want ADL here.

line 66: float_to_size_t:

I don't think the test used is correct. The following program prints "0"
under msvc 8.0:

#include <limits>
#include <iostream>

int main() {
    std::cout << static_cast<std::size_t>(
    static_cast<float>(std::numeric_limits<std::size_t>::max())) << std::endl;


lines 137-140:
Do you want ADL for hash_swap?

line 865:
Is there a reason not to use cached_begin_bucket_?

line 1282:
                return float_to_size_t(ceil(
should qualify ceil with std::

line 1361:
       rehash_impl(static_cast<size_type>(floor(n / mlf_ * 1.25)) + 1);

The implementation files use BOOST_DEDUCED_TYPENAME but
unordered_map and unordered_set use typename. Could you
make it consistant?

I'm getting a lot of warnings on the tests from msvc 8.0 with /W4
because minimal::ptr/const_ptr only defines operator+(int) and the internals
call operator+ with a size_t. Is + required to work for the size_type
or should it be cast to the difference_type explicitly? Also, minimal::ptr
should use std::ptrdiff_t rather than int.

In Christ,
Steven Watanabe

Boost list run by bdawes at, gregod at, cpdaniel at, john at