Re: Iterators for basic_string/vector

[Benjamin Kosnik <bkoz@cygnus.com>]

Wow. Let me say how pleased I am to be reviewing a patch off the CVS
repository! Thank you thank you thank you. Can you do me a favor and
foward me your ChangeLog entry? 

> an overhead: the empty base class has a non-zero size which doubles
> the size of the iterator class (on Intel x86 with g++ 1.1.1 anyway.)
> This is almost certainly unacceptable in this case so for that reason
> I default to a non-inherited implementation.

did you try -fnew-abi? I thought that contained the empty base class
optimization. Nathan knows more about this, but I thought the penalty for
this coding style had been eliminated.

> Secondly, the T* constructor and T* data member are public.  I
> considered making these private and use friendship to access them, but
> that would require forward declarations of basic_string and vector and
> I do not know what the policy is here.  Using friendship would also
> prevent the use of this class in containers defined in the user space.

there is a foward defs file, at least for string. See 

src.libstdc++/stl/bits/stl_string_fwd.h

> Thirdly, I put the iterator template in namespace std:: but perhaps it
> should go somewhere else.

again, I think designating "ext" or somesuch namespace now will pay
dividends in the future. This means we could put STL extensions into this
as well, and more clearly define what is "std" from what is not in iso
14882. 

> I cannot claim to have exhaustively tested the full basic_string and
> vector functionality with these changes, but I have tried a range of
> things in addition to the iterator tests. 

We need a test case for this. Would you be willing to submit one (or what
you are using for the beginnings of one)? See testsuite/21 for some
examples.


Thanks again,
Benjamin