Re: [PATCH 3/5] use SWAP macro
- From
René Scharfe <l.s.r@web.de>
- Date
- Jan 31, 2017, 21:02 UTC
- Message-ID
- <47dca1ce-14db-cba0-bbe0-9873fc0000b5@web.de>
- In-Reply-To
- <xmqqr33ktch9.fsf@gitster.mtv.corp.google.com>
Am 30.01.2017 um 23:22 schrieb Junio C Hamano:
Show 14 quoted lines
> René Scharfe <l.s.r@web.de> writes:
>
>> if (tree2->flags & UNINTERESTING) {
>> - struct object *tmp = tree2;
>> - tree2 = tree1;
>> - tree1 = tmp;
>> + SWAP(tree2, tree1);
>
> A human would have written this SWAP(tree1, tree2).
>
> Not that I think such a manual fix-up should be made in _this_
> patch, which may end up mixing mechanical conversion (which we may
> want to keep reproducible) and hand tweaks. But this swapped swap
> reads somewhat silly.Well, a human wrote "tmp = tree2" -- sometimes one likes to count down instead of up, I guess.
We can make Coccinelle order the parameters alphabetically, but I don't know how to do so without duplicating most of the semantic patch. And thats would result in e.g. SWAP(new_tree, old_tree), which might be odd as well.
I'd just leave them in whatever order they were, but that's probably because I'm lazy.
René