Re: [PATCH 1/5] add SWAP macro
- From
Jeff King <peff@peff.net>
- Date
- Jan 31, 2017, 21:35 UTC
- Message-ID
- <20170131213507.uiwmkkcg7umvd3f4@sigill.intra.peff.net>
- In-Reply-To
- <8e94756a-c3a5-9b81-268d-d0f36876f710@web.de>
On Tue, Jan 31, 2017 at 10:03:01PM +0100, René Scharfe wrote:
Show 7 quoted lines
> > Perhaps we could disallow a side-effect operator in the macro. By > > disallow I mean place a comment at the definition to the macro and > > hopefully catch something like that in code-review. We have the same > > issue with the `ALLOC_GROW()` macro. > > SWAP(a++, ...) is caught by the compiler, SWAP(*a++, ...) works fine. > Technically that should be enough. :) A comment wouldn't hurt, of course.
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 "&".
-Peff