EricWF added a comment.
drive-by comments.
================
Comment at: include/experimental/functional:159
@@ +158,3 @@
+ _LIBCPP_INLINE_VISIBILITY
+ void insert(const key_type &__key, value_type __val)
+ {
----------------
Do we want to copy the `__val` here?
================
Comment at: include/experimental/functional:161
@@ +160,3 @@
+ {
+ __table [__key] = __val; // Would skip_.insert (val) be better here?
+ }
----------------
> Would skip_.insert(val) be better here?
I don't think so but it depends on the semantics of this `insert` method.
Should existing values be overwritten?
================
Comment at: include/experimental/functional:181
@@ +180,3 @@
+ typedef typename std::make_unsigned<key_type>::type unsigned_key_type;
+ typedef std::array<_Value, 1U << (CHAR_BIT * sizeof(key_type))> skip_map;
+ skip_map __table;
----------------
`std::numeric_limits<unsigned_key_type>::max()` might be clearer. It took me a
while to figure out what this was doing.
================
Comment at: include/experimental/functional:192
@@ +191,3 @@
+ _LIBCPP_INLINE_VISIBILITY
+ void insert(key_type __key, value_type __val)
+ {
----------------
Do we want the extra copy of `__val` here?
================
Comment at: include/experimental/functional:215
@@ +214,3 @@
+ _VSTD::is_integral<value_type>::value && // what about
enums?
+ sizeof(value_type) == 1 &&
+ is_same<_Hash, hash<value_type>>::value &&
----------------
Is this only meant to trigger for `char` and `bool`? I think it would be wrong
to use the array specialization for `bool` because it can only represent 2
values but the array will end up having a size of `256`.
http://reviews.llvm.org/D11380
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
