6
\$\begingroup\$

I have implemented a function to concatenate string literals at compile time.

Basic requirements:

  1. Very simple API. Can be used as a one-liner. Must accept variadic string literals.
  2. Highly optimized instruction set, no runtime-overhead. Basically concat("A ", "quick ", "brown ", "fox"); should not differ (much) from writing a single string literal by hand.
  3. The function must accept string literals, static strings (for nested calls) and dynamic strings. The resulting type must be deduced from the arguments at compile time. So that N string literals would result in a single contiguous string literal with no dynamic allocations. But if at least one argument is a dynamic string, the function would yield a dynamic string with one allocation at most.
  4. C++17. I am sure the code can be implemented to support C++11, but personally I don't have a need for that right now.

Implementation:

Unfortunately, one can't return raw arrays, so a custom type was introduced.

  1. The whole concatenation of variadic literals was facilitated by std::tuple_cat's ability to accept std::array.
  2. The string itself is to be accessed via static_cast. Customization point _cast exists to allow user-defined specialization for static_cast from ct_string. This is kind of an experimental approach. One is free to assume that ct_string is a contiguous array of Char of size N. But I don't want to make std::array as a part of specification, since it is mostly used for std::tuple_cat.
  3. I added ct_string specializations for tuple traits, since it is static.
  4. I am not sure if I need or am expected to implement iterator functions for the class.

The Code:

Gist: https://gist.github.com/sergio-eld/e903adfa2f3a157a147cb2d8f83f18e5

/**
* Standard: C++17
* Constexpr string literals concatenation function.
* Accepts both string literals and dynamic strings.
* When all the arguments are static will yield a ct_string object evaluated at compile time.
* If dynamic strings present, will yield an std::string object using single allocation.
* 
* Return type is deduced from variadic arguments.
*
* Usage:
// Example:
// Godbolt: https://godbolt.org/z/rTnnbnozc

#include <iostream>

int main(){
    // static string
    std::cout << eld::concat("The ", "quick ", "brown ", "fox ", "jumps ", "over ", "the ", "lazy ", "dog") << '\n';
    
    // dynamic string
    std::cout << eld::concat("The ", std::string("quick "), "fat ", std::string("frog")) << '\n';
    
    using eld::separator;

    // with separator
    std::cout << eld::concat(separator{", "}, "John Cena") << '\n';
    std::cout << eld::concat(separator{", "}, "The", "quick", "brown", "fox", "jumps", "over", "the", "lazy", "dog") << '\n';
}
*
* TODO: 
* - char arguments support
* - deduction traits for static strings
* - (?) non-string arguments with to_string conversion
*/

#include <cstddef>
#include <utility>
#include <type_traits>
#include <tuple>
#include <array>
#include <string_view>
#include <string>
#include <functional>
#include <numeric>

namespace eld {
    template <size_t N, typename Char = char>
    class ct_string;
}

namespace std {
    template <size_t N>
    struct tuple_size<eld::ct_string<N>> : std::integral_constant<size_t, N> {};

    template <size_t I, size_t N, typename Char>
    struct tuple_element<I, eld::ct_string<N, Char>>
    {
        using type = Char;
    };

    template <size_t I, size_t N, typename Char>
    constexpr auto get(const eld::ct_string<N, Char> &str) noexcept -> Char {
        return str.template get<I>();
    }

    template <typename Char, size_t ... M>
    constexpr auto tuple_cat(const eld::ct_string<M, Char> &... strs) noexcept;
}

namespace eld {

    template <size_t N, typename Char>
    class ct_string 
    {
    public:
        using value_type = Char;

        template <size_t M>
        constexpr ct_string(const value_type (&str)[M])
            : ct_string(str, std::make_index_sequence<M - 1>())
        {}

        constexpr ct_string(std::array<Char, N> arr) noexcept 
            : _value(arr)
        {}

        constexpr ct_string(const ct_string&) noexcept = default;
        constexpr ct_string(ct_string&&) noexcept = delete;

        ct_string& operator=(const ct_string&) = delete;
        ct_string& operator=(ct_string&&) = delete;

        constexpr auto value() const -> std::basic_string_view<Char> {
            return {&_value[0], N};
        }

        constexpr size_t size() const noexcept {
            return _value.size();
        }

        template <size_t I>
        constexpr value_type get() const noexcept {
            static_assert(I < N, "Index is out of bounds");
            return _value[I];
        }

        template <typename C, size_t ... M>
        constexpr static auto cat(ct_string<M, C> ... strs) noexcept -> ct_string<(M + ...), C>{
                    return std::apply(
                [] (auto... cs) -> ct_string<(M + ...), Char> { return std::array{ cs... }; },
                std::tuple_cat(std::tuple_cat(strs._value)...));
        }

        template <typename To>
        constexpr explicit operator To() const noexcept{
            return _cast<To>{}(std::as_const(this->_value));
        }

        friend std::basic_ostream<Char>& operator<<(std::basic_ostream<Char> &os, const ct_string &str){
            os << static_cast<std::string_view>(str);
            return os;
        }

    private:

        // clang requires this!
        template <size_t M, typename C>
        friend class ct_string;

        template <size_t M, size_t ... I>
        constexpr ct_string(const value_type (&str)[M], std::index_sequence<I...>) noexcept
            : _value({str[I]...})
        {}

        // customization point for casting array
        template <typename T, typename...>
        struct _cast
        {
            // would be better to pass as C-array reference instead, not to expose std:array
            constexpr T operator()(const std::array<value_type, N>&) const noexcept;
        };
        
    public:
        /// Internal data representation, not to be accessed or relied upon as a part of specification
        /// It HAS to be public due to C++20 requirement for an object to be used as a Non-Type Template Parameter
        const std::array<value_type, N> _value;
    };

    template <size_t N, typename Char>
    ct_string(const Char (&str)[N]) -> ct_string<N - 1, Char>;

    template <typename Char, size_t N>
    ct_string(std::array<Char, N>) -> ct_string<N, Char>;

    template <size_t N, typename Char>
    template <typename ... Void>
    struct ct_string<N, Char>::_cast<std::basic_string_view<Char>, Void...>{
        constexpr auto operator()(const std::array<Char, N> &arr) const noexcept
            -> std::basic_string_view<Char>{
            return {arr.data(), arr.size()};
        }
    };

    template <size_t N, typename Char>
    template <typename ... Void>
    struct ct_string<N, Char>::_cast<std::basic_string<Char>, Void...>{
        constexpr auto operator()(const std::array<Char, N> &arr) const noexcept
            -> std::basic_string_view<Char>{
            return {arr.data(), arr.size()};
        }
    };

    namespace traits{
        // TODO: deduction rules instead
        template <typename String>
        struct is_static_string : std::false_type {};

        template <size_t N, typename Char>
        struct is_static_string<ct_string<N, Char>> : std::true_type{};

        template <typename Char, size_t N>
        struct is_static_string<Char[N]> : std::true_type{};

        template <typename Char, size_t N>
        struct is_static_string<Char(&)[N]> : is_static_string<Char[N]>{};

        template <typename Char, size_t N>
        struct is_static_string<const Char(&)[N]> : is_static_string<Char[N]>{}; 
    }

    namespace detail {
        class _concat
        {
        private:
            using _static_string = std::true_type;
            using _dynamic_string = std::false_type;

            template <typename ... Strs>
            using _string_type = typename std::conjunction<traits::is_static_string<Strs>...>::type;

        public:
            template <typename ... Strings>
            constexpr auto operator()(const Strings &...strs) const noexcept {            
                return _impl(_string_type<Strings...>{}, strs...);
            }
        private:
            template <typename ... Strings>
            constexpr static auto _impl(_static_string, const Strings &... strs) noexcept {
                return std::tuple_cat(ct_string(strs)...);
            }

            // can it be made constexrp?
            template <typename ... Strings>
            static auto _impl(_dynamic_string, const Strings &... strs) noexcept {
                auto dynamicString = std::string();
                dynamicString.reserve((std::size(strs) + ...));
                (dynamicString.append(static_cast<std::string_view>(strs)), ...);
                return dynamicString;
            }
        };
    }

    template <typename T>
    struct separator
    {
        T value;
    };

    template <typename T>
    separator(T) -> separator<T>;

    template <typename Char, size_t N>
    separator(const Char(&literal)[N]) -> separator<decltype(ct_string(literal))>;

    template <typename FirstString, typename ... Strings>
    constexpr auto concat(const FirstString& first, const Strings& ... strings){
        return detail::_concat{}(first, strings...);
    }

    template <typename T, typename FirstString, typename ... Strings>
    constexpr auto concat(separator<T> sep, const FirstString &fs, const Strings &... strs) noexcept{
        const auto tupleStrings = std::tuple_cat(std::tie(fs), std::tie(sep.value, strs)...);
        return std::apply([](auto&& ... strs){ return concat(std::forward<decltype(strs)>(strs)...);}, tupleStrings);
    }
}

namespace std {
    template <typename Char, size_t ... M>
    constexpr auto tuple_cat(const eld::ct_string<M, Char> &... strs) noexcept{
        return eld::ct_string<0, Char>::cat(strs...);
    }
}

Example: https://godbolt.org/z/rTnnbnozc

#include <iostream>

int main(){
    // static string
    std::cout << eld::concat("The ", "quick ", "brown ", "fox ", "jumps ", "over ", "the ", "lazy ", "dog") << '\n';
    
    // dynamic string
    std::cout << eld::concat("The ", std::string("quick "), "fat ", std::string("frog")) << '\n';
    
    using eld::separator;

    // with separator
    std::cout << eld::concat(separator{", "}, "John Cena") << '\n';
    std::cout << eld::concat(separator{", "}, "The", "quick", "brown", "fox", "jumps", "over", "the", "lazy", "dog") << '\n';
}

I'd be happy to recieve any feedback.

\$\endgroup\$
4
  • \$\begingroup\$ I just looked at your code... Wow... This is a nice solution to the problem. I have no comments and would just give a LGTM. I would replace ct_string with std::string as end result; but that would violate requirement 3; so well done. \$\endgroup\$
    – rioki
    Commented Jan 11, 2023 at 14:27
  • \$\begingroup\$ @rioki thank you! I would rather have a raw string literal as a return type, but that is impossible. Essencially, one should have char [N + M] from char [N] + char [M]. Even if std::string had a constexpr constructor, it's size() would not be possible to use within a constexpr context. \$\endgroup\$ Commented Jan 11, 2023 at 14:45
  • \$\begingroup\$ Yes. I would have probably defined different requirements. I was thinking about something similar to the ""s literal. But with your requirements this is the best option. String literal from constexpr would be a really nifty compiler feature. \$\endgroup\$
    – rioki
    Commented Jan 12, 2023 at 9:59
  • \$\begingroup\$ Micro-review: size_t isn't necessarily (portably) defined - <cstddef> defines std::size_t, but is not required to also define its global-namespace equivalent. \$\endgroup\$ Commented Feb 12, 2023 at 7:59

1 Answer 1

4
\$\begingroup\$

Design review

Cool. The idea is good; it’s something I could see myself using. The implementation is sound. In general, cool.

I like your requirements, except I would strongly recommend moving to C++20 for a number of reasons. The step from C++17 to C++20 is the biggest leap forward in C++ since the step from C++98/03 to C++11. Once C++20 becomes “the norm”, nobody’s going to want to be stuck writing C++17 anymore.

And there are a lot of features C++20 brings that directly impact what you’re trying to do. Like… a lot. Far more than I could list, though I’ll mention some as we go along.

If you do want to stick with C++17, that’s fine, but there will be some restrictions and caveats.

The biggest issue with your design, and the concept in general, is the confusion that comes with the term “static string”. What… exactly… do you think that means? Do you mean a compile-time string? Because that’s what it sounds like you mean. Or do you mean a string that exists in static storage? Because that’s something completely different.

I would advise tightening up your terminology. If you’re going to use “static”, you need to explicitly specify what you mean by it. “Literal”? That’s probably not what you mean: a string literal is a specific syntactic construct in C++, and an array of characters is not a string literal (though a string literal can be converted to an array of characters). “Compile-time”? Maybe. “Constant”? Meh. Really, it’s less important which term you use, than it is to clearly specify what you mean.

The main thing I would suggest, from an overall design perspective, is that your focus should be on the static/compile-time/whatever string type… not the concatenation function. When I teach C++, one of my mantras is “if you get the types right, everything else Just Works”. That’s very true here, too. That ct_string type… that is the real magic underlying your code. Everything works because of that type. But because you have focused on the concatenation and not the type, things are little clunky in places.

For example, let’s say I do: constexpr auto s = eld::concat("a", "b", "c");. Now what? What can I do with s? Well, I can stream it. I can get its size. I can convert it to string_view, using the non-standard .value(). But that’s about it.

Wouldn’t it be cool if I could do stuff like:

constexpr auto s = eld::concat("a", "b", "c");
constexpr auto t = eld::concat("d", "e", "f");

constexpr auto u = s + t;   // u is ct_string{"abcdef"};

constexpr auto c = 'x';
if (std::ranges::find(s, c) != std::ranges::end(s))
    std::cout << s << " contains " << c;

// If I want a path:
auto const path = std::filesystem::path{"/path/to"} / s;

// ... and so on...

All of the ergonomics above would come from the ct_string class.

But okay, let’s say that the ct_string class isn’t the focus here; the concatenation is. Even so, I would suggest a rethink of the concatenation in terms of ct_string. Why? Because:

// Assuming:
constexpr auto operator""_static_string() -> ct_string; // Probably not possible in C++17.

constexpr auto f() -> ct_string
{
    return eld::concat("b");
}

constexpr auto sv = std::string_view{"a"};

// All of the following statements should be identical:
constexpr auto s = eld::concat("a", "b", "c");
constexpr auto s = "ab"_ct_string + "c";
constexpr auto s = "a" + f() + "c";
constexpr auto s = eld::concat("a", "b") + eld::concat("c");
constexpr auto s = eld::concat(eld::concat("a"), eld::concat("b"), eld::concat("c"));

constexpr auto s = sv + eld::ct_string{"bc"};
// This last one is tricky, and may not be possible. The problem is that even
// if a string view is a compile-time string view, you need to get the size
// into the type system.

(There is no practical difference between doing s = concat(a, b, c, d) and s = a + b + c + d at compile-time, but it makes sense to the former at run-time, because there are no intermediary strings created; you can collect all the sizes and allocate the result once. But I find s = a + b + c + d nicer to read, and if you’re doing constexpr auto s = ..., then you know it’s all compile-time, so I’d prefer the nicer format. For cases that could be either compile- or run-time, okay, sure, the function style makes sense.)

I do have to point out that this whole venture is technically not even possible in C++17. At least, it’s probably not possible to do it correctly. Observe:

auto p = std::make_unique<std::array<char, 5>>();

// ... or read from file, or whatever...
(*p)[0] = 'f';
(*p)[1] = 'a';
(*p)[2] = 'k';
(*p)[3] = 'e';
(*p)[4] = '\0';

auto so_called_static_string = eld::ct_string{*p};

// This concatenation can't happen at compile time! It *does* yield a
// ct_string object, as your documentation promises... but not one
// evaluated at compile-time. You could end up with multiple, recursive
// calls to `tuple_cat` creating and copying arrays all happening at
// run-time.
auto s = eld::concat(
    eld::separator{" "},
    "The",
    "quick",
    "brown",
    so_called_static_string);

In C++20, this won’t be a problem because you can simply make the static string constructor(s) consteval. That’s just one of numerous problems that can be solved by ditching C++17, so, again—and, I promise, for the last time (not counting the summary)—I really, really strongly suggest you should do so, and move to C++20.

Alright, on to the code review:

Code review

    template <size_t N, typename Char = char>
    class ct_string;

I’m not a fan of this interface. Firstly, I would probably include traits. But even if not, I would put the character type first. So:

template <typename Char, typename Traits, std::size_t>
class ct_string;

In my mind, the size is the least important part of the type, since it’s going to be deduced or calculated 99% of the time. And ordering the parameters like this brings it in line with the standard string types:

template <typename Char, typename Traits, std::size_t       > class ct_string;
template <typename Char, typename Traits, typename Allocator> class std::basic_string;
template <typename Char, typename Traits                    > class std::basic_string_view;

Which means you don’t need to remember yet another exception to the “rules” of C++, which is mercifully just one bit less cognitive load you have to carry.

Just about the only “downside” of ordering the parameters this way is you can’t sensibly default the size. I mean… you could default it to zero or -1 and it won’t be “wrong”… but it wouldn’t be logical. But in reality, when are you ever going to want to specify the size anyway? It’s not like you’re going to be writing ct_string<4> or ct_string<5> as a matter of course. And you’re certainly never going to write auto s = ct_string<>{}. You’re always going to want the size to be deduced. So who cares if it has no default?

(There is also another argument, one that manifests later when you write functions like cat() and have to reverse the arguments: template <typename C, std::size_t... M>. There will (probably) never be a situation where you are dealing with multiple character types at the same time… but there are (as you have found) good reasons to have multiple sizes in one operation. It is traditional in C++ (for practical reasons), when dealing with something that has both a fixed set of arguments and a variable number of arguments, to have the variable set last. That implies the size should be last.)

Incidentally, it’s std::size_t. The std:: is not optional. Yes, virtually all compilers/standard libraries include size_t in the global namespace for historical reasons (C compatibility). It’s still not the correct way.

namespace std {
    template <size_t N>
    struct tuple_size<eld::ct_string<N>> : std::integral_constant<size_t, N> {};

These days, I think the expert consensus is that opening up the std namespace is a code smell. The preferred way to do this would be:

template <std::size_t N>
struct std::tuple_size<eld::ct_string<N>> : std::integral_constant<std::size_t, N> {};

Note, though, that you have only specialized tuple_size for Char as the default char. ct_string<N, char8_t> (or any other character type) does not match this specialization.

The specialization of tuple_element is fine, but get() and tuple_cat() are problematic.

get() does not correctly handle the reference category, but frankly, there really is no sense specializing it in any case. The tuple/structured binding interface works just fine with get() as a member function. If you want get() to work as an ADL free function, okay, fine, you could do that, too. But if you want std::get() to work for your type… that’s weird because std::get() isn’t a customization point, and, anyway, what you have isn’t correct: aside from the reference category issue, you don’t have support for std::get<T>().

As for std::tuple_cat()… also not a customization point, and, worse, you have changed its behaviour in a surprising way. This is what the documentation for std::tuple_cat() says:

Returns: tuple<CTypes...>(celems...).

See? std::tuple_cat() explicitly returns a std::tuple. You have hijacked the function, and changed the return type. This will be surprising and infuriating, because a standard pattern for converting anything to a tuple is std::tuple_cat(thing). If that thing happens to be a ct_string, the pattern is broken.

Bottom line: don’t specialize std::get() or std::tuple_cat(). Neither are customization points, and anyway, your specializations break the specification’s promises for those functions.

On to the ct_string class.

        template <size_t M>
        constexpr ct_string(const value_type (&str)[M])
            : ct_string(str, std::make_index_sequence<M - 1>())
        {}

I’m not sure why the index sequence is necessary here. I’m guessing it’s because you’ve written yourself into a corner by making _value const… which is a problem I’ll be mentioning later. It should be possible to just copy the array’s contents into _value.

Now, I get that the purpose of this constructor is to convert C-strings—NUL-terminated char arrays. Which means you want "abc", which is char[4] to convert to ct_string<3, char>. But without any constraints, this is a catastrophe waiting to happen.

Observe:

char uh_oh[100]{};
auto boom = eld::ct_string<3, char>{uh_oh};

What you want is three things:

  1. You want to constraint this constructor so that M is less-than-or-equal-to N + 1.
  2. If M is less than N + 1, you want to pad out _value with Char{}s.
  3. If M equals N + 1, you want to confirm that str[N] is Char{}.
        constexpr ct_string(std::array<Char, N> arr) noexcept 
            : _value(arr)
        {}

Is this constructor really necessary? And, more importantly, is it really necessary for it to be part of the public interface? (You’ll note that it’s the constructor I exploited above.)

        constexpr ct_string(const ct_string&) noexcept = default;
        constexpr ct_string(ct_string&&) noexcept = delete;

        ct_string& operator=(const ct_string&) = delete;
        ct_string& operator=(ct_string&&) = delete;

Is it really necessary delete the move ops, and copy assignment? What, exactly, about the type does it improve? Does it make it safer? I can’t see how. It just seems to make it more frustrating to use for no real benefit.

        constexpr auto value() const -> std::basic_string_view<Char> {
            return {&_value[0], N};
        }

_value.data() is less cryptic than &_value[0]. But there’s a more fundamental issue that I might as well bring up here: what if N is zero?

Now, you can’t have a zero-sized array, but without checking for that, you’re relying on the graces of the compiler to actually error-out, rather than allowing non-standard extensions or whatnot. You’re also hoping that the error messages generated make sense, even though they will be referring to hidden implementation details of the class.

You can, though, have a zero-sized string. And that’s where you have problems. Personally, I would make the internal array size N + 1, and add a NUL. This not only solves the zero-sized string problem (because the array size will still be 1), it makes conversion to a C-string trivial. Which can be handy.

Finally, there’s no conceivable way this function can fail, so it should probably be marked noexcept. Speaking of which:

        template <size_t I>
        constexpr value_type get() const noexcept {
            static_assert(I < N, "Index is out of bounds");
            return _value[I];
        }

value() can’t conceivably fail but isn’t marked noexceptget<I>() can conceivably fail, yet is not marked noexcept. I mean, get<I>() can’t fail if it’s constrained to be [0, N), which it is via the static_assert, so there’s no real problem here. I just wanted to point out the way of thinking, as part of the interface design process.

The real problem with get() is that it doesn’t respect the reference category. If *this is a non-const lvalue, it should return a non-const lvalue reference… and so on. (Yes, I know your current implementation can’t return a non-const reference… that’s another problem, which we’ll get to.)

The proper way to implement get() is to quadruplicate it (and, if you care about volatile octuplicate it… but no one cares about volatile):

public:
    template <std::size_t I> constexpr auto get() &       noexcept -> Char&         { return _get<I>(*this); }
    template <std::size_t I> constexpr auto get() const&  noexcept -> Char const&   { return _get<I>(*this); }
    template <std::size_t I> constexpr auto get() &&      noexcept -> Char&&        { return _get<I>(std::move(*this)); }
    template <std::size_t I> constexpr auto get() const&& noexcept -> Char const&&  { return _get<I>(std::move(*this)); }

private:
    template <std::size_t I, typename S>
    static constexpr auto _get(S&& s) noexcept -> decltype(auto)
    {
        static_assert(I < N, "Index is out of bounds");
        return forward<S>(s)._value[I];
    }

(C++23 makes this much easier with deducing this.)

        template <typename C, size_t ... M>
        constexpr static auto cat(ct_string<M, C> ... strs) noexcept -> ct_string<(M + ...), C>{
                    return std::apply(
                [] (auto... cs) -> ct_string<(M + ...), Char> { return std::array{ cs... }; },
                std::tuple_cat(std::tuple_cat(strs._value)...));
        }

There are quite a few issues here.

First, as a matter of style, I’m not a fan of the extra space in size_t ... M. That makes the ellipsis look like a binary operator, which makes it look like a fold expression. The almost universal convention is to put the ellipsis with the type: size_t... M. That’s how you’ve written it later (auto... cs), after all.

Also as a matter of style, the ellipsis in pack expansions is usually attached to the thing being expanded. In other words, not ct_string<M, C> ... strs, but rather ct_string<M, C>... strs. Again, adding that extra space creates confusion, because now it looks like a fold expression. Again, you do the right thing later (twice: std::array{cs...} and std::tuple_cat(strs._value)...). This because super important when you start using decorators, because ct_string<M, C> && ... strs really looks like a fold expression, while ct_string<M, C>&& ... strs does not. Speaking of which:

You are taking all the arguments by value. This is hopefully harmless, if this function always ever runs at compile time. However, as I’ve demonstrated, you can’t be sure of that in C++17. And copying large strings can get expensive. So you should use references to be safe.

Now, as for the implementation of this function… it’s really a bit absurd.

All you need is something like this:

template <typename C, std::size_t... M>
static constexpr auto cat(ct_string<M, C> const&... strs) noexcept
{
    auto res = ct_string<(M + ...), C>{};

    auto p = res._value.begin();
    auto append = [&p](auto&& s) { p = std::ranges::copy(s, p).out; };

    (append(strs), ...);

    return res;
}

(That assumes a default constructor for the static string class, which it doesn’t have. If you don’t want one, there are plenty of ways to work around it, though most require an extra, temporary array. Not a big deal since this is supposed to be a compile-time operation.)

If ct_string had operator+ it would be even simpler:

template <typename C, std::size_t... M>
static constexpr auto cat(ct_string<M, C> const&... strs) noexcept
{
    return (strs + ...);
}

This technically involves a lot of intermediaries, but it’s supposed to be a compile-time op. If there’s any chance it won’t be, I’d go with the first version.

But the real problem with this function is that it doesn’t really make any sense as a static member function of the ct_string class. And you know this, because in order to use it later, you have to write: eld::ct_string<0, Char>::cat(strs...). That’s conceptual gibberish. With that line of code, you’re saying that you’re using the concatenation function of a zero-length string on a bunch of strings of any length. … whut 🤨?

See, the concatenation function should not be a property of the string size. There is no difference between ct_string<0, char>::cat(...) and ct_string<1, char>::cat(...). No part of the concatenation function has anything to do with N; neither the arguments, the return type, nor the algorithm used. If you had a base class—as in template <std::size_t N, typename Char> class ct_string : public ct_string_base<Char>, then it might make sense to have cat() in the base class (so ct_string_base<Char>::cat(...)).

But, really, this seems like something that should be a free function… which, ultimately, it is, in the form of eld::concat(). You’re really kinda going in circles by having both.

        template <typename To>
        constexpr explicit operator To() const noexcept{
            return _cast<To>{}(std::as_const(this->_value));
        }

What exactly is this for?

I know, I read the bit about the experimental approach to casting. But I honestly can’t see the sense in it. ct_string is a string, n’est-ce pas? That means it should behave as a string—it should be printable, it should have a size, it should have character access, etc.. It should also be convertible to the core vocabulary types for strings in C++, which are:

  1. std::string_view; and
  2. a (pointer to a) NUL terminated character array;

… or, for maximum flexibility and efficiency, both.

Once you can do that with a ct_string, you can anything stringy. You don’t need anything else.

Okay, but you also want to be able to do some static trickery. It’s a static string, so, sure, why not. But you already have that by way of your specializations of std::tuple_size, std::tuple_element, and get() (although, the latter is currently done incorrectly).

        const std::array<value_type, N> _value;

Don’t make data members const. It’s an anti-pattern.

I get that you’re in a bit of a pickle because you want this array to be an implementation detail, but it has to be public. The solution, however, is not to make the array const. That is a halfway-nonfix that only creates other headaches.

I would suggest one of two strategies:

  1. Insist on the array being an implementation detail by giving it a really ugly name. Yes, even uglier than a single underscore prefix. Name it something like _internal_value_DONT_MESS_WITH_THIS, and maybe even change the name every version bump, so that anyone who tries to use it faces grief.
  2. Stop worrying and learn to love it. In other words, drop the underscore and just make that value array part of the public interface. Sure people will be able to change the contents of the string as they please… but why is that a problem anyway? People can already do that for std::string, and it hasn’t ruined that class.

If you implement get() properly, people will already be able to access and modify the string anyway, so… what’s the problem? Why do you want it to be immutable anyway? Just because something is static doesn’t mean it has to be constant. In fact, it’s becoming common practice in C++ to do complicated work at compile time, including lots of modifying of values. Even std::string is now constexpr.

So I would suggest to forget immutability; it gains you nothing, and if users really want immutable static strings, well, that’s what const is for.

    namespace traits{
        // TODO: deduction rules instead
        template <typename String>
        struct is_static_string : std::false_type {};

        template <size_t N, typename Char>
        struct is_static_string<ct_string<N, Char>> : std::true_type{};

        template <typename Char, size_t N>
        struct is_static_string<Char[N]> : std::true_type{};

        template <typename Char, size_t N>
        struct is_static_string<Char(&)[N]> : is_static_string<Char[N]>{};

        template <typename Char, size_t N>
        struct is_static_string<const Char(&)[N]> : is_static_string<Char[N]>{}; 
    }

These don’t work. To my knowledge, it is impossible in C++17 to determine whether a string is static or not, for whatever you definition of “static” happens to be.

To see how easy these are to break:

auto p = std::make_unique<char[]>(3);
auto q = reinterpret_cast<char (*)[3]>(p.get());

std::cout << eld::traits::is_static_string<decltype(*q)>::value;

auto s = std::make_unique<eld::ct_string<2, char>>(*q);

std::cout << eld::traits::is_static_string<decltype(*s)>::value;

I suspect you need at least C++20 to verify compile-time strings.

        class _concat
        {
        private:
            using _static_string = std::true_type;
            using _dynamic_string = std::false_type;

            template <typename ... Strs>
            using _string_type = typename std::conjunction<traits::is_static_string<Strs>...>::type;

        public:
            template <typename ... Strings>
            constexpr auto operator()(const Strings &...strs) const noexcept {            
                return _impl(_string_type<Strings...>{}, strs...);
            }
        private:
            template <typename ... Strings>
            constexpr static auto _impl(_static_string, const Strings &... strs) noexcept {
                return std::tuple_cat(ct_string(strs)...);
            }

            // can it be made constexrp?
            template <typename ... Strings>
            static auto _impl(_dynamic_string, const Strings &... strs) noexcept {
                auto dynamicString = std::string();
                dynamicString.reserve((std::size(strs) + ...));
                (dynamicString.append(static_cast<std::string_view>(strs)), ...);
                return dynamicString;
            }
        };

Let’s take a step back and think about what you really want to happen with the concatenation function. What you really want is to vary the return type depending on the arguments. If, and only if, all of the arguments are a type that represents a sequence of characters with a compile-time known size, then you want to return a ct_string. Otherwise, you want to return a std::string.

Let’s focus on that first requirement. You want anything where there is a compile-time-known number of elements, and all the elements are char, to be considered a “static string”. That includes:

  • eld::ct_string<3, char>
  • char[3] (technically, char[4] where the last character is NUL, but whatever)
  • std::array<char, 3>
  • std::tuple<char, char, char>
  • struct foo { char a; char b; char c; } (where foo supports the tuple-like interface)

(I’m assuming that last one, but it makes sense.)

The comment on that last one has an interesting clue: “tuple-like”. If we introduce “tuple-like” as a requirement, we get:

  • eld::ct_string<3, char>
  • char[3] (technically, char[4] where the last character is NUL, but whatever)
  • anything tuple-like:
    • std::array<char, 3>
    • std::tuple<char, char, char>
    • struct foo { char a; char b; char c; } (where foo supports the tuple-like interface)

Technically, ct_string is also tuple-like, but I’m keeping it separate because it is the type being focused on here.

Now, ct_string is, well, ct_string. char[3] is what you seem to be considering a “static string”. (Again, technically char[4], with the trailing NUL, but whatevs.) char[3] is (implicitly) convertible to ct_string. The other three things aren’t currently convertible to ct_string… but they could be, quite trivially. The conversion is simply to ct_string<std::tuple_size_v<T>, char>, and you fill the internal array with std::make_index_sequence<std::tuple_size_v<T>>() and {get<IndexSequence>(t)...}.

In other words: If all the arguments are convertible to ct_string, you want to return a ct_string. Otherwise, a std::string.

See, what I’m getting at is that you are not really looking for static strings… whatever you may mean by “static”. What you are really looking for is much simpler. If all the arguments are ct_string or convertible to ct_string, then you want to return a ct_string.

So what you need is a trait/concept that returns true for ct_string, T[N] (const or not, where T is a character), and anything that is tuple-like where all the tuple elements are the same (and are characters). That trait represents “convertible to a ct_string”, which suggests the name for it.

And the interesting thing is… that’s already your implementation! Your “static” cat() function is just: return std::tuple_cat(ct_string(strs)...);. The only error is that std::tuple_cat() should return a std::tuple(), so you need one more step to convert that tuple to a ct_string. (Or, alternately, not use std::tuple_cat(), and instead have a function like the one I showed above, that just concatenates ct_strings.

One last thing to mention. In the “dynamic” implementation, you do:

                auto dynamicString = std::string();
                dynamicString.reserve((std::size(strs) + ...));
                (dynamicString.append(static_cast<std::string_view>(strs)), ...);
                return dynamicString;

Be careful here. When converting stringy things to string_view, you have to watch out for embedded NUL characters. Your conversion from ct_string to string_view does the right thing… this function does not. If one of the strs is a char array with an embedded NUL, it will degrade to a char*, and then string_view will use std::strlen() to determine the length… which will not be the same as the size of the char array.

You need to be a little more careful here. You can’t just trust conversion to string_view. You need to detect whether the str is a char* or a char (&)[], and behave differently.

Also, you’re not considering that the character type may not be char.

    template <typename T>
    struct separator
    {
        T value;
    };

Mmm, I’m not going to go into any of this separator stuff. It’s not a bad idea, but it’s entirely orthogonal to everything else—the static string type, the concatenation, etc.. Basically, I would suggest cutting the separator stuff out entirely for now, getting everything else right, and then worrying adding separator functionality… which will then be fairly trivial. (And you can determine whether it can be done “statically” by checking whether all the strings and the separator value are convertible to ct_string.)

    template <typename FirstString, typename ... Strings>
    constexpr auto concat(const FirstString& first, const Strings& ... strings){
        return detail::_concat{}(first, strings...);
    }

Your implementation is very close to being a neibloid. I would suggest going all in, and getting all the benefits of neibloids.

In other words, your current design is basically this:

namespace detail {

class _concat
{
public:
    template <typename... Strings>
        /* requires (constraints...) */
    constexpr auto operator()(Strings&&... strings) const
    {
        // implementation...
    }

private:
    // ... implementation details ...
};

} // namespace detail

template <typename FirstString, typename... Strings>
constexpr auto concat(FirstString&& first, Strings&&... strings)
{
    return detail::_concat{}(std::forward<FirstString>(first), std::forward<Strings>(strings));
}

I suggest modifying it to this:

namespace detail {

class _concat
{
public:

    template <typename FirstString, typename... Strings>
        /* requires (constraints...) */
    constexpr auto operator()(FirstString&& first, Strings&&... strings) const
    {
        // implementation...
    }

private:
    // ... implementation details ...
};

} // namespace detail

inline constexpr auto concat = detail::_concat{};

Which, as you can see, is basically the same. There’s just one less level of indirection, and you get the benefits of concat being an object rather than a function (which is what neibloids are all about).

C++20

So, let’s assume that you’re taking my advice and ditching C++17 for C++20. What do you gain?

Well, for starters, your ct_string class becomes, basically:

template <std::size_t N, typename Char>
    requires (N < std::numeric_limits<std::size_t>::max())
class ct_string 
{
public:
    using value_type = Char;

    template <std::size_t M>
        requires (M <= (N + 1))
    consteval ct_string(value_type const (&s)[M])
    {
        // ...
    }

    // any other useful member functions or friends...
};

The magic is the consteval keyword, which makes sure the ct_string class is what you want it to be: a compile-time string class. If you make every method of creating a ct_string consteval, then basically all of the problems with determining whether a string is compile-time or not go away. If it’s a ct_string, it must be a compile-time string.

What you could do is create a concept/trait that detects whether a thing is a static string… where “static string” is defined as: knows its size at compile-time, and has only characters. So: ct_string, char[N], std::tuple<char, char>, std::array<char, 3>, and anything that follows the tuple protocol and is all characters. With that, and some helper functions to extract the size and characters, concat() basically becomes:

template <string_like S1, string_like... S>
    requires (std::same<char_type_of<S1>, char_type_of<S>> and ...)
constexpr auto concat(S1 const& s1, S const&... s)
{
    if constexpr (is_static_string<S1> and ... and is_static_string<S>)
    {
        char_type_of<S1> dummy[1] = {};
        auto res = ct_string<(static_string_size_of<S1> + ... static_string_size_of<S>), char_type_of<S1>>{dummy};

        // do append as shown previously...

        return res;
    }
    else
    {
        auto res = std::basic_string<char_type_of<S1>>{};
        res.reserve((std::size(s1) + ... + std::size(s)));
        (res.append(s1), ..., res.append(to_string_view(s)));
        return res;
    }
}

// You will need:
//  concept string_like:
//      Detects anything stringy. Maybe could just check
//      convertibility to string_view?
//  trait char_type_of<T>:
//      Basically std::ranges::range_value_t<T>, except it has to
//      work even if T is a char* or char const*.
//  concept is_static_string:
//      Returns true for ct_string, character arrays, and
//      anything tuple-like.
//  trait static_string_size_of<T>:
//      Basically tuple_size<T>, but has to work for C arrays.
//  function to_string_view():
//      Basically just a cast to string_view, but has to be smart
//      about C arrays with embedded NULs.

As you can see, there’s not much you need, and most of it is pretty easy to hash out.

There is one little wrinkle in C++20, and it’s that even with the if constexpr above, you can’t be sure that the function is running at compile-time. That means that you might not be able to make ct_string consteval-only until C++23. I’d have to do more thinking to know if this is a deal-breaker or not. C++23 will bring if consteval, which will completely solve the problem of figuring out whether a given concat() is happening at run-time or compile-time, and then everything will for sure work.

I slapped together a simple godbolt to illustrate: https://godbolt.org/z/P8G3YerTn. Note that I made no attempt to make concat() both compile-time and run-time; it’s all compile-time. And I didn’t bother to make any of the concatenation stuff smart enough to do conversions. But you can see that everything in the eld namespace is compile-time only (except for the IOstream stuff, of course).

Summary

  • A generalized concatenation function is a good idea. One that is smart enough to do all the concatenation at compile-time whenever possible is even better.
  • You need to nail down your terminology, and clearly decide and define what you mean by a “static” string.
  • If you mean a compile-time string… then C++17 isn’t going to work for you. It will mostly work, but can be broken. Accept the limitations, or move to C++20 (at least). (Personally, I would recommend splitting the difference: move to C++20, because it’s such a massive improvement in everything (the constraints alone will improve your life immensely), and accept that you can’t make a perfect compile-time-only string type until C++23.)
  • Consider putting the focus on the compile-time string class, rather than the concatenation function. As I always say when I teach C++, it’s all about the types; when you get the types right, everything else Just Works.
  • Forget the separator stuff altogether, at least for now. It’s a distraction. If you get everything else working, then it will be trivial to add.
  • Think about what you are trying to accomplish in terms of interfaces and protocols… especially standard ones. You are trying to make a string class… so what does the standard string class interface look like in C++ (for this, you can use std::string_view as a model of what a constant string looks like). You are also trying to make something that can be manipulated at compile-time, and in the type system, so look into what that usually means in C++. (For example, there is the tuple protocol.) Ideally, if ct_string is supposed to be a string, it should be a more-or-less transparently drop-in replacement for std::string or std::string_view. If ct_string is supposed to be a compile-time string that can be manipulated by the type system, then it would be nice if it could be decomposed as a structured binding, converted to a tuple/array, and so on (that doesn’t mean you need to provide all the functions to convert to tuples/arrays… just implement the standard protocols that allow it; like, if ct_string is tuple-like, then you can convert to a tuple with just auto as_tuple = std::tuple_cat(ct_str);, and conversion to arrays is not much harder).
  • Consider making concat() a neibloid.
  • Never override, overload, or specialize functions in std. Specifically, tuple_cat() and get(). (You can specialize classes, when permitted… never functions.) And whenever you do specialize something in std, follow the rules.
  • Don’t make class data members const.
  • Watch out for embedded NUL.

In addition to focusing on the compile-time string type, I would also recommend making a proper test suite, using a proper test framework. Among other things, test:

  • zero-length strings
  • character types other than char
  • strings with embedded NUL
  • that the things you think are compile-time really are compile-time (in your main(), you have a comment stating that the first concat() produces a static string… are you sure?)

Happy coding!

\$\endgroup\$
11
  • 1
    \$\begingroup\$ A huge thanks for your review. I'm not done reading though) "static" is not about storage there, it's an opposite of "dynamic" like in "dynamic arrays" (though its a general CS term rather than C++) \$\endgroup\$ Commented Feb 12, 2023 at 11:33
  • \$\begingroup\$ Also about std::array example. It is not possible in C++ to overload a function based on constepxr or consteval specifier. So there is no way to detect a constexpr function argument. Neither can you ensure that one passes a literal (that is an NTS string). \$\endgroup\$ Commented Feb 12, 2023 at 11:42
  • \$\begingroup\$ About std::tuple_cat - I noticed that it doesn't return a std::tuple when you pass std::array. As a matter of fact it doesn't return std::array either. \$\endgroup\$ Commented Feb 12, 2023 at 11:54
  • \$\begingroup\$ Also, I intended ct_string to be immutable since string literals are immutable. \$\endgroup\$ Commented Feb 12, 2023 at 12:00
  • 1
    \$\begingroup\$ Basically, the ct_string class was a mean to implement compile-time catfor literals, which in turn was an attempt to implement an embedded DSL for postgres. That is, all the ct_string manipulations are required to be zeroed-out upon optimization. And those kind of are optimized if you look at the godbolt link \$\endgroup\$ Commented Feb 12, 2023 at 12:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.