diff --git a/docs/lessons.rst b/docs/lessons.rst index 40c39ea..a5459c6 100644 --- a/docs/lessons.rst +++ b/docs/lessons.rst @@ -11,7 +11,7 @@ One hurdle we've created for ourselves, is we need both gcc and clang to agree that an expression can be computed at compile-time; otherwise will get false alarms in our IDE (raised by LSP running in the background, which relies on clang). -Must Fully Initialize Memory +Must fully initialize memory ---------------------------- Struggled for a while with the implementation of :ref:xo::flatstring_concat @@ -42,3 +42,57 @@ Correction is to prove to clang that every memory address owned by an empty ``fl flatstring::flatstring() { std::fill_n(value_, N, '\0'); } + +Still need equality comparison alongside spaceship operator +----------------------------------------------------------- + +Had the impression that spaceship operator for :ref:xo::flatstring would be sufficient +to get all six comparison operators: + +.. code-block:: cpp + + template + constexpr auto + operator<=>(const flatstring & s1, + const flatstring & s2) noexcept + { + return (std::string_view(s1) <=> std::string_view(s2)); + } + +We observe this is not the case, at least with gcc 13.1; need to separately define :ref:xo::operator== + +.. code-block:: cpp + + template + constexpr bool + operator==(const flatstring & s1, + const flatstring & s2) noexcept + { + return ((s1 <=> s2) == std::strong_ordering::equal); + } + +Constexpr strict about pointer arithmetic +----------------------------------------- + +Initially attempted to implement :ref:xo::flatstring reverse iterators using char pointers. + +Notice there's an assymetry between reverse iterators and forward iterators. +We can (and do) implement forward iterators using char pointers. +The natural value of ``flatstring::end()`` is a char pointer referring to just past the end of +the string, i.e. to its null terminator. From the compiler's perspective, this is an ordinary +char pointer, just like other iterator values. + +For reverse iterators this isn't the case. The natural value for ``flatstring::rend()`` might +seem to be a char pointer referring to just before the first character in the string. +However this is no longer a valid pointer address -- dereferencing would be undefined behavior. + +In particular, with this implementation, gcc demotes ``flatstring::rend()`` to non-constexpr + +Workaround is to implement a shim iterator class, where representation is pointer to the +character just after the one the iterator position; iterator's ``operator*`` adjusts pointer before +dereferencing. + +This works because gcc can observe that we never dereference a reverse iterator with pointer value +at the beginning of a flatstring. diff --git a/include/xo/flatstring/flatstring.hpp b/include/xo/flatstring/flatstring.hpp index 55d9f1f..e723349 100644 --- a/include/xo/flatstring/flatstring.hpp +++ b/include/xo/flatstring/flatstring.hpp @@ -50,10 +50,80 @@ namespace xo { using iterator = char *; /** @brief representation for a readonly iterator **/ using const_iterator = const char *; - /** @brief representation for a read/write reverse iterator **/ - using reverse_iterator = char *; - /** @brief representation for a readonly reverse iterator **/ - using const_reverse_iterator = const char *; + + /** @brief representation for a read/write reverse iterator + * + * constexpr implementation is tricky here, since we can't + * form the address 'just before the beginning of the string' for @p rend() + * without losing constexprness (at least with gcc 13.1) + * + * Instead iterator always refers to the address immediately after its + * real target. This works since @c rbegin() refers to the char just before + * trailing null + **/ + struct reverse_iterator { + public: + constexpr reverse_iterator(char * p) : p_{p} {} + + constexpr bool _has_pointer() const { return p_ != nullptr; } + + constexpr bool operator==(const reverse_iterator & rhs) const noexcept { + return p_ == rhs.p_; + } + + constexpr char & operator* () const { return *(p_ - 1); } + + constexpr reverse_iterator & operator++ () { + --p_; + return *this; + } + + constexpr reverse_iterator operator++ (int) { + reverse_iterator copy = *this; + --p_; + return copy; + } + + private: + char * p_; + }; + + /** @brief representation for a readonly reverse iterator + * + * constexpr implementation is tricky here, since we can't + * form the address 'just before the beginning of the string' for @p rend() + * without losing constexprness (at least with gcc 13.1) + * + * Instead iterator always refers to the address immediately after its + * real target. This works since @c rbegin() refers to the char just before + * trailing null + **/ + struct const_reverse_iterator { + public: + constexpr const_reverse_iterator(const char * p) : p_{p} {} + + constexpr bool _has_pointer() const { return p_ != nullptr; } + + constexpr bool operator==(const const_reverse_iterator & rhs) const noexcept { + return p_ == rhs.p_; + } + + constexpr const char & operator* () const { return *(p_ - 1); } + + constexpr const_reverse_iterator & operator++ () { + --p_; + return *this; + } + + constexpr const_reverse_iterator operator++ (int) { + const_reverse_iterator copy = *this; + --p_; + return copy; + } + + private: + const char * p_; + }; ///@} /** @defgroup flatstring-constants constants **/ @@ -131,22 +201,22 @@ namespace xo { * * @pre 0<=pos<=N-1 **/ - constexpr value_type & operator[](size_type pos) { return value_[pos]; } - constexpr const value_type & operator[](size_type pos) const { return value_[pos]; } + constexpr value_type & operator[](size_type pos) noexcept { return value_[pos]; } + constexpr const value_type & operator[](size_type pos) const noexcept { return value_[pos]; } ///@} /** @defgroup flatstring-iterators iterators **/ ///@{ constexpr iterator begin() { return &value_[0]; } - constexpr iterator end() { return this->last(); } + constexpr iterator end() { return this->last(); } constexpr const_iterator cbegin() const { return &value_[0]; } constexpr const_iterator cend() const { return const_cast(this)->last(); } constexpr const_iterator begin() const { return cbegin(); } constexpr const_iterator end() const { return cend(); } - constexpr reverse_iterator rbegin() { return this->last(); } - constexpr reverse_iterator rend() { return &value_[0]; } + constexpr reverse_iterator rbegin() { return reverse_iterator(this->last()); } + constexpr reverse_iterator rend() { return reverse_iterator(&value_[0]); } constexpr const_reverse_iterator crbegin() const { return const_cast(this)->last(); } constexpr const_reverse_iterator crend() const { return &value_[0]; } constexpr const_reverse_iterator rbegin() const { return crbegin(); } @@ -156,7 +226,7 @@ namespace xo { /** @defgroup flatstring-assign assignment **/ ///@{ /** @brief put string into empty state. fills entire char array with nulls **/ - void clear() { std::fill_n(value_, N, '\0'); } + constexpr void clear() noexcept { std::fill_n(value_, N, '\0'); } /** @brief replace contents with min(count,N-1) copies of character ch **/ constexpr flatstring & assign(size_type count, value_type ch) { @@ -168,7 +238,7 @@ namespace xo { return *this; } - /** @brief replace contents with first N-1 characters of str **/ + /** @brief replace contents with first N-1 characters of @p x **/ constexpr flatstring & assign(const flatstring & x) { for (std::size_t pos = 0; pos < N-1; ++pos) value_[pos] = x.value_[pos]; @@ -176,13 +246,15 @@ namespace xo { return *this; } /** @brief replace contents with substring [pos,pos+count] of str **/ - constexpr flatstring & assign(const flatstring & x, + template + constexpr flatstring & assign(const flatstring & x, size_type pos, size_type count = npos) { std::size_t i = 0; for (; i < std::min(std::min(count, - std::max(x.capacity-1 - pos, - 0)), + (x.fixed_capacity-1 > pos) + ? x.fixed_capacity-1 - pos + : 0ul), N-1); ++i) value_[i] = x.value_[pos+i]; @@ -274,7 +346,7 @@ namespace xo { * strcmp(s, "obey..."); * @endcode **/ - constexpr operator const char * () const { return value_; } + constexpr operator const char * () const noexcept { return value_; } ///@} private: @@ -293,7 +365,7 @@ namespace xo { } template - constexpr Iterator last() { + constexpr Iterator last() noexcept { Iterator p = &value_[N-1]; /* search backward for first padding '\0' */ @@ -426,6 +498,26 @@ namespace xo { { return (std::string_view(s1) <=> std::string_view(s2)); } + + /** @brief equality comparison for two flatstrings. + * + * Example + * @code + * constexpr bool cmp = (flatstring("foo") == flatstring("foo")); + * static_assert(cmp == true); + * @endcode + * + * @note spaceship operator alone isn't sufficient to get this defined, + * at least with gcc 13.1 + **/ + template + constexpr bool + operator==(const flatstring & s1, + const flatstring & s2) noexcept + { + return ((s1 <=> s2) == std::strong_ordering::equal); + } ///@} } /*namespace xo*/ diff --git a/utest/flatstring.test.cpp b/utest/flatstring.test.cpp index 877a818..197ccfd 100644 --- a/utest/flatstring.test.cpp +++ b/utest/flatstring.test.cpp @@ -3,13 +3,236 @@ #include "xo/flatstring/flatstring.hpp" #include "xo/indentlog/scope.hpp" #include "xo/indentlog/print/tag.hpp" +#include "xo/indentlog/print/hex.hpp" #include +#include //#include namespace xo { using namespace std; namespace ut { + template + void + flatstring_iter_tests(const String & str, const char * text) { + size_t n = ::strlen(text); + + REQUIRE(str.size() == n); + + /* verify range iteration visits contents in order */ + { + size_t i = 0; + for (char ch : str) { + INFO(XTAG(i)); + + CHECK(ch == text[i]); + + ++i; + } + + REQUIRE(i == n); + } + + String str_copy; + + REQUIRE(str_copy.capacity() == str.capacity()); + REQUIRE(str_copy.empty()); + + /* verify const iteration visits string elements in order */ + { + str_copy = str; + REQUIRE(str_copy == str); + + size_t i = 0; + + for (auto ix = str_copy.cbegin(), end_ix = str_copy.cend(); ix != end_ix; ++ix) { + INFO(XTAG(i)); + + char ch = *ix; + + CHECK(ch == text[i]); + + ++i; + } + + REQUIRE(i == n); + } + + /* verify string overwrite through iterator */ + { + size_t i = 0; + + for (auto ix = str_copy.begin(), end_ix = str_copy.end(); ix != end_ix; ++ix) { + INFO(XTAG(i)); + + *ix = ('a' + i); + + ++i; + } + + REQUIRE(i == n); + + for (i = 0; i < n; ++i) { + CHECK(str_copy[i] == ('a' + i)); + } + } + + /* verify reverse iteration visits string elements in reverse order */ + { + str_copy = str; + REQUIRE(str_copy == str); + + size_t i = 0; + + for (auto ix = str_copy.rbegin(), end_ix = str_copy.rend(); ix != end_ix; ++ix) { + INFO(XTAG(i)); + + char ch = *ix; + + CHECK(ch == text[n-1-i]); + + ++i; + } + + REQUIRE(i == n); + } + + /* verify string overwrite through reverse iterator */ + { + str_copy = str; + REQUIRE(str_copy == str); + + size_t i = 0; + + for (auto ix = str_copy.rbegin(), end_ix = str_copy.rend(); ix != end_ix; ++ix) { + INFO(XTAG(i)); + + *ix = ('a' + i); + + ++i; + } + + REQUIRE(i == n); + + for (i = 0; i< n; ++i) { + CHECK(str_copy[n-1-i] == ('a' + i)); + } + } + + /* verify const reverse iteration visits string elements in reverse order */ + { + str_copy = str; + REQUIRE(str_copy == str); + + size_t i = 0; + + for (auto ix = str_copy.crbegin(), end_ix = str_copy.crend(); ix != end_ix; ++ix) { + INFO(XTAG(i)); + + char ch = *ix; + + CHECK(ch == text[n-1-i]); + + ++i; + } + + REQUIRE(i == n); + } + } + + template + void + flatstring_assign_tests(const String1 & str, const char * text, + const String2 & str2, const char * text2) { + INFO(tostr(XTAG(str), XTAG(text), XTAG(text2))); + + String1 str_copy; + + str_copy.assign(str.c_str()); + REQUIRE(str_copy == str); + + /* verify assignment from C-style string **/ + { + str_copy.assign(text2); + + INFO(tostr(XTAG(str_copy), XTAG(text2))); + + REQUIRE(::strncmp(str_copy.c_str(), text2, + std::min(::strlen(text2)+1, str_copy.capacity())) == 0); + } + + /* verify assignment from prefix of C-style string */ + for (size_t prefix = 0, n_prefix = ::strlen(text2); prefix < n_prefix; ++prefix) + { + str_copy.assign(str); + + REQUIRE(str_copy == str); + + str_copy.assign(text2, prefix); + + INFO(tostr(XTAG(prefix), XTAG(str_copy), XTAG(text2))); + + if (prefix == 0) { + REQUIRE(str_copy.empty()); + } else { + REQUIRE(str_copy.size() == std::min(prefix, str_copy.capacity())); + REQUIRE(::strncmp(str_copy.c_str(), text2, + std::min(prefix, str_copy.capacity())) == 0); + } + } + + /* verify assignment from substring */ + String2 text2_copy; + text2_copy.assign(text2); + + INFO(tostr(XTAG(text2_copy))); + + for (size_t i = 0, n = text2_copy.size(); i < n; ++i) { + /* deliberately letting j extend beyond the end of text2_copy */ + for (size_t j = i; j < n+10; ++j) { + INFO(tostr(XTAG(n), XTAG(i), XTAG(j))); + + str_copy.assign(str); + + REQUIRE(str_copy == str); + + str_copy.assign(text2_copy, i, j-i); + + INFO(tostr(XTAG(str_copy.fixed_capacity), XTAG(str_copy))); + + REQUIRE(str_copy.size() == std::min(j-i, + std::min(text2_copy.size()-i, + str_copy.capacity()))); + REQUIRE(::strncmp(str_copy.c_str(), text2_copy.c_str() + i, + std::min(j-i, str_copy.capacity())) == 0); + } + } + } + + template + void + flatstring_concat_tests(const String1 & str, const char * text, + const String2 & str2, const char * text2) + { + flatstring concat; + + REQUIRE(concat.empty()); + + /* forcing concat to occur at runtime */ + { + concat = flatstring_concat(str, str2); + auto req_str = string(text) + string(text2); + + REQUIRE(::strcmp(concat.c_str(), req_str.c_str()) == 0); + } + { + concat = flatstring_concat(str2, str); + auto req_str = string(text2) + string(text); + + REQUIRE(::strcmp(concat.c_str(), req_str.c_str()) == 0); + } + } + template void flatstring_runtime_tests(const String & str, const char * text) { @@ -22,16 +245,48 @@ namespace xo { REQUIRE(strcmp(str.c_str(), text) == 0); REQUIRE(strcmp(str, text) == 0); - /* verify range iteration visits contents in order */ + String str2 = str; + + { + string str3{str.str()}; + + REQUIRE(::strcmp(str3.c_str(), str.c_str()) == 0); + } + + REQUIRE(string_view(str2) == string_view(str)); + + { + auto cmp = (str2 <=> str); + REQUIRE(cmp == strong_ordering::equal); + } + + { + bool cmp = (str2 == str); + INFO(xtag("cmp", cmp)); + REQUIRE(str2 == str); + + bool cmp2 = (str2 != str); + REQUIRE(cmp2 != cmp); + } + + str2.clear(); + REQUIRE(str2.empty()); + + str2.assign(100, ' '); + REQUIRE(str2.size() == str2.capacity()); + + /* verify entirely ' ' */ { size_t i = 0; - for (char ch : str) { + for (char ch : str2) { INFO(XTAG(i)); - CHECK(ch == text[i]); + CHECK(ch == ' '); ++i; } + + REQUIRE(i == str2.size()); } } @@ -44,8 +299,9 @@ namespace xo { * REQUIRE() calls to do verification that relies on non-constexpr calls such as * strlen(), strcmp() */ -# define LITERAL_TEST_BODY(name, text) \ - constexpr flatstring name{text}; \ +# define LITERAL_TEST_BODY(name, name2, text, text2) \ + constexpr flatstring name{text}; \ + constexpr flatstring name2{text2}; \ static_assert(name[0]==text[0]); \ static_assert(name.at(0)==text[0]); \ static_assert(name.empty() == true || name.empty() == false); \ @@ -54,10 +310,10 @@ namespace xo { static_assert(name.end() != nullptr); \ static_assert(name.cbegin() != nullptr); \ static_assert(name.cend() != nullptr); \ - static_assert(name.rbegin() != nullptr); \ - static_assert(name.rend() != nullptr); \ - static_assert(name.crbegin() != nullptr); \ - static_assert(name.crend() != nullptr); \ + static_assert(name.crbegin()._has_pointer()); \ + static_assert(name.crend()._has_pointer()); \ + /*static_assert(name.rbegin() != nullptr);*/ \ + /*static_assert(!name.rend());*/ \ static_assert(name.size() >= 0); \ static_assert(name.c_str() != nullptr); \ static_assert((name <=> name) == 0); \ @@ -68,14 +324,11 @@ namespace xo { static_assert(!(name > name)); \ static_assert(!(name < name)); \ flatstring_runtime_tests(name, text); \ - REQUIRE(name.fixed_capacity == strlen(text)+1); \ - REQUIRE(name.capacity() == strlen(text)); \ - REQUIRE(name.size() == strlen(text)); \ - REQUIRE(name.length() == strlen(text)); \ - REQUIRE(strcmp(name.c_str(), text) == 0); \ - REQUIRE(strcmp(name, text) == 0); \ + flatstring_iter_tests(name, text); \ + flatstring_assign_tests(name, text, name2, text2); \ + flatstring_concat_tests(name, text, name2, text2); \ static_assert(string_view(name) == string_view(name)); \ - + /* end LITERAL_TEST_BODY */ TEST_CASE("flatstring", "[flatstring][compile-time]") { @@ -92,19 +345,21 @@ namespace xo { /* mostly compile-time tests here */ - LITERAL_TEST_BODY(s1, "h"); - LITERAL_TEST_BODY(s2, "he"); - LITERAL_TEST_BODY(s3, "hel"); - LITERAL_TEST_BODY(s4, "hell"); - LITERAL_TEST_BODY(s5, "hello"); - LITERAL_TEST_BODY(s6, "hello,"); - LITERAL_TEST_BODY(s7, "hello, "); - LITERAL_TEST_BODY(s8, "hello, w"); - LITERAL_TEST_BODY(s9, "hello, wo"); - LITERAL_TEST_BODY(s10, "hello, wor"); - LITERAL_TEST_BODY(s11, "hello, worl"); - LITERAL_TEST_BODY(s12, "hello, world"); - LITERAL_TEST_BODY(s13, "hello, world!"); + LITERAL_TEST_BODY(s1, t1, "h", "abracadabra!"); + LITERAL_TEST_BODY(s2, t2, "he", "bracadabra!"); + LITERAL_TEST_BODY(s3, t3, "hel", "racadabra!"); + LITERAL_TEST_BODY(s4, t4, "hell", "acadabra!"); + LITERAL_TEST_BODY(s5, t5, "hello", "cadabra!"); + LITERAL_TEST_BODY(s6, t6, "hello,", "adabra!"); + LITERAL_TEST_BODY(s7, t7, "hello, ", "dabra!"); + LITERAL_TEST_BODY(s8, t8, "hello, w", "abra!"); + LITERAL_TEST_BODY(s9, t9, "hello, wo", "bra!"); + LITERAL_TEST_BODY(s10, t10, "hello, wor", "ra!"); + LITERAL_TEST_BODY(s11, t11, "hello, worl", "a!"); + LITERAL_TEST_BODY(s12, t12, "hello, world", "!"); + LITERAL_TEST_BODY(s13, t13, "hello, world!", ""); + + static_assert(s1 == s1); static_assert(s1 != s2); static_assert(s2 != s3); @@ -123,6 +378,26 @@ namespace xo { static_assert(s4 > s3); static_assert(s5 > s4); static_assert(s13 > s12); + + /* concat */ + static_assert(flatstring_concat(s1,t1) == flatstring("habracadabra!")); + + /* clear */ + auto s13_copy = s13; + s13_copy.clear(); + + REQUIRE(s13_copy.empty()); + + constexpr auto s13_copy2 = s13; + + static_assert(s13_copy2.size() == s13.size()); + + //cerr << "s13=[" << s13 << "] s13_copy2=[" << s13_copy2 << "]" << endl; + //cerr << xtag("s13", hex_view(s13.c_str(), s13.c_str() + s13.capacity(), true)) << endl; + //cerr << xtag("s13_copy2", hex_view(s13_copy2.c_str(), s13_copy2.c_str() + s13_copy2.capacity(), true)) << endl; + + REQUIRE(s13_copy2 == s13); + } /*TEST_CASE(flatstring)*/ } /*namespace ut*/