RFC: Add operator""_uint256 compile-time user-defined literal
#31991
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31991. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
l0rinc
force-pushed
the
l0rinc/implicit_hex_to_uint256
branch
from
6a07360 to
b340e03
Compare
l0rinc
changed the title
_hex to uint256 and remove consteval_ctoroperator""_uint256 compile-time user-defined literal
l0rinc
force-pushed
the
l0rinc/implicit_hex_to_uint256
branch
from
b340e03 to
fbbdbaa
Compare
l0rinc
force-pushed
the
l0rinc/implicit_hex_to_uint256
branch
from
fbbdbaa to
1d06571
Compare
Co-authored-by: Hodlinator <[email protected]>
l0rinc
force-pushed
the
l0rinc/implicit_hex_to_uint256
branch
from
1d06571 to
8c27aaf
Compare
I think the upstream bug link may be wrong. https://developercommunity.visualstudio.com/t/Can-not-initialize-array-with-consteval/10828473 looks closer |
maflcko
left a comment
maflcko
left a comment
There was a problem hiding this comment.
tend toward NACK for now, as I don't see the goal or benefit
Would be happy to ack a doc fix of the upstream msvc bug report
| consensus.nSubsidyHalvingInterval = 210000; | ||
| consensus.script_flag_exceptions.emplace( // BIP16 exception | ||
| uint256{"00000000000002dc756eebf4f49723ed8d30cc28a5f108eb94b1ba88ac4f9c22"}, SCRIPT_VERIFY_NONE); | ||
| "00000000000002dc756eebf4f49723ed8d30cc28a5f108eb94b1ba88ac4f9c22"_uint256, SCRIPT_VERIFY_NONE); |
There was a problem hiding this comment.
Not sure what the goal here is, or the improvement. Looks like extra code is added to do the same thing in two different ways?
Yes, it seems like our issue was closer this "sub-issue" in the currently linked issue: https://developercommunity.visualstudio.com/t/consteval-conversion-function-fails/1579014#T-N10550785 and they apparently only fixed the primary issue. :\ |
4 participants
Follow-up to Add mainnet assumeutxo param at height 880,000, looking to simplify specifying explicit hashes in the source code.
It's not a full implementation yet, I'm looking for concept ack/nack and full CI feedback for now - applying it to the whole codebase may follow based on that.
This is also a follow-up to refactor: Replace ParseHex with consteval ""_hex literals, meant to simplify the usages (values beginning with the hexadecimal data, followed by the type conversion, similarly to
sizeof(uint256)vsuint256::size()) and unifying hex parsing code.Similarly to
""_hex, the""_uint256has the same compile-time constraints as its constructor (properly sized with actual hexadecimal values), but usesdetail::Hexparser (reversed) (used in most other places for hex parsing) instead ofbase_blob(std::string_view hex_str).The new implementation also makes it obvious that the values are reversed in this case.
I thought of removing the
consteval_ctorhack as well, now that consteval conversion is claimed to work in visual studio, but since https://godbolt.org/z/j837req5h still seems to fail for latestx64 msvc v19, I postponed doing that.