Refactor hex decoding utilities #648
Conversation
chfast
force-pushed
the
hex_refactor
branch
2 times, most recently
from
6806151
to
3b05ee3
Compare
Codecov Report
@@ Coverage Diff @@
## master #648 +/- ##
==========================================
+ Coverage 92.84% 92.99% +0.14%
==========================================
Files 23 25 +2
Lines 3552 3626 +74
Branches 376 377 +1
==========================================
+ Hits 3298 3372 +74
Misses 144 144
Partials 110 110 |
chfast
force-pushed
the
cli_file
branch
2 times, most recently
from
f317de3
to
f5cf3ab
Compare
| @@ -160,7 +159,7 @@ TEST(tool_commands, bench_inconsistent_output) | |||
| auto vm = evmc::VM{evmc_create_example_vm()}; | |||
| std::ostringstream out; | |||
|
|
|||
| const auto code = *from_hex("6000 54 6001 6000 55 6000 52 6001 601f f3"); | |||
| const auto code = *from_hex("60005460016000556000526001601ff3"); | |||
There was a problem hiding this comment.
So these tests technically can still have spaces, correct?
There was a problem hiding this comment.
No. We apply filter only when loading from a file.
tools/evmc/skip_space_iterator.hpp
Outdated
|
|
||
| /// The input filter iterator which skips whitespace characters from the base input iterator. | ||
| template <typename BaseIterator> | ||
| struct skip_space_iterator |
There was a problem hiding this comment.
Haven't fully reviewed this file, but intuitively looks ok.
There was a problem hiding this comment.
We extract the filter function (isspace) to get generic filter iterator (like boost::filter_iterator). This can be done later to filter out literal separator (').
|
Would be nice to have a test that checks how both |
Generalize the core from_hex() to take the input as pair in input iterators. The optional 0x prefix is still omitted but the "skip space" feature has been dropped.
chfast
force-pushed
the
hex_refactor
branch
4 times, most recently
from
c0337b9
to
5cec7bd
Compare
| // Filter the input. | ||
| std::string out; | ||
| std::copy(skip_space_iterator{in_buffer.begin(), in_buffer.end()}, | ||
| skip_space_iterator{in_buffer.end(), in_buffer.end()}, std::back_inserter(out)); |
There was a problem hiding this comment.
For filter iterator you need iterator pair so for the end filter iterator double end is provided. I considered some other options (default argument value, sentinel) but boost::filter_iterator does the same so I left it as is.
There was a problem hiding this comment.
But couldn't you just pass end() as the iterator?
There was a problem hiding this comment.
std::copy(skip_space_iterator{in_buffer.begin(), in_buffer.end()}, in_buffer.end(), std::back_inserter(out));
Isn't end an iterator itself?
There was a problem hiding this comment.
Yes, this is also "workable" but the iterators would have different types.
- Additional overloads for comparison operators are needed (so we can compare
filter_iteratorwithBaseIterator). - The
std::copy()and many algorithms cannot be used because they expectInputIterators to be of the same type. https://en.cppreference.com/w/cpp/algorithm/copy.
I expect much better user experience with std::range, but I also have 0 experience with these so far.
There was a problem hiding this comment.
Alright, I was wondering as it looked weird. Fine this way to avoid more code.
This adds the filter_iterator input iterator adapter to filter input values. With the additional is_not_space predicate these restores the option to filter-out whitespace before parsing hex. This is useful e.g. when loading hex from a file.
Generalize the
core from_hex()to take the input as pair in input iterators. The optional 0x prefix is still omitted but the "skip space" feature has been dropped.Add
skip_space_iterator. This restores the option to filter-out whitespace before parsing hex. This is useful e.g. when loading hex from a file.