On 27/11/2019 06:46, Peter Dimov wrote:

Vinnie Falco wrote:

...

In which use-cases do you anticipate the need for a constexpr static_string?

When manipulating strings at compile time? Such as s1 += s2?

Wouldn't that use case be better served with an immutable string and operator+? At compile time, you can maintain the invariant length() == capacity(), which this type cannot do (and is not intended for).

In the same vein, more of it should be noexcept - why aren't empty() and size() ?

empty() and size() not being noexcept is more of an oversight on our part. As for why more of the member functions aren't noexcept, they have the same exception specifications as those of their counterparts in std::string. That being said, although they do call "dangerous" operations like Traits::length that are not noexcept, such functions don't throw (at least for std::char_traits), so its difficult to specify as noexcept since Traits could also be a user defined type.

You can add a specialisation for size 0 that turns it into an empty struct.

Certainly, it would be fairly trivial to do so.

I suggested changing this to the minimum size needed to store values 0 to N in the previous email discussion, but that hasn't been implemented.

Sure, that's extremely trivial to do, and seems like a great idea. On Tue, Nov 26, 2019 at 7:25 PM Peter Dimov via Boost wrote:

Gavin Lambert wrote:

...

On 27/11/2019 06:46, Peter Dimov wrote:

...

Vinnie Falco wrote:

...

In which use-cases do you anticipate the need for a constexpr static_string?

When manipulating strings at compile time? Such as s1 += s2?

Wouldn't that use case be better served with an immutable string and operator+?

Not in general.

constexpr string_view f( int x );

constexpr auto g( int n ) { fixed_string<512> s;

for( int i = 0; i < n; ++i ) { s += f( i ); }

return s; }

You can rewrite this specific example using recursion but in general, it'd be painful and unreadable.

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Vinnie Falco wrote:

On Tue, Nov 26, 2019 at 8:31 AM Phil Endecott via Boost wrote:

...

- Much more of it should be constexpr. Taking the opposite position, why should it be constexpr?

I think best practice is to declare everything constexpr unless it can't be. It's a bit like const was 20 years ago - does anyone else remember dealing with old code that didn't declare anything const?

...

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).

Your application seems to be for largish buffers; mine is for smallish strings. For the smallest strings a static_string can be smaller than a std::string, and for slightly larger strings the space overhead if the actual string is shorter than the capacity may be an acceptable trade-off for the improved performance. For example: struct NameAndAddress { static_string<14> firstname; static_string<14> surname; static_string<30> address_line_1; static_string<22> city; static_string<10> postcode; }; (Yes, yes, you probably wouldn't want to make those fields fixed-size in real code, but it's an example we can all understand.) That struct is trivially-copyable. Copying a version that used std::string would involve multiple memory allocations and could be an order of magnitude slower. Now: std::string a = s.firstname + " " + s.surname + ",\n" + s.address_line_1 + ",\n" + s.city + ",\n" + s.postal_code; That will create a temporary static_string<97> and copy from that into the std::string. But it needs a series of temporaries and probably uses a few hundred bytes of stack. Yes this isn't as good as we could get from a concat(...) function (or expression templates) that computed the size of the required buffer up-front and copied all of the arguments once, but it's still going to be an improvement over using std::string's operator+.

...

- You can add std::hash overloads.

I'm not seeing the value in these features. What's the use case? No one will be using static_string as the key for a container, since it wastes space.

using customer_code = static_string<11>; std::unordered_map customers; Vinnie, are you saying that this (i.e. small strings that are trivially- copyable) is not a supported use for the library? If you are, please say so and we can just reject it!

...

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.

Well again I'll contend that your purpose for the container is not the only one.

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.

If you want to get a string_view of a substring of a std::string, I think you can write something like std::string s = .....; std::string_view sv(s); // view of all of s std::string_view sub_sv = sv.substr(pos,count); // view of a subview of s Or in one line: auto sub_sv = std::string_view(s).substr(pos,count); Maybe std::string should have a method to do that more succinctly? Anyway, someone wanting to get a string_view of a substring of a fixed_string can do exactly the same thing.

...

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."

Vinnie, I have literally just said that I'd prefer this to be an improved static_vector and then an adaptor that adds all the string methods on top of static_vector<char>. I think that would be better because it gives us three useful things "for the price of one", i.e. a better static_vector, a static_string, and a "stringification" adaptor that can be used independently. So you're calling me "not normal" for suggesting that. I think that's rude. You've also been rude to Gonzalo Brito Gadeschi and the whole of WG21 in parts of your message that I've not quoted. Please don't do that. Unfortunately I don't have the time to respond to everything else. Regards, Phil.

On Wed, Nov 27, 2019 at 11:23 AM Vinnie Falco via Boost < boost@lists.boost.org> wrote:

On Wed, Nov 27, 2019 at 8:36 AM Phil Endecott via Boost

...

...it's still going to be an improvement over using std::string's operator+.

Okay, I agree that we need to do _something_. I'm not sure exactly what that something is. I'm hesitant to endorse operator+ because of the unpredictable behavior. How about a free function `concat(...)` which allows the caller to optionally specify the maximum capacity of the resulting string,

I was thinking the same thing. Zach

Vinnie Falco wrote:

How about a free function `concat(...)` which allows the caller to optionally specify the maximum capacity of the resulting string

See also http://open-std.org/JTC1/SC22/WG21/docs/papers/2019/p1228r1.html IMO, that paper unnecessarily combines string concatenation with number-formatting, but it is still interesting to look at. It converts all of its arguments to string_views, sums the lengths (at run time), allocates a std::string of that length, and copies the arguments into it. You could extend that to (a) permit the return type to be specified as an additional template parameter, and (b) default that return type to a static_string if the capacities of all the arguments are finite. Regards, Phil.

Em qua., 27 de nov. de 2019 às 13:36, Phil Endecott via Boost < boost@lists.boost.org> escreveu:

struct NameAndAddress { static_string<14> firstname; static_string<14> surname; static_string<30> address_line_1; static_string<22> city; static_string<10> postcode; };

I was going to write a review, but I think I can borrow your example to show the single point I'm concerned about. char[N] has alignment of char. std::size_t has not and structs like this will consume much more memory because this single member: https://github.com/18/fixed_string/blob/9730545e0fa37b43299559fca8253c434c67... I'm concerned about this alignment and I'd rather have the size being computed by searching for the NULL-terminator (or also stored in a byte). Maybe a policy template parameter could be used to select the implementation. I don't really care about which solution is chosen as long as I can opt-in for a struct which has the same alignment as char (and it is trivially copyable). -- Vinícius dos Santos Oliveira https://vinipsmaker.github.io/

Vin?cius dos Santos Oliveira wrote:

Em qua., 27 de nov. de 2019 ?s 13:36, Phil Endecott via Boost < boost@lists.boost.org> escreveu:

...

struct NameAndAddress { static_string<14> firstname; static_string<14> surname; static_string<30> address_line_1; static_string<22> city; static_string<10> postcode; };

I was going to write a review, but I think I can borrow your example to show the single point I'm concerned about.

char[N] has alignment of char. std::size_t has not and structs like this will consume much more memory because this single member

This is avoided if you use an appropriate smaller type for the size, i.e. uint8_t when capacity <= 255.

I'd rather have the size being computed by searching for the NULL-terminator

Yes a string that does this can also be useful, particularly for the very smallest strings, but I would not suggest trying to combine it with this implementation. Regards, Phil.

wt., 26 lis 2019 o 18:34 Vinnie Falco via Boost napisał(a):

On Tue, Nov 26, 2019 at 8:31 AM Phil Endecott via Boost wrote:

...

I would much prefer static_string

Same here... I was foolish to rename it.

...

- Much more of it should be constexpr.

I agree that it could be constexpr (at least some of it) but this can be added as an improvement later.

Taking the opposite position, why should it be constexpr? The purpose of this container is to serve as a performant substitute for std::string when the maximum size is known ahead of time or can be reasonably estimated. In which use-cases do you anticipate the need for a constexpr static_string? It would certainly waste space, unless the caller sizes it perfectly. But then what's the difference between that and a constexpr string literal or string view?

In my opinion people are too quick to say "constexpr it" without evaluating the necessity. I'm not saying we shouldn't do it (but see below), however lets not pretend that there are legions of users eager for a constexpr fixed capacity mutable string.

...

There is also a question of whether all of the elements must be initialised (to 0) by the ctor to be constexpr, which could be costly for a large but sparse container, though

I think initializing everything to 0 is a non-starter for the non-constexpr case, due to the performance penalty. And initializing to 0 only for constexpr requires new language features I believe? std::is_constexpr_evaluated or some such.

...

it does mean that subsequent push_back()s don't need to store terminators;

It would still need to null-terminate, for the case where characters are removed from the string and subsequently reinserted.

...

- You can add a specialisation for size 0 that turns it into an empty struct.

- You can add a string literal operator"".

- You can add std::hash overloads.

I'm not seeing the value in these features. What's the use case? No one will be using static_string as the key for a container, since it wastes space. Why do we need a string literal operator, to produce a string constant that takes up more space than necessary? And why are we "optimizing" the case where size==0? Is this something that anyone will notice or care about (aside from WG21 of course, which often concerns itself with not-relevant things).

...

There are various possibilities for what should happen if the capacity is exceeded, including throwing an exception, asserting, and allowing undefined behaviour. I'm not sure what I would really prefer though I think I'd be more likely to assert() than to throw. See P0843r2 again which discusses this.

Quoting some nonsense from p0843r2:

1. Throw an exception. 2. Make this a precondition violation. ... It could throw either std::out_of_bounds or std::logic_error but in any case the interface does not end up being equal to that of std::vector. ... The alternative is to make not exceeding the capacity a precondition on the static_vector's methods.

So basically Gonzalo (the paper's author) says that throwing an exception of the wrong type makes the interface different from std::vector, thus the solution is to make the interface even more different from vector by requiring callers to check sizes. Gonzalo lists existing practice of Boost.Container's static_vector, with throws on overflow, as existing practice. Only in the bizarro world of WG21 could someone write a paper which quotes existing practice, then proceed to deviate from existing practice for illogical reasons.

We can return to sanity by recognizing what the purpose of static_string is for:

"...a performant substitute for std::string when the maximum size is known ahead of time or can be reasonably estimated."

Throwing an exception is the obvious rational choice. It lets static_string be mostly a drop-in replacement, without requiring call sites to add extensive code to check for preconditions. It guarantees that any successful modifications to the static_string will be exact (no truncation). And most importantly it matches existing practice (e.g. boost::container::static_vector and others like it).

Actually, boost::container::static_vector defines the container beyond its capacity as a precondition: https://www.boost.org/doc/libs/1_71_0/doc/html/boost/container/static_vector... And this does not require of a user any extensive "checking" of preconditions. Te user simply knows her program's flow, and already knows the preconditions are not exceeded. For instance, the logic of the application dictates that strings are never resized, or for every name initially created only zero or one push_back()s are performed with a single character. Regards, &rzej;

...

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).

...

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.

...

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.

Regards

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

On Mon, Dec 2, 2019 at 3:07 AM Andrzej Krzemienski via Boost < boost@lists.boost.org> wrote:

wt., 26 lis 2019 o 18:34 Vinnie Falco via Boost napisał(a):

...

On Tue, Nov 26, 2019 at 8:31 AM Phil Endecott via Boost wrote:

...

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.

Even if this is a valid design choice when considered in separation, it is clearly in conflict with the goal of being a drop-in replacement for std::string. It looks like the library is aiming at being a drop-in replacement for some subset of usages of std::string. It would be beneficial to outline this subset clearly in the initial sections of the documentation.

+1. FWIW, I find "this is a drop-in replacement" to be a typically unattainable goal for anything that is not based on some kind of polymorphism (compile- or runtime). That is, the polymorphic drop-in mechanism only cares about a subset of API and/or behavior. Zach

On Mon, Dec 2, 2019 at 12:13 PM Krystian Stasiowski via Boost < boost@lists.boost.org> wrote:

On Mon, Dec 2, 2019 at 11:57 AM Zach Laine via Boost < boost@lists.boost.org> wrote:

...

+1. FWIW, I find "this is a drop-in replacement" to be a typically unattainable goal for anything that is not based on some kind of polymorphism (compile- or runtime). That is, the polymorphic drop-in mechanism only cares about a subset of API and/or behavior.

Perhaps we provide two separate functions like was mentioned previously, where one returns a fixed_string and the other returns a string_view.

Perhaps. I think you were on the right track changing substr() to return a string_view, just not to worry too much about being a drop-in replacement. I think you should only worry about API compatibility where it makes sense. Zach

On Mon, Dec 2, 2019 at 1:57 PM Peter Dimov via Boost wrote:

Krystian Stasiowski wrote:

...

Perhaps we provide two separate functions like was mentioned previously, where one returns a fixed_string and the other returns a string_view.

I think that this is almost certainly the right thing to do, both for fixed_string and for std::string. Call the string_view returning function `s.subview()` or just `s.view()`, leave `s.substr()` to return by value.

I like the ring of subview