Document sizeof(size_t) assumptions and compiler assumptions in assumptions.h #15522

Document sizeof(size_t) assumptions and compiler assumptions by adding compile-time checks in assumptions.h.

utACK c7ea8d3236e7c1b0c198345cc78a6754338d3724

utACK c7ea8d3236e7c1b0c198345cc78a6754338d3724 (I don't think these are necessary assumptions, you're supposed to use uintptr_t if you want an integer type the same size as a pointer, but ok we do make it in practice)

@scravy @laanwj Added compiler assumptions for completeness. Please re-review :-)

I am not sure that the C++11 assumption is needed. The assumptions in here are about the platform, i.e. hardware, system configuration, etc.

Whether bitcoin is built using C++11 or not is pretty much up to the build system, not so much to the underlying platform (whereas I might just be able to configure/make on some alien system violating assumptions about certain types).

I'm not fundamentally opposed it, but I also do not see any necessity for it.

@scravy When adding assumptions.h the goal was to document all tacit assumptions we make that don't follow directly from the standard used. Not specifying the standard was just an oversight from my part -- it should have been there from the beginning :-)

A thought experiment that can be made when considering if an assumption should be specified explicitly:

• Could it help sound static analysis or other types of formal analysis if an assumption we make was specified explicitly in assumptions.h?

If the answer is yes the assumption should probably be explicitly stated in assumptions.h :-)

I am not sure that the C++11 assumption is needed. The assumptions in here are about the platform, i.e. hardware, system configuration, etc.

I tend to agree. I like documenting specific, concrete assumptions here about platforms and environments. Asserting the C++11 macros seems too sweeping and general. Is there any specific reason bitcoind cannot be built with C++14 or 19 mode if so desired? Will this result in expected bugs besides "we haven't tested it"? If not, let's not include it (have retracted my ACK for now).

@laanwj I've now changed to __cplusplus >= 201103L to make sure the compiler supports "at least" the final version of C++11. This will allow for building also in say C++14 or 17 mode. Please re-review :-)

@laanwj An alternative would be to simply issue a warning if a non-C++11 (ISO/IEC 14882:2011) compiler is used:

#if __cplusplus != 201104L
#warning ...
#endif

WDYT? :-)

@laanwj Ping - what is your preference? :-)

FWIW: I've verified that a disassembly of the bitcoind binary built with this patch applied is identical to a disassembly of the bitcoind binary built against master (as expected).

This is fine, let's not have an endless discussion over a pretty insignificant change like this.

FWIW: I've verified that a disassembly of the bitcoind binary built with this patch applied is identical to a disassembly of the bitcoind binary built against master (as expected).

THanks for testing, utACK