On 2014-01-10 04:14, Kinkie wrote:
It's been there forever, and a build with the most recent clang
version is failing on it:
store_repl_heap.cc:224
try_again:
if (!heap_nodes(h->theHeap) > 0)
Does anyone have any idea about what this code is supposed to do? A
cursory look at the code would replace it with
if (heap_empty(h->theHeap))
but before doing that I'd like to make sure I didn't misunderstand.
Ew. Tricky little piece of garbage.
1) That nasty little ! operator has highest precedence just to confuse
things.
So this is ((!heap_nodes(X)) > 0)
2) ! operator working on an integer of any kind is equivalent to (FOO ==
0)
So this is ((heap_nodes(X) == 0) > 0)
3) implicit casting of boolean to integer true is non-0, false ==0.
So this is (heap_nodes(X) == 0)
4) And then there is the heap_*(X) defines:
#define heap_nodes(heap) ((heap)->last)
#define heap_empty(heap) (((heap)->last <= 0) ? 1 : 0)
5) "last" is an unsigned value.
So actually the define should be:
#define heap_empty(heap) (((heap)->last == 0) ? 1 : 0)
6) == operator outputs a boolean
So actually the define should be:
#define heap_empty(heap) ((heap)->last == 0)
7) ==0 is equivalent to !
So actually the define should be:
#define heap_empty(heap) !((heap)->last)
So putting all these together the redux is:
A) the current form:
(!heap_nodes(X)) > 0
(!heap_nodes(X)) > 0
(heap_nodes(X) == 0) > 0
(heap_nodes(X) == 0)
((X)->last == 0)
!((X)->last)
B) the proposed empty form:
heap_empty(X)
((X->last <= 0) ? 1 : 0)
(((X)->last == 0) ? 1 : 0)
((X)->last == 0)
!((X)->last)
So yes I concur, the proposed replacement is equivalent. But it will
result in the if ((...)) syntax which clang very much disagrees with.
You will need to also fix the heap_empty macro definition to
!((heap)->last) just to get it to compile portably.
Amos
