2

I have the following simple expression in my Catch Testframework. Ideally, the test should issue a warning, if the test fails.

Unfortunately, Catch is not able to compile the following code fragment:

#define CATCH_CONFIG_MAIN
#include "catch2.hpp"

TEST_CASE("Simple") {
    int a = 4;
    int b = 1;
    CHECK(a == 5 || b == 2);
}

Visual Studio 2015 issues the following error:

error C2676: Binary operator "||": "const Catch::BinaryExpr<LhsT,const int &>" does not define this operator or a conversion for this operator of to suitable type

I would expect something like the following:

4==5 || 1==2 => false || false => false

Is this possible with Catch or do I have to use extra parantheses:

#define CATCH_CONFIG_MAIN
#include "catch2.hpp"

TEST_CASE("Simple") {
    int a = 4;
    int b = 1;
    CHECK((a == 5 || b == 2));
}
Aleph0
  • 5,816
  • 4
  • 29
  • 80

2 Answers2

4

If you read the docs you will find a section saying:

CHECK( a == 2 || b == 1 ); This expression is too complex because of the || operator. If you want to check that one of several properties hold, you can put the expression into parenthesis (unlike with &&, expression decomposition into several CHECKs is not possible).

Also note that...

I would expect something like the following:

4==5 || 1==2 => false || false => false

... your expectations are slightly off. || is short-circuited, meaning that if the first operand evaluates to false, the second is not evaluated.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
  • Many thanks. I should have read the docs beforehand. So I have to rely on the parentheses solution. Also the comment on the short-circuited operator is perfectly understandable. – Aleph0 May 03 '18 at 08:43
  • btw I dont buy their argument of `||` being "too complex", it is rather the limitations of macros that make a workaround necessary – 463035818_is_not_an_ai May 03 '18 at 08:45
  • @Aleph0 even if you got the solution, please add the error message to your question. It will help others that encounter the same problem. Error message are usually easier to search&find than "cannot compile" ;) – 463035818_is_not_an_ai May 03 '18 at 08:48
  • @user463035818: CATCH does *magic* to allow to display nice message for `CHECK(a == b)` (displaying values of `a` and `b`) by introducing object to store operands and operators.Short-circuit and operator precedence make it `||` too complex for CATCH. – Jarod42 May 03 '18 at 09:39
  • @Jarod42 hm I understand, it was just a random macro rant and I never used CATCH myself. Actually after reading the error message, i have to admit that it looks like they did a nice job ;) – 463035818_is_not_an_ai May 03 '18 at 09:46
  • I explained the reasons for the limitation on my answer. – Xarn May 04 '18 at 08:50
2

Catch Classic used to provide somewhat better error, as it would error out during instantion of a type named something along the lines of CATCH_STATIC_ASSERT_EXPRESSION_TOO_COMPLEX_REWRITE_..., but apparently that went away during the rewrite of expression capture layer.

Anyway, the simple answer was already provided by user463035818, enclose the expression in parentheses. I want to expand on why expressions containing operators && or || are not supported.

There are three parts to why && and || operators are not supported, two of them are practical, and one is philosophical.

1) The TMP magic used to decompose the expression and pretty-print it cannot properly emulate short-circuiting behaviour of the built-in operators. This means that the behaviour of such expressions would be different from user's expectations.

2) The already mentioned TMP magic is fairly expensive in regards to compile times, even though we have specialized paths for unary and binary expressions. Having a general path for variadic expressions would likely end up being prohibitively expensive.

3) The use of || in an assertion is usually a code (test) smell. In a reasonable unit test, you should be able to express the expected result exactly, and for cases where this is not true (e.g. iterating through an unordered set), logical or still isn't a good tool solution.

Note 1: Using && (logical and) in a unit test is completely reasonable and in fact it is fairly trivial to rewrite an expression using && to compile and still shortcircuit:

REQUIRE((expr1 && expr2 && expr3));

REQUIRE(expr1);
REQUIRE(expr2);
REQUIRE(expr3);

Note 2: This started out as a comment to the other answer, but then it went over the allowed length and I noticed that the error message for Catch2 is indeed bad, so I opened up an issue.

Xarn
  • 3,460
  • 1
  • 21
  • 43