refactor: use braced init for integer literals instead of c style casts #23829

See #23810 for more context. This is broken out from that PR, as it is less breaking, and should be trivial to review and merge.

EDIT: Long term, the intention is to remove all C-style casts, as they can dangerously introduce reinterpret_casts. This is one step which removes a number of trivially removable C-style casts

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See 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.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

Concept ACK. I think this is fine to do because:

• C++11 braced init is safer, since it has more compile time checks
• This patch has no conflicts as of now
• bitcoind is not changed by this patch. I checked clang++ -O2 and g++ -O2 on my system.

Instead of linking to a pull request for context, I think OP can be improved to give a clear motivation. I think this patch is by itself worthwhile and should be considered independent of the other pull request.

review ACK 9db89e1efa4332109eb1cb53e153844797a60350

However, I can also see a point about not changing existing code for style reasons, unless it is touched.

ACK 9db89e1efa4332109eb1cb53e153844797a60350

I agree with the reason given by @MarcoFalke in the above comment.

1. C++11 braced init is safer since it has more compile-time checks
2. This patch has no conflicts as of now

I have reviewed the code, and I think all the appropriate changes are correctly made in this PR.

However, I can also see a point about not changing existing code for style reasons unless it is touched.

It makes sense. But these style changes will need to be done sooner or later. And since there are not many conflicts (as of now) for this PR, I think these changes can be safely integrated into the codebase.

I took another look here and it seems the changes are incomplete. For example, there are a lot of occurrences in the RPC layer:

        ret.pushKV("height", (int64_t)stats.nHeight);
        ret.pushKV("bestblock", stats.hashBlock.GetHex());
        ret.pushKV("txouts", (int64_t)stats.nTransactionOutputs);
        ret.pushKV("bogosize", (int64_t)stats.nBogoSize);

If the changes are made complete here, it will probably be a change too large and not worth it, so I'll downgrade my ACK to a -0.

I took another look here and it seems the changes are incomplete. For example, there are a lot of occurrences in the RPC layer:

        ret.pushKV("height", (int64_t)stats.nHeight);
        ret.pushKV("bestblock", stats.hashBlock.GetHex());
        ret.pushKV("txouts", (int64_t)stats.nTransactionOutputs);
        ret.pushKV("bogosize", (int64_t)stats.nBogoSize);

If the changes are made complete here, it will probably be a change too large and not worth it, so I'll downgrade my ACK to a -0. @MarcoFalke the purpose of this PR is to not be complete :D If it was complete, it would simply be #23810.

The scope of this PR as I understand it (although original title was misleading), was to only apply this to integer literals (I originally said "integer constants").

I could expand the scope to all integer constants, but that would potentially be too breaking (according to what you've said). Let's maybe do that in the future.

@MarcoFalke Did my scope clarification (PR only intended to touch integer literals) clarify your concerns? It seems to me this PR only would cause conflicts in two other PRs, so there being lot's of conflicts isn't a concern, and it shouldn't change the binary.

@MarcoFalke if possible, I'd like to continue moving forward with this. Did you have any concerns I haven't addressed?

I have rebased this PR now that #23438 has been merged. @ajtowns @achow101, this PR will cause a few minor and easily resolvable conflicts in your PR (#24032, #23304). Do either of you have an objection to this PR being merged, and hence requiring a rebase in your PRs?

24032 will need a rebase after #23508 is merged anyway

@PastaPastaPasta are you still working on this?

I'd love for it to get merged. And would be happy to rebase it. But it seems like most maintainers here don't actually want to review / merge it sadly.

Rebased :) Hopefully we can get a few reviews on this simple PR. I have liked seeing in my continued rebases as we are refactoring places where ints were previously used and are now using dedicated types that actually mean something.

ACK f2fc03ec856d7d19a20c482514350cced38f9504 Verified no values are being changed.

Checked that f2fc03ec856d7d19a20c482514350cced38f9504 does not change the binary on my system with my compiler