Re: [PATCH 1/5] add SWAP macro

[Jeff King <peff@peff.net>]

On Wed, Feb 01, 2017 at 12:28:13PM +0100, Johannes Schindelin wrote:

> > One funny thing is that your macro takes address-of itself, behind the
> > scenes. I wonder if it would be more natural for it to take
> > pointers-to-objects, making it look more like a real function (i.e.,
> > SWAP(&a, &b) instead of SWAP(a, b)". And then these funny corner cases
> > become quite obvious in the caller, because the caller is the one who
> > has to type "&".
> 
> But forcing SWAP(&a, &b) would make it even more cumbersome to use, and it
> would also make it harder to optimize, say, by using registers instead of
> addressable memory (think swapping two 32-bit integers: there is *no* need
> to write them into memory just to swap them).

I don't find the register thing compelling at all. I'd expect modern
compilers to optimize *(&a) into just "a" and keep using a register. I'm
sure there are compilers that don't, but I'm also not sure how much we
_care_. If your compiler doesn't do basic micro-optimizations, then you
live with it or get a better compiler.

I'm much more interested in what's readable and maintainable, versus
trying to micro-optimize something that hasn't even been shown to be
measurably interesting.

That said...

> And I think I should repeat my point that this discussion veers towards
> making simple swaps *more* complicated, rather than less complicated. Bad
> direction.

I'm not altogether convinced that SWAP() is an improvement in
readability. I really like that it's shorter than the code it replaces,
but there is a danger with introducing opaque constructs. It's one more
thing for somebody familiar with C but new to the project to learn or
get wrong.

IMHO the main maintenance gain from René's patch is that you don't have
to specify the type, which means you can never have a memory-overflow
bug due to incorrect types. If we lose that, then I don't see all that
much value in the whole thing.

-Peff