The unit test is probably best converted to Catch if we're going to mess with it -- that shouldn't mean much more than removing the driver main function, wrapping the whole thing in a TEST_CASE block, and replacing all assert with REQUIRE. See https://github.com/philsquared/Catch/blob/master/docs/tutorial.md if you're not familiar.

The remove-return change looks reasonable, but rather than do half a job let's replace the other 8 asserts in that file if we're doing one.

The point of the PR is to enable a CI build with disabled assertions/invariants etc. Converting the unit test to Catch is out of scope. The same holds for the change to remove_returns.cpp: it fails to compile with clang++ -DNDEBUG, this PR fixes that. Changing assertions to invariants everywhere is worth doing, but not the purpose of this PR.

Suggest replacing with #1537 and #1538 which implement remove-returns assert->invariant and Catch-ifying the sharing-node test respectively.

I agree. This PR is now obsolete.

Great, I will ping you when they are merged (hopefully hours not days :)) Would you mind checking that the build is successful with develop+1537+1538 in your configuration?

@andreast271 those two are both now merged; latest develop should work as you intend.

@smowton thank you very much. I replied to your comment from 10:50 GMT because I saw a problem. I guess you never got that message ...

I don't see anything here between my comment 10:50 and my comment 13:15, no. Repeat it?