[cfe-commits] [libcxx] r113731 - in /libcxx/trunk: include/iterator include/type_traits src/hash.cpp

Howard Hinnant hhinnant at apple.com
Tue Sep 14 13:26:41 PDT 2010


Hi Matthieu,

Thanks for the review!

On Sep 14, 2010, at 11:40 AM, Matthieu Monrocq wrote:

> Hi Howard,
> 
> I find strange to use `const typename Container::value_type&` instead of `typename Container::const_reference` for the insert iterators: it prevents the user from using a proxy instead of a plain reference (as it would prevent her to use a smart pointer instead of a plain pointer were you to use `const typename Container::value_type*` instead of `typename Container::const_pointer`).

My first preference is also Container::const_reference.  That being said, when vector<bool>::const_reference is actually a bool [see below] (which is specified by the standard), then the use of Container::const_reference creates an ambiguous overload set:

  back_insert_iterator<Container>& operator=(bool value);
  back_insert_iterator<Container>& operator=(bool&& value);

This ambiguity was noted and

http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-active.html#1334

was opened to address it.  The solution currently given in 1334 isn't the one I coded because some others on the committee are proposing what I did code.  This new solution will hopefully be published publicly soon.  At any rate, the coded solution works for vector<bool>::const_reference==bool.

libc++'s implementation doesn't use vector<bool>::const_reference==bool at the moment.   Instead it uses a true proxy class which emulates a const bool&.  I believe this is higher quality than what the standard specifies, not strictly conforming, and I've received no complaints on it.  At any rate, the const value_type& solution is also passing tests with this vector<bool>::const_reference.

So in summary, this approach does seem to work with proxy references.  Mainly because proxy references always seem to need an implicit conversion to their value_type.

> 
> Also, there seems to be a discrepancy (for the insert iterators) between the parameter of the template `Container` and the value you used as a replacement `_Container` (note the leading underscore) on the lines 154, 175 and 197. It is fine on the other lines because the template parameter is indeed `_Container` later on.
> 

Ah, you've caught an error in the "synopsis comments" at the top of the header.  I accidentally put _Container up there instead of Container in a few places.  Thanks!

-Howard





More information about the cfe-commits mailing list