Re: [PATCH 3/5] use SWAP macro

From: Junio C Hamano <hidden>
Date: 2017-01-30 22:22:23

René Scharfe [off-list ref] 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.

quoted hunk

diff --git a/diff-no-index.c b/diff-no-index.c
index f420786039..1ae09894d7 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c

@@ -186,9 +186,8 @@ static int queue_diff(struct diff_options *o,
 
 		if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
 			unsigned tmp;
-			const char *tmp_c;
 			tmp = mode1; mode1 = mode2; mode2 = tmp;
-			tmp_c = name1; name1 = name2; name2 = tmp_c;
+			SWAP(name1, name2);

Curious that mode swapping is left for a later iteration.

quoted hunk

diff --git a/diff.c b/diff.c
index f08cd8e033..9de1ba264f 100644
--- a/diff.c
+++ b/diff.c

@@ -5118,13 +5118,11 @@ void diff_change(struct diff_options *options,
 
 	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
 		unsigned tmp;
-		const unsigned char *tmp_c;
-		tmp = old_mode; old_mode = new_mode; new_mode = tmp;
-		tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c;
+		SWAP(old_mode, new_mode);
+		SWAP(old_sha1, new_sha1);
 		tmp = old_sha1_valid; old_sha1_valid = new_sha1_valid;
 			new_sha1_valid = tmp;

So is this one.

quoted hunk

diff --git a/merge-recursive.c b/merge-recursive.c

...
-			tmp = ren2;
-			ren2 = ren1;
-			ren1 = tmp;
+			SWAP(ren2, ren1);

A human would have written this SWAP(ren1, ren2).

Re: [PATCH 3/5] use SWAP macro

From: René Scharfe <hidden>
Date: 2017-01-31 21:03:35

Am 30.01.2017 um 23:22 schrieb Junio C Hamano:

René Scharfe [off-list ref] writes:

quoted

 		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é

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help