Pride and revulsion

Here's a bug that bit me recently. I wrote a very small quick and dirty TDD header for C in my CyberDojo. It has a macro called CHECK_EQ which you use as follows:
CHECK_EQ(int, 42, 9*6);
Part of the macro looks like this:
#define CHECK_EQ(type, expected, actual) \
    ... \
    type e = expected; \
    type a = actual; \
    ... \
Can you see the problem? Well, suppose it's used like this:
    int e = 42;
    CHECK_EQ(int, e, 9*6);
Can you see it now? The problem is that this line in the macro
    type e = expected; \
gets expanded into this line:
    type e = e; \
Ooops. I thought about choosing a very cryptic name instead of x. Perhaps incorporating __LINE__ as a suffix. But I felt there had to be a solution that would always work. After some thought I came up with this. Brace yourself...
#define CHECK_EQ(type, expected, actual) \
   ...
   if (#expected[0] != 'x') \
   { \
       type x0 = expected; \
       type x1 = actual; \
       ... \
   } \
   else \
   { \
       type y0 = expected; \
       type y1 = actual; \
       ... \
   } \
   ...
Like Tom Duff, I feel a mixture of pride and revulsion at this discovery.

8 comments:

  1. When did e become x?

    Also, don't you need #actual[0] != 'x' too?
    And what if #expected[0] == 'x' and #actual[0] == 'y' ?

    I think your "cryptic name [...] incorporating __LINE__" sounds safer.

    But, better still, delegate straight on to a function. I presume the macro is only there for stringizing the expression values anyway?

    ReplyDelete
  2. Yes you do Phil. Like I said - pride and revulsion. Delegating straight to a function is best. Although there is another gotcha waiting for you if you do that... Another post

    ReplyDelete
  3. I've remembered why forwarding was a problem. I use the macro arguments expected and actual twice; once as arguments to a real function that tells me if they are equal or not, and once again as arguments to a real function that prints me a diff. If there are side-effects I only want them to occur once.

    ReplyDelete
  4. Oh there's a lot of gotchas. I think I've hit most of them with Catch already :-)

    ReplyDelete
  5. Odd - your follow-up on forwarding didn't appear until I posted my last reply.

    Yes, I had the double-evaluation issue with Catch. I went to great lengths to overcome it.

    Interestly, the first part of the solution was to forward it all on to a function :-)

    I think the difference is you're using C, whereas I had access to C++ templates.

    To paraphrase Homer Simpsons: "Templates: The cause, and solution, to all of mans problems".

    (BTW, the reason that forwarding to a function was only part of the solution is that capturing literal values as typed arguments makes them behave differently in some cases)

    ReplyDelete
  6. Yes I don't have templates in C of course.
    I'm interested in the cases when capturing literal values as typed arguments makes them behave differently...

    ReplyDelete
  7. It's not quite a strict guarantee that you won't clash with a local variable name, but...

    (Looks like the code formatting will get completely mauled no matter I try, so also posted to https://gist.github.com/2ad2815cd3e3925acaff.)

    void handle_failure(char const *msg, char const *file, int line);

    #define CHECK_EQ(expected, actual) \
    do { \
    if ((expected) != (actual)) { \
    handle_failure(#expected " != " #actual, __FILE__, __LINE__); \
    } \
    } while (0)

    #define CHECK_EQ_INT(expected, actual) \
    do { \
    int _expected_CHECK_EQ_INT = (expected); \
    int _actual_CHECK_EQ_INT = (actual); \
    if (_expected_CHECK_EQ_INT != _actual_CHECK_EQ_INT) { \
    char buf[100]; \
    sprintf(buf, "%d != %d", _expected_CHECK_EQ_INT, _actual_CHECK_EQ_INT); \
    handle_failure(buf, __FILE__, __LINE__); \
    } \
    } while (0)

    ReplyDelete