Use explicit casting in cuckoocache's compute_hashes(...) to clarify integer conversion #12770

Use explicit casting in cuckoocache's compute_hashes(...) to clarify integer conversion.

I discussed this code with the code's author @JeremyRubin who suggested patching it to avoid any confusion.

At least one static analyzer incorrectly warns about a shift past bitwidth (UB) here, so this patch will help avoid confusion for human reviewers and static analyzers alike :-)

utACK https://github.com/bitcoin/bitcoin/pull/12770/commits/b46d077851047929ca6f49401a124ff69ed9f7e0

b46d077 looks fine, but shouldn't the static analyser be fixed if this is an incorrect warning?

@JeremyRubin do you want to ACK this?

@MarcoFalke Thanks for the utACK. Yes, I'll report this to the static analyser authors.

Thanks. Looks like this can be closed then.

utACK b46d077.

I don't think this changes any behavior, but I think it might be more clear to apply the cast as such:

             (uint32_t)(((uint64_t)hash_function.template operator()<2>(e) * (uint64_t)size) >> 32),

@MarcoFalke I think it's fine to do this even if it is technically a linter bug. It took me a few minutes to read specs of operators, integer conversion hierarchies, and such to be sure that the behavior was correct.

Updated: Now applying the cast as @JeremyRubin suggested.

Please re-review :-)

utACK 9142dfea8181c6649c8d6a8775d53bc3e14de847

@JeremyRubin @dcousens @promag @donaloconnor Would you mind re-reviewing? :-)

utACK https://github.com/bitcoin/bitcoin/commit/9142dfea8181c6649c8d6a8775d53bc3e14de847 :-)

If you check the blame, you actually want @sipa to sign off on this one (he wrote that specific section).

utack otherwise