[review][fixed_string] FixedString review starts Nov 25
The Boost formal review of Krystian Stasiowski and Vinnie Falco's FixedString library will take place November 25 - December 4, 2019. This is an early call for you to consider participating in the review. Feedback and constructive criciticism from experts and not-so-experts alike is very much welcome. FixedString being a relatively small and straightforward library, you will find it very easy to familiarize yourself with it and contribute back to the community. FixedString provides a dynamically resizable string of characters with compile-time fixed capacity and contiguous embedded storage in which the characters are placed within the string object itself. Its API closely resembles that of std::string. GitHub repository: https://github.com/18/fixed_string Online docs: https://18.github.io/doc/fixed_string If you feel like reviewing FixedString you can start now prepping --though I'd ask you to refrain from submitting actual reviews till Nov 25. Read the docs, download/fork the code, read/run it, maybe try integrating it into some project of yours, ask questions to the authors, preferrably on Boost or Boost Users mailing lists. The FixedString initiative arose from a very lively discussion within the community: https://lists.boost.org/Archives/boost/2019/09/246943.php , so I sincerely hope this review will be a fruitful, crowded one. Don't let me down! Best regards, Joaquín M López Muñoz Review manager for candidate Boost.FixedString
Joaquin M López Muñoz wrote:
GitHub repository: https://github.com/18/fixed_string Online docs: https://18.github.io/doc/fixed_string
Could a single header version be temporarily provided for the review, so that we can play with it on Godbolt? The current header arrangement makes that impossible. (Actually, a single header for this specific library doesn't seem a bad idea in principle - the separation appears to provide no value in this case.) Also, a synopsis section in the documentation might have been nice, to allow one to see all the declarations in one place. (Admittedly, the main header does serve that purpose somewhat.) What is the C++ standard requirement of this library?
This is my review of the proposed fixed_string library. Notes: - The list of member functions should include a full declaration so I know what's what. Other than that it does the job, though it would be nice to have a hierarchical TOC on the side for reference. - Add constexpr and noexcept where appropriate. - There was a discussion on whether or not resize should call boost::throw_exception. It should (as it does) because A) buffer overruns are gnarly and B) the overhead of the check is negligible, considering this is supposed to be a universal library. - Operator+ should be provided. I'm not sure whether it should use capacity N+M or something else, but all technical questions notwithstanding, users ought to be able to concatenate strings easily, e.g. if I want to add a "ms" to a number I shouldn't be jumping through hoops. - It's nice that the authors are concerned about physical coupling, but there are good reasons to provide (in addition) a single-header version (e.g. Godbolt). My vote is to accept.
On 5/12/2019 11:32, Emil Dotchevskit wrote:
- Operator+ should be provided. I'm not sure whether it should use capacity N+M or something else, but all technical questions notwithstanding, users ought to be able to concatenate strings easily, e.g. if I want to add a "ms" to a number I shouldn't be jumping through hoops.
Is there some reason why you consider operator+= insufficient for that purpose? Perhaps this needs to be stated more clearly in the docs, but the impression I get is that the primary intended usage is something like: fixed_string<512> buffer; // some arbitrarily large N buffer += prefix; buffer += suffix; consume(buffer);
On Wed, Dec 4, 2019 at 2:45 PM Gavin Lambert via Boost < boost@lists.boost.org> wrote:
On 5/12/2019 11:32, Emil Dotchevskit wrote:
- Operator+ should be provided. I'm not sure whether it should use capacity N+M or something else, but all technical questions notwithstanding, users ought to be able to concatenate strings
easily, e.g.
if I want to add a "ms" to a number I shouldn't be jumping through
hoops.
Is there some reason why you consider operator+= insufficient for that purpose?
There are two use cases: 1. The user knows he is working with fixed_string, and he chooses to call op+. There's no reason to take away this choice. 2. The user has a generic function that concatenates "strings". We need good reasons if we want to force him to rewrite the function. It is possible that there could be some problem with op+, but at this point it seems mostly theoretical. I presume the second case is rather rare.
śr., 4 gru 2019 o 23:33 Emil Dotchevski via Boost <boost@lists.boost.org> napisał(a):
This is my review of the proposed fixed_string library.
- There was a discussion on whether or not resize should call boost::throw_exception. It should (as it does) because A) buffer overruns are gnarly and B) the overhead of the check is negligible, considering this is supposed to be a universal library.
- Operator+ should be provided. I'm not sure whether it should use capacity N+M or something else, but all technical questions notwithstanding, users ought to be able to concatenate strings easily, e.g. if I want to add a "ms" to a number I shouldn't be jumping through hoops.
One possibility here, assuming that * concatenating fixed_string<N> with std::string is out of the question and * fixed_string<N> is a drop-in replacement for std::string and * resizing over capacity is treated as a correct (albeit rare) situation in the program is to simply provide `fixed_string<N> operator+(fixed_string<N>, fixed_string<N>)`. This will be useful when some part of the program uses all `fixed_string`s with the same capacity anyway. If fixed_string is a drop-in replacement for string one would not be surprised with this signature. For std::string, one is not surprised if string a was able to store its contents, and b was able to store its contents, but there was no room to store the concatenated version of a and b and this resulted in an exception. Regards, &rzej;
participants (5)
-
Andrzej Krzemienski -
Emil Dotchevski -
Gavin Lambert -
Joaquin M López Muñoz -
Peter Dimov