maflcko
commented at 9:56 pm on December 17, 2024:
member
Span has some issues:
It does not support fixed-size spans, which are available through std::span.
It is confusing to have it available and in use at the same time with std::span.
It does not obey the standard library iterator build hardening flags. See #31272 for a discussion. For example, this allows to catch issues like the one fixed in commit fabeca3458b38a3d8930cb0cbc866388c3f120f1.
Both types are type-safe and can even implicitly convert into each other in most contexts.
However, exclusively using std::span seems less confusing, so do it here with a scripted-diff.
DrahtBot
commented at 9:56 pm on December 17, 2024:
contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
#30987 (Don’t zero-after-free DataStream: Faster IBD on some configurations by davidgumberg)
#30214 (refactor: Improve assumeutxo state representation by ryanofsky)
#29675 (wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys by achow101)
#29491 ([EXPERIMENTAL] Schnorr batch verification for blocks by fjahr)
#29418 (rpc: provide per message stats for global traffic via new RPC ‘getnetmsgstats’ by vasild)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot renamed this:
refactor: Use std::span over Span
refactor: Use std::span over Span
on Dec 17, 2024
DrahtBot added the label
Refactoring
on Dec 17, 2024
maflcko
commented at 9:56 pm on December 17, 2024:
member
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
l0rinc
commented at 12:52 pm on December 18, 2024:
Q: we don’t need perfect forwarding anymore because we can const the read-only version?
maflcko
commented at 2:36 pm on December 18, 2024:
There is nothing to be forwarded, so it seems clearer not doing it. You can think of a span as an object that stores a pointer and a size. Obtaining a pointer and a size can be done from a const reference as well, especially if only a pointer to const memory is needed.
in
src/span.h:121
in
02ae908080outdated
290@@ -291,9 +291,10 @@ template <typename B>
291 concept BasicByte = requires { UCharCast(std::span<B>{}.data()); };
292293 // Helper function to safely convert a Span to a Span<[const] unsigned char>.
l0rinc
commented at 12:57 pm on December 18, 2024:
comments here need to be updated, too
maflcko
commented at 3:45 pm on December 18, 2024:
thx, done
in
src/test/span_tests.cpp:58
in
b66694f7b8outdated
The reason why std::vector<bool> is not spannable is because the iterator is not contiguous, not due to the type of the data pointer
std::span is more correct in that it does all the checks, so my recommendation would be to just remove the test and all related code, as explained in #28721 (review)
l0rinc
commented at 1:25 pm on December 18, 2024:
contributor
Concept ACK
Thank you for doing these big-bang cleanups!
I understand it’s still a draft, but consider the following code parts that still need to be touched:
l0rinc
commented at 3:28 pm on December 18, 2024:
contributor
Can FuzzedDataProvider and CHMAC_SHA256 take a const std::span<const uint8_t> parameter now instead of separate data and size - or is that out of scope?
maflcko force-pushed
on Dec 18, 2024
maflcko force-pushed
on Dec 18, 2024
maflcko
commented at 3:48 pm on December 18, 2024:
member
Can FuzzedDataProvider and CHMAC_SHA256 take a const std::span<const uint8_t> parameter now instead of separate data and size - or is that out of scope?
I want to keep the scope minimal here. It is already more than 500 lines modified, albeit most of them trivially. In theory I could even limit the scope a bit more and defer the scripted-diff (and later commits) for later.
maflcko force-pushed
on Dec 19, 2024
maflcko force-pushed
on Dec 19, 2024
maflcko force-pushed
on Dec 19, 2024
vasild
commented at 6:36 pm on December 19, 2024:
contributor
Concept ACK
maflcko force-pushed
on Dec 19, 2024
achow101 referenced this in commit
e6f14241f6
on Dec 30, 2024
maflcko force-pushed
on Jan 2, 2025
maflcko force-pushed
on Jan 3, 2025
maflcko force-pushed
on Jan 3, 2025
maflcko force-pushed
on Jan 14, 2025
fanquake
commented at 5:12 pm on January 21, 2025:
member
Rebase now that we’ve got leveldb + the sha changes in?
l0rinc
commented at 5:13 pm on January 21, 2025:
contributor
the UCharCast ones
maflcko force-pushed
on Jan 21, 2025
maflcko force-pushed
on Jan 21, 2025
maflcko marked this as ready for review
on Jan 21, 2025
maflcko
commented at 10:01 pm on January 21, 2025:
member
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
DrahtBot added the label
CI failed
on Jan 24, 2025
maflcko force-pushed
on Jan 24, 2025
maflcko force-pushed
on Jan 24, 2025
DrahtBot removed the label
CI failed
on Jan 24, 2025
maflcko
commented at 10:53 am on January 24, 2025:
member
Rebased and updated the description with the merged UB fix it found to serve as additional motivation
fanquake requested review from theuni
on Jan 24, 2025
in
src/cluster_linearize.h:78
in
fa76d5aabaoutdated
74@@ -75,7 +75,7 @@ class DepGraph
75 *
76 * @param depgraph The original DepGraph that is being remapped.
77 *
78- * @param mapping A Span such that mapping[i] gives the position in the new DepGraph
79+ * @param mapping A std::span such that mapping[i] gives the position in the new DepGraph
nit: this new std::byte std::span looks a bit awkward now
in
src/bench/load_external.cpp:48
in
fa76d5aabaoutdated
45@@ -46,7 +46,7 @@ static void LoadExternalBlockFile(benchmark::Bench& bench)
46 ss << static_cast<uint32_t>(benchmark::data::block413567.size());
47 // We can't use the streaming serialization (ss << benchmark::data::block413567)
48 // because that first writes a compact size.
49- ss << Span{benchmark::data::block413567};
50+ ss << std::span{benchmark::data::block413567};
I think it is clearer to keep span-serialization documented here. Previously, the data was contained in a vector, which made this span constructor required (std::vector<uint8_t> block413567).
This changed in commit faecca9a85c1c1917d024f55cca34d19cc94f3b9, which forgot to update this comment. So I think a separate commit could update the comment here.
I know, but this is unrelated to this pull request. std::span serialization is allowed today on current master (faaf4800aa752dde63b8987b1eb0de4e54acf717), so if someone wanted to remove this, it could be done unrelated to this pull request.
I don’t really see the risk of documenting that span serialization is intended, but I am happy to reword or remove the comment.
(Resolving for now, but I am still happy to reword or remove the comment.)
in
src/test/span_tests.cpp:49
in
fa76d5aabaoutdated
47-// don't work. This makes it is possible to use the Span constructor in a SFINAE
48+// Make sure template std::span template deduction guides accurately enable calls to
49+// std::span constructor overloads that work, and disable calls to constructor overloads that
50+// don't work. This makes it is possible to use the std::span constructor in a SFINAE
51 // contexts like in the Spannable function above to detect whether types are or
52 // aren't compatible with Spans at compile time.
0// don't work. This makes it possible to use the std::span constructor in a SFINAE1// contexts like in the Spannable function above to detect whether types are or2// aren't compatible with std::span at compile time.
in
src/script/interpreter.h:340
in
fa76d5aabaoutdated
334@@ -335,13 +335,13 @@ class DeferringSignatureChecker : public BaseSignatureChecker
335 };
336337 /** Compute the BIP341 tapleaf hash from leaf version & script. */
338-uint256 ComputeTapleafHash(uint8_t leaf_version, Span<const unsigned char> script);
339+uint256 ComputeTapleafHash(uint8_t leaf_version, std::span<const unsigned char> script);
340 /** Compute the BIP341 tapbranch hash from two branches.
341 * Spans must be 32 bytes each. */
This looks like a good use-case for a static extent :)
(as a follow-up)
l0rinc
commented at 10:34 pm on January 27, 2025:
contributor
I have reviewed until fa76d5aaba34928cf403b5e142705fd03e26553f - mostly nits, only concern is a possible slight behavior change.
Could you please check if the serialization method changes that resulted from this are expected (likely applied to the array-backed spans with fixed sizes only)?
DrahtBot requested review from l0rinc
on Jan 27, 2025
DrahtBot
commented at 4:55 am on January 28, 2025:
contributor
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
DrahtBot added the label
CI failed
on Jan 28, 2025
maflcko force-pushed
on Feb 4, 2025
maflcko
commented at 9:29 pm on February 4, 2025:
member
Rebased and added one doc-only commit to address all feedback. Should be trivial to re-review.
in
src/span.h:14
in
9999fb5bd4outdated
10@@ -11,49 +11,49 @@
11 #include <type_traits>
12 #include <utility>
1314-/** A Span is an object that can refer to a contiguous sequence of objects.
15+/** A std::span is an object that can refer to a contiguous sequence of objects.
shouldn’t these go to the developer notes now? We don’t have files for other std types locally which explain their behavior.
maflcko
commented at 10:25 pm on February 4, 2025:
I can see it both ways. The span.h will remain to hold std::span helpers, so it seems fine to keep it there. It is also fine to move it the dev notes, but this should be a separate commit, to make review easier. Finally, it is also fine to remove it, as the std lib has documentation and doesn’t need additional ones in this repo?
I don’t really mind either way and I think anything is fine, so I just try to make it as easy to review for reviewers.
I find the suggestions helpful, I’m fine with moving them to doc separately.
The span's should likely be adjusted here as well, maybe to instances of std::span or something similar.
maflcko
commented at 12:40 pm on February 5, 2025:
Adjusted span's, so resolving this for now.
l0rinc
commented at 10:21 pm on February 4, 2025:
contributor
Docker seems to complain:
You have reached your pull rate limit
But locally all tests were passing for me.
I think this is an important change, I’d just a like a few minor changes (like remaining “Spans” references), and I’d prefer doing the unrelated header changes in a different PR for transparency.
DrahtBot removed the label
CI failed
on Feb 4, 2025
maflcko force-pushed
on Feb 5, 2025
in
src/test/span_tests.cpp:47
in
fa04eec5adoutdated
55-// Make sure template Span template deduction guides accurately enable calls to
56-// Span constructor overloads that work, and disable calls to constructor overloads that
57-// don't work. This makes it is possible to use the Span constructor in a SFINAE
58+// Make sure template std::span template deduction guides accurately enable calls to
59+// std::span constructor overloads that work, and disable calls to constructor overloads that
60+// don't work. This makes it is possible to use the std::span constructor in a SFINAE
Thx, happy to push, but I’ll wait until you are done with your review until you reached an ack-state before pushing again. Otherwise, I’ll just spend cycles on finding the right commit to fixup the unrelated typo and then pushing it for each typo/nit separately. It is also easier for reviewers to place a single ack and a single re-ack, instead of re-acks for each typo individually.
I posted this again since I saw that the build was failing, thought you’ll need to push soon anyway.
Will the copyright changes remain part of this PR? I would prefer reviewing that separately, but if you insist, I can do it here as well.
maflcko
commented at 11:01 am on February 5, 2025:
Up to you, you can skip review of the commit (and say so, and ask me to drop it), or you can review it. I’ll just wait for your ack and further comments, so that everything can be wrapped up in as few force pushes and re-reviews as possible.
The CI issue was on the CI machine, so I fixed it there and did a re-run.
Reviewed it in more detail, git diff --name-only HEAD~6..HEAD basically only contains doc/developer-notes.md in addition to the previous commit - which doesn’t contain a copyright header. The other skipped files don’t contain any dates, so it’s a correct change, nicely done.
maflcko
commented at 12:58 pm on February 5, 2025:
Pushed the typo fix, so closing this for now.
in
src/span.h:295
in
faa0e0b66aoutdated
289@@ -290,9 +290,10 @@ template <typename B>
290 concept BasicByte = requires { UCharCast(std::span<B>{}.data()); };
291292 // Helper function to safely convert a Span to a Span<[const] unsigned char>.
293-template <typename T> constexpr auto UCharSpanCast(Span<T> s) -> Span<typename std::remove_pointer<decltype(UCharCast(s.data()))>::type> { return {UCharCast(s.data()), s.size()}; }
294+template <typename T, size_t N> constexpr auto UCharSpanCast(std::span<T, N> s) -> std::span<typename std::remove_pointer_t<decltype(UCharCast(s.data()))>> { return {UCharCast(s.data()), s.size()}; }
295296 /** Like the Span constructor, but for (const) unsigned char member types only. Only works for (un)signed char containers. */
Is this still accurate, now that we’re always explicitly const-ing?
0... but forconst unsigned char member types
maflcko
commented at 12:37 pm on February 5, 2025:
The comment is for the second function below, see #31519 (review)
in
src/random.h:403
in
fa04eec5adoutdated
399@@ -400,8 +400,8 @@ class FastRandomContext : public RandomMixin<FastRandomContext>
400 return ReadLE64(buf.data());
401 }
402403- /** Fill a byte Span with random bytes. This overrides the RandomMixin version. */
404- void fillrand(Span<std::byte> output) noexcept;
405+ /** Fill a byte std::span with random bytes. This overrides the RandomMixin version. */
maflcko
commented at 12:58 pm on February 5, 2025:
thx, done
fanquake added this to the milestone 30.0
on Feb 5, 2025
l0rinc approved
l0rinc
commented at 12:12 pm on February 5, 2025:
contributor
I have recreated the changes locally and compared it against your change and reviewed the diffs one-by-one.
Based on my understanding, your changes are accurate, nicely transitioning to the new type in careful commits, you have thought of every corner case.
Rebased the change against latest master, all tests are passing locally as well and I’m getting the same scripted diff results (rewritten slightly for mac).
DrahtBot requested review from theuni
on Feb 5, 2025
maflcko force-pushed
on Feb 5, 2025
l0rinc
commented at 1:01 pm on February 5, 2025:
contributor
ACKfa77685ba5c7185781acca04f57399cdcd19e9f7
Sjors
commented at 9:17 am on February 10, 2025:
member
Concept ACK. I’ll need to rebase some things, but I’m favor of getting that over with.
theuni
commented at 5:15 pm on February 14, 2025:
member
@fanquake do you have an opinion on this for v29? Getting it in would make future backports easier. Could also see waiting until after branch-off to be less disruptive though.
Edit: Just noticed the v30 milestone. I guess that answers my question :)
l0rinc
commented at 8:47 pm on February 26, 2025:
contributor
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
maflcko force-pushed
on Feb 27, 2025
maflcko
commented at 10:26 am on February 27, 2025:
member
thx, done. Should be trivial to re-ack with git range-diff bitcoin-core/master A B
DrahtBot removed the label
CI failed
on Feb 27, 2025
l0rinc
commented at 2:49 pm on February 27, 2025:
contributor
ACKfab51b26d68b37f5914314ad29eeb930538ea7ae
Compared to previously acked fa77685ba5c7185781acca04f57399cdcd19e9f7, net.cpp and pcp_tests.cpp had header & Span -> std::span changes only.
DrahtBot requested review from Sjors
on Feb 27, 2025
laanwj
commented at 3:24 pm on March 3, 2025:
member
The resulting type here is missing the N parameter, which prevents it from forwarding extent info. That constructor is explicit, so we have to name the type.
With that done, there’s a single case of a span being reused with an incompatible extent size. Full necessary change is:
0diff --git a/src/span.h b/src/span.h
1index 62e035d2d35..5e34cd91909 100644
2--- a/src/span.h
3+++ b/src/span.h
4@@ -105,7 +105,7 @@ template <typename B>
5 concept BasicByte = requires { UCharCast(std::span<B>{}.data()); };
6 7 // Helper function to safely convert a span to a span<[const] unsigned char>.
8-template <typename T, size_t N> constexpr auto UCharSpanCast(std::span<T, N> s) -> std::span<std::remove_pointer_t<decltype(UCharCast(s.data()))>> { return {UCharCast(s.data()), s.size()}; }
9+template <typename T, size_t N> constexpr auto UCharSpanCast(std::span<T, N> s) { return std::span<std::remove_pointer_t<decltype(UCharCast(s.data()))>, N>{UCharCast(s.data()), s.size()}; }
1011 /** Like the std::span constructor, but for (const) unsigned char member types only. Only works for (un)signed char containers. */
12 template <typename V> constexpr auto MakeUCharSpan(const V& v) -> decltype(UCharSpanCast(std::span{v})) { return UCharSpanCast(std::span{v}); }
13diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
14index bc50bf408eb..0fa6f7d8e5a 100644
15--- a/src/test/util_tests.cpp
16+++ b/src/test/util_tests.cpp
17@@ -160,8 +160,8 @@ BOOST_AUTO_TEST_CASE(parse_hex)
18 BOOST_CHECK_EQUAL_COLLECTIONS(hex_literal_span.begin(), hex_literal_span.end(), expected.begin(), expected.end());
1920 const std::vector<std::byte> hex_literal_vector{operator""_hex_v<util::detail::Hex(HEX_PARSE_INPUT)>()};
21- hex_literal_span = MakeUCharSpan(hex_literal_vector);
22- BOOST_CHECK_EQUAL_COLLECTIONS(hex_literal_span.begin(), hex_literal_span.end(), expected.begin(), expected.end());
23+ auto hex_literal_vec_span = MakeUCharSpan(hex_literal_vector);
24+ BOOST_CHECK_EQUAL_COLLECTIONS(hex_literal_vec_span.begin(), hex_literal_vec_span.end(), expected.begin(), expected.end());
2526 constexpr std::array<uint8_t, 65> hex_literal_array_uint8{operator""_hex_u8<util::detail::Hex(HEX_PARSE_INPUT)>()};
27 BOOST_CHECK_EQUAL_COLLECTIONS(hex_literal_array_uint8.begin(), hex_literal_array_uint8.end(), expected.begin(), expected.end());
Arguably this is a bug introduced in this PR, as all the other conversion functions forward the extent info correctly.
I also noticed some inconsistencies while trying to rebase #31868 (review) on top of this but didn’t get to investigating why the size doesn’t always propagate - thanks for the patch.
Not sure what you mean with “retain”. In current master there is no static extent, as this is using Span.
Also, streams don’t make use of static extents at all. See also #31519 (review):
Presumably some of the streams could benefit from static extents, but that’s waaay overkill for here.
So my preference would be to not mix refactoring with questionable performance improvements.
in
src/serialize.h:539
in
fab51b26d6outdated
535@@ -536,10 +536,10 @@ struct CustomUintFormatter
536 if (v < 0 || v > MAX) throw std::ios_base::failure("CustomUintFormatter value out of range");
537 if (BigEndian) {
538 uint64_t raw = htobe64_internal(v);
539- s.write(AsBytes(Span{&raw, 1}).last(Bytes));
540+ s.write(std::as_bytes(std::span{&raw, 1}).last(Bytes));
Not sure. Even if this was the only place that makes use of this, for consistency it should be kept anyway. Otherwise, someone will have to touch this file again to add it back once it is used.
l0rinc
commented at 11:14 pm on March 9, 2025:
contributor
Found a few more static extent opportunities, but I personally don’t mind doing these in separate PRs either.
refactor: Return std::span from MakeByteSpan
In theory this commit should only touch the span.h header, because
std::span can implicilty convert into Span in most places, if needed.
However, at least when using the clang compiler, there are some
false-positive lifetimebound warnings and some implicit conversions can
not be resolved.
Thus, this refactoring commit also changed the affected places to
replace Span with std::span.
fa720b94be
refactor: Return std::span from MakeUCharSpan
This is possible and safe, because std::span can implicitly convert into
Span, if needed.
fadf02ef8b
test: Fix broken span_tests
* The comment is wrong claiming that void* was returned when void was
returned in reality.
* The namespace is missing a name, leading to compile errors that are
suppressed with non-standard pragmas, and leading to compile errors in
future commits. Instead of using more non-standard suppressions, just
add the missing name.
* The SpanableYes/No types are missing begin/end iterators, which will
be needed when using std::span.
fa27e36717
refactor: Make Span an alias of std::span
This uses a macro, which can be a bit more brittle than an alias
template. However, class template argument deduction for alias templates
is only implemented in clang-19.
scripted-diff: Bump copyright headers after std::span changes
Historically, the headers have been bumped some time after a file has
been touched. Do it now to avoid having to touch them again in the
future for that reason.
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended 's;( 20[0-2][0-9])(-20[0-2][0-9])? The Bitcoin Core developers;\1-present The Bitcoin Core developers;g' $( git show --pretty="" --name-only HEAD~1 )
-END VERIFY SCRIPT-
fa942332b4
refactor: Avoid false-positive gcc warning
GCC 14.2.1 will complain about a dangling reference after replacing Span
wiht std::span. This is a false-positive, because std::find does not
return a reference.
Remove the `&` to silence the warning. Also use ranges::find while
touching the line.
src/i2p.cpp:312:21: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
312 | const auto& pos = std::find(kv.begin(), kv.end(), '=');
| ^~~
src/i2p.cpp:312:36: note: the temporary was destroyed at the end of the full expression ‘std::find<__gnu_cxx::__normal_iterator<const char*, span<const char> >, char>((& kv)->std::span<const char>::begin(), (& kv)->std::span<const char>::end(), '=')’
312 | const auto& pos = std::find(kv.begin(), kv.end(), '=');
| ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
fa4d6ec97b
bench: Update span-serialize comment
Commit faecca9a85c1c1917d024f55cca34d19cc94f3b9 changed the type of
block413567 from vector to span, but forgot to update the comment. Do it
now.
ffff4a293a
maflcko force-pushed
on Mar 12, 2025
maflcko requested review from l0rinc
on Mar 17, 2025
l0rinc
commented at 12:53 pm on March 17, 2025:
contributor
Rebased it against latest master, ran all tests locally, searched for remaining \bSpan\b and compared to previously acked fab51b26d68b37f5914314ad29eeb930538ea7ae the only diff was @theuni’s comment.
0---
1diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
2--- a/src/test/util_tests.cpp(revision dc8b8e9e3340daf058a9f00afb66550cfe4f70b1)
3+++ b/src/test/util_tests.cpp(date 1742215795870)
4@@ -160,8 +160,8 @@
5 BOOST_CHECK_EQUAL_COLLECTIONS(hex_literal_span.begin(), hex_literal_span.end(), expected.begin(), expected.end());
6 7 const std::vector<std::byte> hex_literal_vector{operator""_hex_v<util::detail::Hex(HEX_PARSE_INPUT)>()};
8- hex_literal_span = MakeUCharSpan(hex_literal_vector);
9- BOOST_CHECK_EQUAL_COLLECTIONS(hex_literal_span.begin(), hex_literal_span.end(), expected.begin(), expected.end());
10+ auto hex_literal_vec_span = MakeUCharSpan(hex_literal_vector);
11+ BOOST_CHECK_EQUAL_COLLECTIONS(hex_literal_vec_span.begin(), hex_literal_vec_span.end(), expected.begin(), expected.end());
1213 constexpr std::array<uint8_t, 65> hex_literal_array_uint8{operator""_hex_u8<util::detail::Hex(HEX_PARSE_INPUT)>()};
14 BOOST_CHECK_EQUAL_COLLECTIONS(hex_literal_array_uint8.begin(), hex_literal_array_uint8.end(), expected.begin(), expected.end());
15Index: src/span.h
16===================================================================
17diff --git a/src/span.h b/src/span.h
18--- a/src/span.h(revision dc8b8e9e3340daf058a9f00afb66550cfe4f70b1)
19+++ b/src/span.h(date 1742215795870)
20@@ -105,7 +105,7 @@
21 concept BasicByte = requires { UCharCast(std::span<B>{}.data()); };
2223 // Helper function to safely convert a span to a span<[const] unsigned char>.
24-template <typename T, size_t N> constexpr auto UCharSpanCast(std::span<T, N> s) -> std::span<std::remove_pointer_t<decltype(UCharCast(s.data()))>> { return {UCharCast(s.data()), s.size()}; }
25+template <typename T, size_t N> constexpr auto UCharSpanCast(std::span<T, N> s) { return std::span<std::remove_pointer_t<decltype(UCharCast(s.data()))>, N>{UCharCast(s.data()), s.size()}; }
2627 /** Like the std::span constructor, but for (const) unsigned char member types only. Only works for (un)signed char containers. */
28 template <typename V> constexpr auto MakeUCharSpan(const V& v) -> decltype(UCharSpanCast(std::span{v})) { return UCharSpanCast(std::span{v}); }
The remaining static extent changes can be done in follow-ups.
Let’s get this merged! \:D/
reACKffff4a293ad878494e12f8f00108cc99ee2b713e
DrahtBot requested review from laanwj
on Mar 17, 2025
maflcko removed review request from theuni
on Mar 18, 2025
maflcko requested review from theuni
on Mar 18, 2025
theuni approved
theuni
commented at 9:52 pm on March 19, 2025:
member
Thanks for addressing my comment.
I agree, this will be a fun one to get merged. I have several local branches with fun changes on top of this one.
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2025-04-01 00:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
