Fixes when compiling in c++11 mode #5509

pull theuni wants to merge 5 commits into bitcoin:master from theuni:c++11-fixes changing 6 files +724 −719

theuni commented at 2:26 AM on December 19, 2014: member
Follow-up of #5415. Fixes builds like: ./configure CXXFLAGS="-std=c++11" No rush, I just wanted to get these fixed up while they were fresh in my mind. Fixes all known (to me) build issues except the boost issue mentioned below, tested with several compilers/stdlibs.
- Fix a boost incompatibility with c++11 and list_of
- Don't forward-declare std::map types. See: http://llvm.org/bugs/show_bug.cgi?id=13471
- Watch out for user-defined-literal syntax in tests
There's also an annoying boost bug that causes problems with copy_file on some platforms: https://svn.boost.org/trac/boost/ticket/10038

It's a bit more complicated to fix, so I'm leaving that as a separate issue. Ideally we could work around it by just using a different function. As a temporary workaround, adding BOOST_NO_SCOPED_ENUMS to CPPFLAGS works.

As mentioned in a commit message, the MOVEONLY commits are very simple despite what the nasty git diffs show, it's just moved code.
fanquake commented at 11:35 AM on December 19, 2014: member

Tested on OSX with boost 1.56.0 & ./configure CXXFLAGS="-DDEBUG_LOCKORDER -g -std=c++11" compiling now works fine.

Verified that 9fbeea7d5b0a3505f173f690d1b0f677113ffeed & 6362b7799628a2bfcfbf99ecdfcf8289fe488e53 are indeed MOVEONLY.
fanquake commented at 11:13 AM on December 23, 2014: member

Also tested with with Boost 1.57.0
laanwj added the label Build system on Jan 6, 2015
laanwj commented at 2:25 PM on January 6, 2015: member

utACK, but needs rebase
theuni commented at 7:08 PM on January 6, 2015: member

@laanwj Ok to rebase here after #5513 is pulled in, since that will conflict here? That'd spare a move-only verification.
theuni force-pushed on Jan 7, 2015
theuni commented at 2:03 AM on January 7, 2015: member

Thanks! Rebased after #5513. Confirmed still working on osx 10.9 with libc++.
laanwj added the label Improvement on Jan 8, 2015
fanquake commented at 9:38 AM on February 2, 2015: member

Ping @theuni. Needs rebase.
build: fix newer boost build with c++11 a2b04ddfe6

c++11: MOVEONLY: move function definitions out of the header

These need to be moved out of the header in order to resolve a circular
dependency between CWallet and CTxWallet. See next commit.

bbacd88204

c++11: MOVEONLY: break circular dependency in wallet

c++11 (libc++'s stdlib implementation anyway) doesn't allow for map types to be
forward-declared. for example:

class foo;
std::map<int, foo> bar; // error, foo has not been defined.
class foo{};

Since CWallet and CWalletTx are inter-dependent, but only std::map<*,CWalletTx>
is used, forward-declare CWallet instead and define CWalletTx first.

Despite the mangled git diff, this change only amounts to moving ~320 lines in
a single chunk.

3447cf87e2

c++11: don't forward-declare types used in maps 5a6155ccfa
c++11: These look like user-defined literals.
Add a space to keep the compiler happy
7f991e364e
theuni force-pushed on Feb 4, 2015

in src/chainparams.cpp:None in 7f991e364e

 162 | -        base58Prefixes[EXT_PUBLIC_KEY] = boost::assign::list_of(0x04)(0x88)(0xB2)(0x1E);
 163 | -        base58Prefixes[EXT_SECRET_KEY] = boost::assign::list_of(0x04)(0x88)(0xAD)(0xE4);
 164 | +        base58Prefixes[PUBKEY_ADDRESS] = std::vector<unsigned char>(1,0);
 165 | +        base58Prefixes[SCRIPT_ADDRESS] = std::vector<unsigned char>(1,5);
 166 | +        base58Prefixes[SECRET_KEY] =     std::vector<unsigned char>(1,128);
 167 | +        base58Prefixes[EXT_PUBLIC_KEY] = boost::assign::list_of(0x04)(0x88)(0xB2)(0x1E).convert_to_container<std::vector<unsigned char> >();

laanwj commented at 11:10 AM on February 4, 2015:

I'm terrified. This makes me yearn for the old days:

static const uint8_t ext_public_key[] = {0x04, 0x88, 0xB2, 0x1E};
base58Prefixes[EXT_PUBLIC_KEY] = std::vector<uint8_t>(ext_public_key, ext_public_key+sizeof(ext_public_key));

Boost doesn't add anything of value here. (edit: heck, or even just four push_backs, it's four bytes we're talking about...)

theuni commented at 8:07 PM on February 4, 2015:

@laanwj I was trying to keep the changes to a minimum here for the sake of review, but I very much agree that this is ugly as hell and completely unnecessary. I have changes in a refactor branch that remove boost here altogether and make this much more straightforward. Would you like me to go ahead and pull those out and PR them?

in src/wallet.cpp:None in 7f991e364e

1014 | @@ -1015,6 +1015,193 @@ set<uint256> CWalletTx::GetConflicts() const
1015 |      return result;
1016 |  }
1017 |  
1018 | +CAmount CWalletTx::GetDebit(const isminefilter& filter) const

laanwj commented at 11:23 AM on February 4, 2015:

Yes, thanks for moving this to the .cpp file, all that code doesn't belong in the header in the first place.

laanwj commented at 11:48 AM on February 4, 2015: member

Verified moveonly the MOVEONLY commits
laanwj merged this on Feb 4, 2015
laanwj closed this on Feb 4, 2015
laanwj referenced this in commit 31dedb463b on Feb 4, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

theuni

fanquake

laanwj

Labels

Refactoring Build system

Linked (view graph)

#5415 build: fix build with newer boost and c++11