Thread (40 messages) 40 messages, 7 authors, 2017-05-02

Re: [PATCH 1/5] add SWAP macro

From: Jeff King <hidden>
Date: 2017-02-01 11:48:12

On Wed, Feb 01, 2017 at 12:28:13PM +0100, Johannes Schindelin wrote:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help