Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add static_string variant without extra space overhead #23

Open
jm4R opened this issue Jul 8, 2021 · 3 comments
Open

Add static_string variant without extra space overhead #23

jm4R opened this issue Jul 8, 2021 · 3 comments
Labels
Feature New feature or request

Comments

@jm4R
Copy link

jm4R commented Jul 8, 2021

Current design doesn't guarantee anything about space occupied by static_string<N>. User can't assume that sizeof(basic_static_string<N, CharT>) == (N+1) * sizeof(CharT) (nor sizeof(basic_static_string<N, CharT>) == N * sizeof(CharT) as N doesn't take final null-terminator into account) because the class stores additional size_ field. This is a good thing for std::string compatibility which doesn't assume that stored string can't have null characters.

On the other hand, there are still situations where null-terminated cstring-wrapper could be handy. To show the example, we can have a pseudo-POD structure with standarized layout:

struct user_pod
{
  std::uint32_t id;
  char name[64];
  std::uint32_t something;
};

Such structures requires using non-modern C functions (like std::strncpy) to fill it. This could be replaced with something more handy:

struct user_pod
{
  std::uint32_t id;
  boost::cstring<63> name;
  std::uint32_t something;
};

...where boost::cstring is a working name of a hypothetical specific class with a strong guaranty that underlying object stores only a CharT array of size N+1 (or N, depending on further design) and therefore that objects of that class are trivially copyable.

Implementation of such class doesn't seem to be a hard work - most of existing code could be reused, only static_string_base should be conditional. To ilustrate possible design:

Wandbox link

#include <iostream>
#include <cstring>

namespace boost {

namespace detail {

template<std::size_t N, typename CharT, typename Traits>
class static_string_base
{
  using traits_type = Traits;
  using value_type = typename traits_type::char_type;
  using size_type = std::size_t;
public:
  std::size_t size_impl() const noexcept
  {
    return size_;
  }
private:
    size_type size_ = 0;
    value_type data_[N + 1]{};
};

template<std::size_t N, typename CharT, typename Traits>
class static_nullterminated_string_base
{
    using traits_type = Traits;
    using value_type = typename traits_type::char_type;
public:
  std::size_t size_impl() const noexcept
  {
    return traits_type::length(data_);
  }
private:
    value_type data_[N + 1]{};
};
    
} // namespace detail

template<std::size_t N, typename CharT, bool NullTerminated = false, typename Traits = std::char_traits<CharT>>
class basic_static_string : private std::conditional_t<NullTerminated, detail::static_nullterminated_string_base<N, CharT, Traits>, detail::static_string_base<N, CharT, Traits>>
{
  // ...
};

template<std::size_t N>
using static_string = basic_static_string<N, char, false, std::char_traits<char>>;

template<std::size_t N>
using cstring = basic_static_string<N, char, true, std::char_traits<char>>;

}

int main() {
    constexpr auto SIZE = 64;
    static_assert(sizeof(boost::static_string<SIZE>) != SIZE + 1);
    static_assert(sizeof(boost::cstring<SIZE>) == SIZE + 1);
    return 0;
}

The question is: could such new functionallity be in scope of this library? Would you accept PR with such extension?

@sdkrystian sdkrystian added the Feature New feature or request label Oct 25, 2021
@sdkrystian
Copy link
Member

Personally I don't believe this falls within the scope of the library, but I'd like to see what @vinniefalco and/or @pdimov have to say on the matter.

@pdimov
Copy link
Member

pdimov commented Oct 25, 2021

static_cstring might be useful, but its interface should probably be slightly different.

@vinniefalco
Copy link
Member

Without taking an opinion on whether the functionality is in scope, I would say that a new class is preferable to adding more template parameters to the existing class. In any event, we're in the middle of a release cycle so now might not be the best time to add anything :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants