✨ 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.
There was a problem hiding this comment.
This is to allow explicit use of the clangd resolver for this repo. Without this, developers will need to manually configure this for every machine as this is a non-standard way of interacting with clangd. If we want, we can add a neovim analog as well. Its not too uncommon to see editor metadata that is relevant and needed for a specific repo to appear in that repo
| 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