util: Make Assume() usable as unary expression #21317

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2102-utilAssumeUnary changing 2 files +4 −1

MarcoFalke commented at 3:31 PM on February 28, 2021: member

Assume shouldn't behave different at the call site depending on build flags. Currently compilation fails if it is used as expression. Fix that by using the lambda approach from Assert() without the assert().
MarcoFalke commented at 3:32 PM on February 28, 2021: member

Reported here: #21236 (review)
practicalswift commented at 3:45 PM on February 28, 2021: contributor

Concept ACK
DrahtBot added the label Utils/log/libs on Feb 28, 2021
jnewbery commented at 10:43 AM on March 4, 2021: member

Concept ACK
MarcoFalke force-pushed on Mar 4, 2021
util: Make Assume() usable as unary expression fa4cebadcf
MarcoFalke force-pushed on Mar 4, 2021

in src/test/util_tests.cpp:81 in fa4cebadcf

  75 | @@ -76,6 +76,9 @@ BOOST_AUTO_TEST_CASE(util_check)
  76 |      const int two = *Assert(p_two);
  77 |      Assert(two == 2);
  78 |      Assert(true);
  79 | +    // Check that Assume can be used as unary expression
  80 | +    const bool result{Assume(two == 2)};
  81 | +    Assert(result);

MarcoFalke commented at 11:08 AM on March 4, 2021:

fun fact: I tried to minimize the test by writing Assert(Assume(two == 2)), but that only works in C++20.

hebasto commented at 2:24 PM on October 31, 2021:

The added lines are intended to test Assume, therefore we could s/Assert(result);/BOOST_CHECK(true);/ to suppress "Test case util_tests/util_check did not check any assertions" in

$ ./src/test/test_bitcoin -t util_tests/util_check -l test_suite
Running 1 test case...
Entering test module "Bitcoin Core Test Suite"
test/util_tests.cpp(48): Entering test suite "util_tests"
test/util_tests.cpp(75): Entering test case "util_check"
Test case util_tests/util_check did not check any assertions
test/util_tests.cpp(75): Leaving test case "util_check"; testing time: 9563us
test/util_tests.cpp(48): Leaving test suite "util_tests"; testing time: 9634us
Leaving test module "Bitcoin Core Test Suite"; testing time: 9695us

*** No errors detected

If Assert(result); is really required (but I cannot see reasons), maybe add BOOST_CHECK(true); as it is done here: https://github.com/bitcoin/bitcoin/blob/7efc628539573af4b4a76d93b853cc46e9e52eae/src/test/util_tests.cpp#L102 ?

MarcoFalke commented at 2:31 PM on October 31, 2021:

BOOST_CHECK(result); should also work

hebasto commented at 2:46 PM on October 31, 2021:

Right. I meant it :) My copy-paste error...

jnewbery commented at 10:20 AM on March 17, 2021: member

ACK fa4cebadcffd9112da4b13c7cc7ccf21e2bee887
practicalswift commented at 7:20 PM on March 17, 2021: contributor

cr ACK fa4cebadcffd9112da4b13c7cc7ccf21e2bee887: patch looks correct and commit hash starts with fa
MarcoFalke merged this on Mar 22, 2021
MarcoFalke closed this on Mar 22, 2021
MarcoFalke deleted the branch on Mar 22, 2021
sidhujag referenced this in commit a2d9d9ae65 on Mar 22, 2021
Fabcien referenced this in commit 594794938f on Jan 21, 2022
PastaPastaPasta referenced this in commit e235a5873f on Jun 19, 2022
PastaPastaPasta referenced this in commit 9101e82bf2 on Jun 19, 2022
PastaPastaPasta referenced this in commit 10979f1a74 on Jun 27, 2022
DrahtBot locked this on Oct 31, 2022

Contributors

Labels