Page MenuHomePhabricator

Merge #13534: Don't assert(foo()) where foo() has side effects

Authored by nakihito on Dec 16 2019, 21:06.



6ad0328f1c Don't assert(foo()) where foo has side effects (practicalswift)

Pull request description:

Don't `assert(foo())` where `foo` has side effects.

From `assert(3)`:

> If the macro `NDEBUG` is defined at the moment `<assert.h>` was last included, the macro `assert()` generates no code, and hence does nothing at all.

Bitcoin currently cannot be compiled without assertions, but we shouldn't rely on that.

Tree-SHA512: 28cff0c6d1c2fb612ca58c9c94142ed01c5cfd0a2fecb8e59cdb6c270374b215d952ed3491d921d84dc1b439fa49da4f0e75e080f6adcbc6b0e08be14e54c170

Partial backport of Core PR13534

Test Plan
make check

Diff Detail

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Owners added a reviewer: Restricted Owners Package.Dec 16 2019, 21:06

Review note: and were excluded because we do not have that code in our code base. The former was removed in D3136 and the latter was not backported.

Fabien added inline comments.
58 ↗(On Diff #14882)

nit: rename ret => checked to avoid merge conflicts

This revision is now accepted and ready to land.Dec 17 2019, 10:32