Re: interesting tidbit in store_repl_heap.cc

Tested and committed.
Thanks!

On Thu, Jan 9, 2014 at 11:17 PM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> 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

-- 
    /kinkie