✨ Added scatter_span functionality#3
Conversation
…in its scatter_span
print helper functions have been left in the test for debugging purposes
PhazonicRidley
force-pushed
the
scatter_span_base
branch
from
eea3c49 to
387b994
Compare
kammce
left a comment
kammce
left a comment
There was a problem hiding this comment.
Great work! Looking at the tests, I can see that this works just like I wanted it to. I'm excited to be able to write.
m_serial->write({ header, body, footer});
And no longer need an extra array or to add make_scatter_array each time.
|
|
||
| # Allowed for clangd resolver script | ||
| !.vscode/settings.json No newline at end of file | ||
| !.vscode/settings.json |
There was a problem hiding this comment.
I'd rather us just remove the line not allowing .vscode into the code base.
| def build(self): | ||
| cmake = CMake(self) | ||
| cmake.configure() | ||
| cmake.configure(cli_args=["-Wno-dev"]) |
There was a problem hiding this comment.
| cmake.configure(cli_args=["-Wno-dev"]) | |
| cmake.configure() |
| namespace mem::detail { | ||
| template<typename T, size_t N> | ||
| struct scatter_array_storage | ||
| { | ||
| std::array<std::span<T const>, N> m_internal_arr; | ||
| }; | ||
| }; // namespace mem::detail |
There was a problem hiding this comment.
| namespace mem::detail { | |
| template<typename T, size_t N> | |
| struct scatter_array_storage | |
| { | |
| std::array<std::span<T const>, N> m_internal_arr; | |
| }; | |
| }; // namespace mem::detail | |
| namespace { | |
| template<typename T, size_t N> | |
| struct scatter_array_storage | |
| { | |
| std::array<std::span<T const>, N> m_internal_arr; | |
| }; | |
| }; // namespace |
Since we aren't using headers anymore, we don't need to do the whole "detail" dance. You should be able to just use an anon namespace within a module interface.
| }; | ||
| }; // namespace mem::detail | ||
|
|
||
| export namespace mem { |
There was a problem hiding this comment.
| export namespace mem { | |
| namespace mem { |
Refrain from exporting the whole namespace. Export each item we want available for our public APIs.
| * @details Works similarly to @c std::span, but the viewed elements do not | ||
| * need to be contiguous in memory. Each constituent chunk is a separate | ||
| * @c std::span<T const>. Iterating yields each chunk a span in order. |
There was a problem hiding this comment.
Whats up with the @c things within the comment?
| constexpr scatter_span<T>(std::initializer_list<std::span<T const>> p_il) | ||
| : m_spans(p_il.begin(), p_il.size()) | ||
| , m_start_pos(0) | ||
| , m_final_len(p_il.size() == 0 ? 0 : (p_il.end() - 1)->size()) |
There was a problem hiding this comment.
| , m_final_len(p_il.size() == 0 ? 0 : (p_il.end() - 1)->size()) | |
| , m_final_len([p_il]() { | |
| if (p_il.size() == 0) { return 0; } | |
| else { return p_il.back().size(); } | |
| }()) |
This is a suggestion of what you could turn the ternery into if you'd prefer to use a lambda. But if you do stick with the ternary, can you change the API to use .back() vs end() - 1. I think its better expressed that way.
| size_t start_pos = | ||
| boundary ? 0 : p_args.offset - (cur_len - effective_span_size); |
There was a problem hiding this comment.
| size_t start_pos = | |
| boundary ? 0 : p_args.offset - (cur_len - effective_span_size); | |
| size_t start_pos = 0; | |
| if (boundary) { start_pos = p_args.offset - (cur_len - effective_span_size); } |
I find if statements are a bit cleaner and less obtuse than ternaries.
| auto const* first = m_ssp->m_spans.data(); | ||
| auto const* last = m_ssp->m_spans.data() + m_ssp->m_spans.size() - 1; |
There was a problem hiding this comment.
Any reason not to use begin() and end() for these? Last could also just be m_ssp->m_spans.back().
| expect(that % scatter_span_eq(offset_greater_than_len, {})); | ||
| }; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Can we add a test to also verify that the scatter span object can be used within a ranged for loop. Currently we use that property within the sub scatter span, but if we ever change the implementation of that function to no longer use the ranged-for loop, then we might silently break it without knowing.
| #include <cstddef> | ||
| #include <span> | ||
|
|
||
| export module scatter_span; |
2 participants
Added the basic functionality for scatter span using modules and as cheaply as possible (memory and timing wise). Primarily geared towards embedded systems but can be used anywhere