Re: [ Review ] [ fixed_string ] Accept

From: Krystian Stasiowski (sdkrystian_at_[hidden])
Date: 2019-12-05 07:09:27

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_at_[hidden]> wrote:

> -1 - Naming.
>      fixed_string<500> works, wfixed_string<500> doesn't,
> basic_fixed_string<char, 5000> 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.