AMDG Ion Gaztañaga <igaztanaga <at> gmail.com> 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. allocator.hpp line 118: typedef typename Allocator::value_type value_type; Is there a reason you're not using allocator_value_type? lines 165 and 217: reset(ptr_); I don't think you want ADL here. hash_table.hpp: 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; } hash_table_impl.hpp 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); *std::*floor? 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