Re: [boost] Re: static sized strings

From: Rob Stewart (stewart_at_[hidden])
Date: 2004-05-26 08:13:33

From: "Reece Dunn" <msclrhd_at_[hidden]>
> Rob Stewart wrote:
> >From: "Reece Dunn" <msclrhd_at_[hidden]>
> > > Rob Stewart wrote:
> > > >* Should fixed_string support zero-length strings?
> > >
> > > You can have zero-length strings in my implementation (this is what
> > > fized_string() defaults to). What you *can't* have is a zero-*capacity*
> > > string, which is the same as CharT[ 0 ].
> >
> >I did mean zero-capacity.  I didn't look at your implementation
> >at this point, but I did notice that John's class declares a size
> >N array, so for N == 0, his would fail to compile.  Is the same
> >true of yours?
> 
> yup. With mine, you'll also get an additional error, e.g.:
>    fixed_string.hpp(101) : error C2039: 'value' : is not a member of 
> 'boost::fixed_string<n>::zero_buffer_error'
> 
> Is there any reason you'd want a zero-capacity buffer?

My only thought was generic code that might be called upon to use
a zero-capacity string.  Such a string wouldn't be of much use,
of course, so it is probably best to disallow it.  I just wasn't
sure whether yours would disallow it.

> > > >* fixed_string's at() shouldn't test the length and throw an
> > > >   exception since detail::basic_string_impl::at() does it and
> > > >   detail::basic_string_impl::operator[] calls it.
> > >
> > > fixed_string tests against capacity as a measure against [] usage that 
> >would
> > > cause buffer overflow, e.g.
> > >    fixed_srting< 5 > str;
> > >    str[ 10 ] = 'a'; // ok: exception
> > > vs
> > >    char str2[ 5 ];
> > >    str[ 10 ] = 'a'; // foobar
> >
> >Fine, but there should only need to be one test per call to
> >at().  If size() can never exceed capacity, by enforcing
> >invariants elsewhere, then at() only needs to check that the
> >index doesn't exceed size().
> 
> But the standard specifies that at will throw if i > size(), but operator[] 
> does not. The i > capacity() check in the implementation is a sanity check 
> for operator[] and the real at function [note: I will change the name as 
> suggested to prevent hiding]. Therefore it is technically redundant for at, 
> but needed for []. I could supply an additional function (adding to those 
> that need to be implemented by the string provider) or add a i > capacity() 
> check on operator[] (deviating from the standard, but providing a sanity 
> check).

OK.  I understand your purpose now.  Since the purpose of
fixed_string is to provide buffer overflow protection and fixed
size, and to otherwise satisfy the requirements of
std::basic_string, I think it is most appropriate to add the
check the one place it is needed: operator [].  That avoids the
added overhead for at() and puts the check precisely where it's
needed.  I think the right behavior is for basic_string_impl to
rely on an at()-like function in fixed_string to get access to an
element, and for basic_string_impl to do all of the range
checking.  That simplifies the job for the derived class'
implementor and ensures that the tests are done correctly.

> >If you're not going to implement get_allocator(), then it should
> >be in basic_string_impl and shouldn't be among the requirements
> >levied upon derivates.
> 
> ok. Do you have a way to do this?

Sorry, I meant to write, "then it should not be in
basic_string_impl."

> > >From your fixed_string.hpp:
> >       if( l == npos ) l = StringPolicy::length( s ) + 1;
> 
> This is referring to a char_traits policy (as it operates on strings):
> 
>       template< size_t n, typename CharT = char, class StringPolicy = 
> std::char_traits< CharT > >
>       class fixed_string;
> 
> The basic_string_impl used StringPolicy for the Derived type (now renamed to 
> Derived) because it was based around flex_string. Sorry for the confusion.

I see now.  It was confusing.  May I suggest that StringPolicy be
named "CharStringPolicy" and that fixed_string::string_policy be
named "char_string_policy" or something like that?  This will
further distance the policy class from fixed_string itself.

> > > To simplify my design, I could implement basic_string_impl using the 
> >policy
> > > approach that flex_string uses. The problem with this is that it 
> >requires a
> > > usage like:
> > >
> > >    flex_string< fixed_string< 10, char > > str;
> > >
> > > so in order to get the fixed_string< n > syntax, I would need two 
> >classes
> > > instead of just the one: this was something I was trying to move away 
> >from,
> >
> >Sorry, I don't know anything about flex_string, but what you've
> >shown would only serve to make your string class more painful
> >than John's (or your) current approach.
> 
> It would be easy to make it usable like it currently is:
> 
>    template< size_t n, typename CharT = char, class Traits = 
> std::char_traits< CharT > >
>    class fixed_string: public detail::basic_string_impl< 
> detail::fixed_string_impl< n, CharT, Traits >, ... >
>    {
>       public:
>          // constructors
>    };
> 
> It's just that it means that you need to do this for every string 
> implementation. Or you could use the template typedef hack, so it would be:
> 
>    fixed_string< 10 >::type str;
> 
> but this looks odd, especially to novice users or people who don't have 
> experience with template metaprogramming. This was one of the motivations 
> for my approach as opposed to the flex_string approach.

OK, but I'm not certain what this buys you.  If it improves
matters for you, then your alternative approach in which
fixed_string_impl is a basic_string_impl parameter is much
better.  It's a one-time imposition on the developer of a derived
class and has no direct impact on clients of the derived classes.

The only concern I have is the (likely) slower compilation speed
and worsened error messages.

-- 
Rob Stewart                           stewart_at_[hidden]
Software Engineer                     http://www.sig.com
Susquehanna International Group, LLP  using std::disclaimer;