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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
