Follow-up: Use load_be more efficiently #4085
Conversation
| @@ -420,13 +420,15 @@ void BigInt::assign_from_bytes(std::span<const uint8_t> bytes) { | |||
| secure_vector<word> reg((round_up(full_words + (extra_bytes > 0 ? 1 : 0), 8))); | |||
|
|
|||
| for(size_t i = 0; i != full_words; ++i) { | |||
| reg[i] = load_be<word>(&bytes[length - sizeof(word) * (i + 1)], 0); | |||
| reg[i] = load_be<word>(bytes.last<sizeof(word)>()); | |||
There was a problem hiding this comment.
Hmm so the loaders can omit the index when loading from a span but not an array?
There was a problem hiding this comment.
If by "array" you mean "raw pointer to data", yes. My guess is, that this wasn't possible before the load/store refactoring either. Or perhaps I forgot to translate a legacy overload; but given that the rest of the code base compiled, I doubt that.
There was a problem hiding this comment.
That's true, the index was always required previously. My point is now the behavior is inconsistent depending on the type of argument, which is obviously surprising.
https://en.wikipedia.org/wiki/Principle_of_least_astonishment
There was a problem hiding this comment.
Oh, right. I believe, leaving out the index-overload for ranges was my deliberate decision. I.e. rather set up the range or view accordingly before invoking the loader than pass an offset/index.
On more general terms: The structure of the loaders (with detail::load_any and the constexpr-magic) has the disadvantage that it is fairly hard to see for us which overloads are actually legal. 🤔
|
FWIW: I would hope to eventually purge the raw-pointer overloads altogether; making it consistent again... |
See: #4056 (comment)
Closes #4080