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.
When did e become x?
ReplyDeleteAlso, 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?
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
ReplyDeleteI'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.
ReplyDeleteOh there's a lot of gotchas. I think I've hit most of them with Catch already :-)
ReplyDeleteOdd - your follow-up on forwarding didn't appear until I posted my last reply.
ReplyDeleteYes, 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)
Yes I don't have templates in C of course.
ReplyDeleteI'm interested in the cases when capturing literal values as typed arguments makes them behave differently...
It's not quite a strict guarantee that you won't clash with a local variable name, but...
ReplyDelete(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)
Thanks Roger.
ReplyDelete