Page MenuHomePhabricator

Don't assert(...) with side effects

Authored by Fabien on Jun 10 2019, 13:09.



This diffs fixes the assertions introduced in D3232 and adds a linter to
avoid reproducing the issue.

The fix is a backport of core PR14088
The linter is based on the second commit, but integrated into arcanist.

Test Plan
./src/test/test_bitcoin -t scheduler_tests

Revert the changes in scheduler_tests and run:

arc lint -- src/test/scheduler_tests.cpp

The linter should catch the errors

Diff Detail

rABC Bitcoin ABC
Lint Not Applicable
Tests Not Applicable

Event Timeline

jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
63 ↗(On Diff #9289)

Nit: It may improve this message to refer to what the original comment did in test/lint/

# PRE31-C (SEI CERT C Coding Standard):
# "Assertions should not contain assignments, increment, or decrement operators."
This revision is now accepted and ready to land.Jun 10 2019, 16:43

Rebase and update error message.