Static String data type for C++ container library #1031
Conversation
ComicSansMS
force-pushed
the
iox2-938-base-library-static-string
branch
from
c693bf5 to
48291bb
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1031 +/- ##
==========================================
- Coverage 80.20% 80.18% -0.02%
==========================================
Files 258 258
Lines 32252 32252
==========================================
- Hits 25867 25862 -5
- Misses 6385 6390 +5 🚀 New features to boost your workflow:
|
| constexpr StaticString(StaticString const&) noexcept = default; | ||
| constexpr StaticString(StaticString&&) noexcept = default; | ||
|
|
||
| template <uint64_t M, std::enable_if_t<(N > M), bool> = true> |
There was a problem hiding this comment.
What do you think about having a static_assert instead of the enable_if to provide the user a clearer error message why their code doesn't compile?
There was a problem hiding this comment.
static_assert does not play nice with SFINAE (at least not without resorting to some tricks), which makes it an inferior choice for restricting overload sets.
The problem with static_assert is that is evaluated too late: The constrained function will participate in overload resolution, same as all other candidates of the overloard set and the best match will win irrespective of constraints. Only then will the static_assert be evaluated and if its constraints fail it will be a hard error, as all other candidates have already been discarded. This is highly undesirable in contexts in which you want to handle a constraint failure as a recoverable compile time error, which is very common with overload sets.
I will think about how to improve the diagnostics here.
|
Regarding the failing Windows builds: #1049 |
ComicSansMS
force-pushed
the
iox2-938-base-library-static-string
branch
from
0fc8854 to
eda8d79
Compare
|
@ComicSansMS Just nitpicking here, don't need to change it in this PR, but just for information - the commits in this repository are conventionally written using the imperative form. You can see it in the commit history. See this link, which is also included in the PR template: Another nice article: |
ComicSansMS
force-pushed
the
iox2-938-base-library-static-string
branch
from
eda8d79 to
4beed71
Compare
Thanks, I completely forgot about the imperative forms. I adjusted all commit messages. |
ComicSansMS
force-pushed
the
iox2-938-base-library-static-string
branch
from
4beed71 to
9a4aa9e
Compare
| return std::all_of(str.unchecked_access().data() + str.size(), | ||
| str.unchecked_access().data() + str.capacity() + 1, | ||
| [](char character) -> bool { return character == '\0'; }); | ||
| } |
| auto sut = *iox2::container::StaticString<STRING_SIZE>::from_utf8("ABC"); | ||
| ASSERT_TRUE(sut.unchecked_code_units().back_element()); | ||
| sut.unchecked_code_units().back_element().value().get() = 'Z'; | ||
| ASSERT_STREQ(sut.unchecked_access().c_str(), "ABZ"); |
There was a problem hiding this comment.
Please add ASSERT_TRUE(free_space_is_all_zeroes(sut)); here, too.
Notes for Reviewer
Implementation of StaticString for C++ base library.
Pre-Review Checklist for the PR Author
Convert to draft)SPDX-License-Identifier: Apache-2.0 OR MITiox2-123-introduce-posix-ipc-example)[#123] Add posix ipc example)task-list-completed)Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References
Closes #938.