Make fixed CAmounts and related sanity function constexpr #11205

pull danra wants to merge 2 commits into bitcoin:master from danra:refactor/constexpr-amount changing 1 files +4 −4

danra commented at 6:25 PM on August 31, 2017: contributor

This would allow the fixed amounts and sanity function to be used in constexpr contexts.
MarcoFalke commented at 8:37 PM on August 31, 2017: member

If we'd really wanted to do that, a project wide diff would be appropriate.
danra commented at 8:53 PM on August 31, 2017: contributor

@MarcoFalke I don't mind doing this project-wide where appropriate, but I'd appreciate pointers to the files where it would be relevant. I believe it would make sense to constexpr-ize conceptual things which it would make sense to test or express more complex concepts at compile time.
fanquake added the label Refactoring on Sep 1, 2017

in src/amount.h:27 in a7bd3b0e72 outdated

  22 | @@ -23,7 +23,7 @@ static const CAmount CENT = 1000000;
  23 |   * critical; in unusual circumstances like a(nother) overflow bug that allowed
  24 |   * for the creation of coins out of thin air modification could lead to a fork.
  25 |   * */
  26 | -static const CAmount MAX_MONEY = 21000000 * COIN;
  27 | -inline bool MoneyRange(const CAmount& nValue) { return (nValue >= 0 && nValue <= MAX_MONEY); }
  28 | +constexpr CAmount MAX_MONEY = 21000000 * COIN;
  29 | +constexpr bool MoneyRange(const CAmount& nValue) { return (nValue >= 0 && nValue <= MAX_MONEY); }

promag commented at 12:07 PM on September 4, 2017:

Rename nValue with amount and remove ()?

promag commented at 12:08 PM on September 4, 2017: member

IMO either mention amount.h or do what @MarcoFalke said.

Make fixed CAmounts and related sanity function constexpr

This would allow the fixed amounts and sanity function to be used in constexpr contexts.

5dc5a4c412

Cleanup MoneyRange() sanity function 2ea55e73e0
danra force-pushed on Sep 4, 2017
laanwj commented at 10:14 PM on September 5, 2017: member

No problem with this, though there seems to be no specific reason for doing this until we need those values in a constexpr context.
MarcoFalke commented at 10:21 PM on September 5, 2017: member

Agree with @laanwj. This is obviously an improvement, but I fail to see the rationale to do this refactoring only in this specific file. This will just cause a bunch of "Make constexpr" commits over the next couple of months.

Either do a scripted diff for the whole project if people agree to do that or leave it until there is an actual use case.
laanwj commented at 9:37 PM on September 7, 2017: member

Closing this, as there is no agreement to do this.
laanwj closed this on Sep 7, 2017
DrahtBot locked this on Sep 8, 2021

Contributors

Labels