build: Enable C++11 in build, require C++11 compiler

laanwj commented at 2:17 pm on December 3, 2015: member

Implements #6211.

Just meant for testing for now. Probably doesn’t pass Travis.

I’m running into an issues with boost 1.55 locally: https://svn.boost.org/trac/boost/ticket/10038

0/usr/include/boost/filesystem/operations.hpp:381: undefined reference to `boost::filesystem::detail::copy_file(boost::filesystem::path const&, boost::filesystem::path const&, boost::filesystem::copy_option, boost::system::error_code*)'

TODO:

Travis fails on “This is not a C++11 compiler”

Possibly TODO:

Make sure Boost/Qt/BDB++ is compiled with c++11 in depends?

laanwj added the label Build system on Dec 3, 2015

jtimon commented at 0:55 am on December 4, 2015: contributor

Concept requete-ACK, of course, I don’t think anybody will dare to nack this (besides tested nits [and making travis happy]).

dcousens commented at 1:17 am on December 4, 2015: contributor

concept ACK

droark commented at 11:37 pm on December 14, 2015: contributor

Concept ACK.

luke-jr commented at 1:31 am on December 15, 2015: member

Concept ACK, but only when it’s determined to be safe (especially against possibly invisible consensus failures) and can be supported on all stable distros with standard OS libraries…

dcousens commented at 2:05 am on December 15, 2015: contributor

on all stable distros

For reference, that list of distros:

Arch
Debian
Fedora
Gentoo stable
OS X
RHEL
Slackware
Ubuntu

Those ticked have native support of GCC ^5.1.y in the latest stable release (AFAIK). Please let me know if any corrections need to be made.

This doens’t necessarily act as a block, assuming #6211 (comment)

droark commented at 2:14 am on December 15, 2015: contributor

@dcousens - Don’t forget Windows (well, MinGW)! I guess this one’s a little tricky since the Windows build is cross-compiled. I think Ubuntu 14.04 will be fine, although 16.04 (LTS) will be necessary if GCC 5 features are required, I suppose, due to Gitian requirements.

dcousens commented at 2:16 am on December 15, 2015: contributor

Maybe throw the switch once 16.04 (LTS) comes out and somebody can get Gitian to play nice with it?

That sounds OK to me, it seems like a realistic timeline too.

droark commented at 2:19 am on December 15, 2015: contributor

That sounds OK to me, it seems like a realistic timeline too. @dcousens - Sorry for editing what you replied to. :) But yeah, maybe aim for throwing the switch around the time 16.04 comes out if, for whatever reasons, it’s decided that GCC 5 support is required/highly desirable. 15.10 has it but I think Gitian officially supports only LTS releases.

luke-jr commented at 4:03 am on December 15, 2015: member

~~Fedora can’t be fully supported due to the ECDSA mess they’ve made. Disagree with it not being a blocker.~~ Is GCC 5 now known to solve all the ABI issues, or at least the ones that could potentially affect Core?

Edit: Forgot we switched to libsecp256k1, nevermind re Fedora.

laanwj commented at 2:23 pm on December 17, 2015: member

Ok, getting kind of tired of this, but postponing this another major release, I’m not going to push for this again.

laanwj closed this on Dec 17, 2015

jgarzik commented at 2:24 pm on December 17, 2015: contributor

@luke-jr No reason why Fedora cannot be supported. A fedora programmer added the following commit to picocoin, illustrating a Fedora-compatible approach: https://github.com/jgarzik/picocoin/pull/44

sipa commented at 3:50 pm on December 17, 2015: member

@jgarzik That seems unrelated to C++11?

nathan-at-least commented at 4:16 pm on December 17, 2015: none

I’m working on a fork off of Bitcoin core to implement zerocash. We require C++11 due to dependencies (eg libsnark).

It’s not much effort to make C++11 compile, link, run, and pass make check. Hidden changes to consensus are another matter which we don’t have a solid grasp on.

We’ve been working on a fork off of both v0.10.0 and v0.11.2 with g++ -std=c++11. This has required a few modifications to the upstream bitcoin code, different in each of these branches. Here are some notes (though I may be forgetting details).

First, some strong caveats:

Our only testing of consensus changes has been make check, so the following notes are only a starting point for C++11 support in bitcoin core. (Edit: -because we haven’t done rigorous consensus regression testing. Any advice here would be greatly appreciated.)
On top of this, on our branch atop v0.10.0, we disabled many tests that had hard-coded test vectors (such as serialized structures from production main-net) rather than correctly fixing the tests.
On v0.11.2 we have not disabled any upstream tests, and have only added C++11 mode and linked in our extra dependencies. (The v0.10.0 is a messy proof of concept. Now we’re starting fresh with more rigour.)
We only test/develop on debian or arch linux with non-“default’ packages installed. Furthermore we only build with the depends system, something close to make -C ./depends NO_QT=1 && ./autogen.sh && ./configure --prefix=./depends/… --with-gui=no CXXFLAGS='-O0 -g' && make V=1
No one on our team has a strong C++ background. :-<

So, I scanned through our deltas from upstream to hunt down changes I believe are necessitated by C++11 support:

We’ve found that upgrading to boost 1.57.0 atop v0.10.0, or to either boost 1.57.0 or 1.59.0 atop bitcoin v0.11.2 resolves the linker error in @laanwj ’s original comment.
- The patches in ./depends/patches/boost do not apply to boost 1.57.0 (and we haven’t tried them 1.59.0). I examined the 1.57.0 code to see if I could update the patches, but couldn’t find relevant target code after about an hour of effort, so I gave up and deleted these patches! (I asked in #bitcoin-dev about this and at least one person told me that’s probably ok because upstream probably addressed the same patched issue.)
Boost’s list_of(x)(y)(z) syntax didn’t work (for boost 1.57.0 atop bitcoin v0.10.0), so we switched to C++11 initializer syntax { x, y, z }.
In bitcoin v0.11.2 there is one unit test that relies on exceptions propagating out of a dtor: src/test/reverselock_tests.cpp L52-L58 at 7e278929. In C++11 this causes a process to abort unless you explicitly add an attribute to the dtor as in ~reverse_lock() noexcept(false) {…}.

I hope this helps. If I had time I’d work on this PR directly, but I can’t promise anything.

jgarzik commented at 4:18 pm on December 17, 2015: contributor

@sipa It is directly related to luke’s comment.

sipa commented at 4:21 pm on December 17, 2015: member

@jgarzik Oops, I missed that. @luke-jr That’s irrelevant. We don’t use OpenSSL’s EC code anywhere anymore.

I am really in favor of C++11 and don’t understand the fuss at this point. There are potential problems when mixing C++ code compiled against different versions of the std library, but as far as I can tell, those are things that result in link-time errors?

I would like to see this reopened.

jgarzik commented at 7:11 pm on December 17, 2015: contributor

Agree - urge re-opening this - this is not a big risk.

laanwj reopened this on Dec 17, 2015

droark commented at 9:25 pm on December 17, 2015: contributor

Thanks for reopening, @laanwj.

One thing I would like to say is that some IBLT simulation code (non-Core) was written in C++11. I believe the plan is to try to recycle some of it for the eventual Core commit. It would be nice to know if C++11 will be acceptable so that everybody working on the Core patch(es) can use or modify code as necessary.

Also, I agree with @sipa. AFAIK, any library issues would come up as linker errors. I’m not aware offhand of any massive gotchas that would come with a switch to GCC 5 and a bit of time for distros to recompile everything. (AFAIK, Ubuntu did most of that before releasing 15.10, although it sounds like more work is needed for 16.04.) If anybody’s aware of any specific issues, I’m all ears.

sipa commented at 9:50 pm on December 17, 2015: member

This was discussed in the weekly IRC meeting. It seems we’ll aim for C++11 in 0.13, by first adding build configuration and testing for C++11 on some targets (but not all), and if no problems appear, switch everything to C++11 and start using C++11 features.

theuni commented at 9:51 pm on December 17, 2015: member

Looks like the only remaining issue in that list should be the throwing dtor.

nathan-at-least commented at 0:10 am on December 18, 2015: none

The patch for the dtor “fix” is:

 0diff --git a/src/reverselock.h b/src/reverselock.h
 1index 567636e..db5c626 100644
 2--- a/src/reverselock.h
 3+++ b/src/reverselock.h
 4@@ -17,7 +17,7 @@ public:
 5         lock.unlock();
 6     }
 7
 8-    ~reverse_lock() {
 9+    ~reverse_lock() noexcept(false) {
10         lock.lock();
11     }

This allows reverselock_error test to pass (link in my prior comment). However, it makes me uncomfortable because I don’t understand if both enabling c++11 and adding this attribute will lead to identical behavior as before.

This patch is enabling a mechanism used for test verification, but we may not want that behavior in the application code. The only use I see is in src/scheduler.cpp L73 at cd3f12c6 (latest on branch master).

jtimon commented at 5:21 am on December 18, 2015: contributor

c++11 for 0.13? I should miss more meetings so things go more my way…

laanwj added the label Refactoring on Feb 16, 2016

jtimon commented at 6:47 pm on March 16, 2016: contributor

what’s the latest status on this?

laanwj commented at 7:20 pm on March 16, 2016: member

what’s the latest status on this? @theuni has been chasing Travis about dependency caching support on the VMs that support c++11 (Ubuntu 14.04+) We could in principle do without it, disabling qt builds - or using the distro’s native qt development headers, but if possible it’d be preferable to keep using the current system. That’s what holding it up.

MarcoFalke commented at 12:20 pm on April 17, 2016: member

Wouldn’t you need to bump travis to trusty as part of this pull?

laanwj commented at 10:43 am on April 18, 2016: member

@theuni is working on that

laanwj added this to the milestone 0.13.0 on Apr 19, 2016

MarcoFalke commented at 11:47 am on April 26, 2016: member

Travis failure:

0configure: error: *** A compiler with support for C++11 language features is required.

Maybe retrigger after the cache for master is populated? Otherwise a rebase/fresh commit is needed because travis is not building against master.

build: Enable C++11 build, require C++11 compiler

Implements #6211.

67969af09f

laanwj force-pushed on Apr 26, 2016

laanwj commented at 2:15 pm on April 26, 2016: member

Okay, rebased to master.

theuni commented at 9:31 pm on April 26, 2016: member

@laanwj https://github.com/theuni/bitcoin/commit/deaf59fdf658100be8472d261615241ba22a055c Should get this up and going. clang needs an explicit -stdlib=libc++ for darwin.

depends: use c++11 a398549b3b

laanwj commented at 10:47 am on April 27, 2016: member

@theuni thanks, cherry-picked

theuni commented at 4:39 pm on April 27, 2016: member

Woohoo, ACK!

droark commented at 4:42 pm on April 27, 2016: contributor

Nice! Code looks good to me. Untested ACK.

sipa commented at 4:59 pm on April 27, 2016: member

utACK https://github.com/bitcoin/bitcoin/commit/a398549b3bfc976f1b6408f41af7dac3514e2553

paveljanik commented at 5:45 pm on April 27, 2016: contributor

ACK https://github.com/bitcoin/bitcoin/pull/7165/commits/a398549b3bfc976f1b6408f41af7dac3514e2553

configure output diff on OS X:

0+checking whether g++ supports C++11 features by default... no
1+checking whether g++ supports C++11 features with -std=c++11... yes

and the build log contains millions of added -std=c++11 as expected.

TheBlueMatt commented at 7:58 pm on April 27, 2016: member

No longer the latest version of the C++11 macro (see http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=history;f=m4/ax_cxx_compile_stdcxx.m4). Do we want to update?

droark commented at 9:09 pm on April 27, 2016: contributor

Tested ACK with the latest C++11 macro, as pointed out by Matt. Compiles fine on the latest OS X (10.11.4) and Xcode (7.3), make check passes, and I get the same output as Janik.

randy-waterhouse commented at 0:21 am on April 28, 2016: contributor

ACK. builds, tests, runs.

Throws a new build warning “httpserver.cpp:255:36: warning: ‘auto_ptr’ is deprecated …”

paveljanik commented at 5:07 am on April 28, 2016: contributor

@randy-waterhouse What compiler do you use please?

Edit: Travis has the warning here: https://travis-ci.org/bitcoin/bitcoin/jobs/126071750#L3267

theuni commented at 5:20 am on April 28, 2016: member

@paveljanik That’s a legitimate warning. We’ll need to auto_ptr -> unique_ptr. But that’s a hard fork :p

fanquake commented at 7:12 am on April 28, 2016: member

Tested on OSX 10.11.4 & Xcode 7.3. compiles fine, make check passes. Same configure diff as above.

build: update ax_cxx_compile_stdcxx to serial 4 2aacc72727

laanwj renamed this:
~~WIP: build: Enable C++11 in build, require C++11 compiler~~
build: Enable C++11 in build, require C++11 compiler
on Apr 28, 2016

laanwj commented at 8:18 am on April 28, 2016: member

removed WIP: tag, added commit that updates ax_cxx_compile_stdcxx (thanks @TheBlueMatt for noticing) @randy-waterhouse Indeed, the auto_ptr should be replaced, let’s do that in a later pull.

paveljanik commented at 8:39 am on April 28, 2016: contributor

reACK https://github.com/bitcoin/bitcoin/pull/7165/commits/2aacc72727b891aa6f42e97d16e8954fba014703

doc: Add note about new build/test requirements to release notes

[skip ci]

7df92242a9

laanwj force-pushed on Apr 28, 2016

laanwj merged this on Apr 28, 2016

laanwj closed this on Apr 28, 2016

laanwj referenced this in commit 06162f19d7 on Apr 28, 2016

zkbot referenced this in commit 33f7145fb2 on Nov 29, 2017

zkbot referenced this in commit b2399c1951 on Nov 30, 2017

zkbot referenced this in commit d3ca2706a8 on Nov 30, 2017

zkbot referenced this in commit 02c4467cb5 on Nov 30, 2017

fanquake referenced this in commit e727c2bdca on May 4, 2020

sidhujag referenced this in commit fae55b66f9 on May 5, 2020

fanquake referenced this in commit 3af495d697 on Sep 2, 2021

sidhujag referenced this in commit 1dedbbcb8b on Sep 2, 2021

MarcoFalke locked this on Sep 8, 2021

build: Enable C++11 in build, require C++11 compiler #7165