Hi, Thank you for the review! Here are a few comments: On Wed, Dec 4, 2019 at 8:27 PM JeanHeyd Meneide via Boost < boost@lists.boost.org> wrote:
-1 - Naming. fixed_string<500> works, wfixed_string<500> doesn't, basic_fixed_string
doesn't either. It uses a different naming scheme from the standard, and led me around in circles on first use until I forced myself to read the template head of the class declaration. But it's Boost, so we can afford to be different and stuff.
The name will be changed basic_foo_bar, and aliases will be added for foo_bar/wfoo_bar etc. As for what foo_bar will actually be is TBD. -0 - operator+
I wanted it. It wasn't there. I can see why, since you would have to pessimistically expand the size to the max between the two. Do enough additions and you can explode the final result's stack size several, so this isn't really a loss. Besides,-0 is just 0 on any reasonable 2's complement machine.
It will be implemented. I'm leaning towards just making the capacity be the sum of the capacities of the operands (for fixed_string and array of CharT). As for the other overloads (CharT*, string_view, etc) these operations will either remain deleted, or just result in the type of the fixed_string operand and throw if the capacity is exceeded.
-0 - noexcept It's 2019, everyone's getting on the noexcept train. Don't let it leave you behind!
I agree. For the most part, this has been taken care of on the live branch (save for overloads that convert to string_view_type, but that will be done very soon) -2500 - Constexpr is lacking.
I could not compile any of itsy.bitsy's constexpr, compile-time, or static_assert tests. I could not compile any of the static_assert tests from latest private text development. This prevents a wide range of applications, including my failure to substitute boost.fixed_string for Hana's Fixed String implementation in CTRE and failure to use fixed_string as a compile-time buffer for encoded and decoded text.
I agree, we should have constexpr, and that currently is my top priority. However, there is the problem that pre C++20 constexpr requires initialization of all members, which is undesirable. Your suggestion to use a union with a dummy value unfortunately does not work for C++11 and 14, since that would result in access to non-existent array elements. In C++17, this is no longer the case, as when the left operand of an assignment expression nominates a union member, it will begin the lifetime of the member (in certain cases, there are a few restrictions on the types that this can be done for). However, for C++11 and 14, there currently exists no solution other than just zero-initializing the array.