Re: Fixed String review

From: Gavin Lambert (boost_at_[hidden])
Date: 2019-11-27 00:12:33

On 27/11/2019 06:33, Vinnie Falco wrote:
>> I would much prefer static_string
> 
> Same here... I was foolish to rename it.

Neither one is a great choice, as both "static" and "fixed" can be 
synonyms of "immutable", which this is not.

"fixed_capacity_string" is the most accurate name -- while you might 
think that's too long, I think it's better to be properly descriptive 
and allow consumers to typedef it to something easier to type if they 
wish to.

>> Regarding your deviations from std::string:
>> operator+ has been omitted because you consider it's not possible
>> to determine a reasonable return type; isn't it just size N+M ?
>> Do you reject that just because it could end up rather large after
>> a sequence of additions?  I'd agree if it were N*M but not really for
>> N+M.  Have I missed something?
> 
> Yes. It is too easy for someone to change existing code that does
> a+b+c+d where the operands are strings. Users could the get surprising
> behavior of tens or hundreds of kilobytes of stack consumption. There
> are other ways to implement operator+ but we prefer to the omit
> interfaces that can have surprising behavior, or that have non-obvious
> results (such as truncating).

The most logical capacity for the result of operator+ would probably be 
std::max(a.capacity(), b.capacity()).  Bear in mind that we are assuming 
that both actual string values are significantly shorter than their 
capacities throughout most of their lifetime.

(This is a reasonably safe assumption especially when you *don't* have 
an operator"" that produces exactly-right-sized strings.)

In an ideal world, you'd be able to factor in the current lengths, but 
of course that isn't available at compile time so can't be used to 
determine the type.

Having said that, operator+ is still going to cause an allocation, so it 
does still make sense to omit it from the interface for that reason, so 
that people think about it more (usually by switching to zero-allocation 
append or +=).

>> You've chosen to return a string_view from substr().  I think that's
>> a bit dangerous; code that does "auto a = s.substr(...)" could end
>> up with a dangling view when s is a fixed string; this makes it
>> more difficult than it should be to migrate from one string type
>> to another, or to write generic code.
> 
> Again we have to refer to the purpose of the container. People are
> using this for performance, the very last thing they want from
> substr() is to receive a copy. Users can opt-in to making a copy if
> they want, by constructing a new static_string from the string view
> returned by substr. If we go with your suggestion, no one can opt out
> of copies.

Perhaps as a compromise (due to the difference from std::string 
interface anyway) this method should be named substr_view?  This would 
then alert someone converting from one to the other to look at each call 
site and verify that it isn't going to produce a dangling view.

>> My initial thought was that static_string should be implemented
>> on top of boost::container::static_vector<char>. My rationale...
> 
> No normal user is going at static_string and think "I would use this
> if it required Boost.Container." Fewer dependencies is better. And,
> this library is designed to work without the rest of Boost (the
> "standalone" mode). I want people to be able to do "vcpkg install
> static_string" and then be ready to boogie. They will still have to
> type `boost::` to access it.

That just means that standalone mode would need to depend on a 
standalone implementation of static_vector, which vcpkg would pull in as 
well.  Or you should give up on a standalone mode and have your vcpkg 
depend on Boost.

Dependencies can be annoying, but constantly reinventing existing 
vocabulary types is also annoying.  Waiting until they get adopted by 
the standard before using them in other libraries is a bit silly.