Hi Junio,

On Wed, 17 Nov 2021, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

quoted

diff --git a/mergesort.c b/mergesort.c
index 6216835566..bd9c6ef8ee 100644
--- a/mergesort.c
+++ b/mergesort.c

@@ -63,7 +63,7 @@ void *llist_mergesort(void *list,
 		void *next = get_next_fn(list);
 		if (next)
 			set_next_fn(list, NULL);
-		for (i = 0; n & (1 << i); i++)
+		for (i = 0; n & ((size_t)1 << i); i++)

I was a bit concerned about the operator precedence (some of which I
remember by heart, some not), but according to
https://en.cppreference.com/w/c/language/operator_precedence the cast has
a higher precedence than the shift operator.

I would have preferred an extra pair of parentheses around `(size_t)1` so
that I (and other readers) do not have to remember or look up the operator
precedence, but it _is_ correct.

Interesting.

I do not quite see the need for it myself, but if we wanted to, we
can smoke them out with this, I think.

	$ cat >contrib/coccinelle/cast.cocci <<-\EOF
	@@
	type T;
	expression V, C;
	@@
	-(T) V << C
	+((T) V) << C
	EOF

Cute.

However, I am not a fan of fixing what ain't broken (the many refactorings
in v2.34.0 did cause quite some fall-out work in git-for-windows/git and
microsoft/git, after all, and at least _I_ do not yet see much benefit to
show for that price we paid for those refactorings).

And the primary benefit of enclosing the left operand in parentheses is to
make reviews easier, so I would prefer to leave existing (read:
_already-reviewed_) code alone.

Thanks,
Dscho

	$ make contrib/coccinelle/cast.cocci.patch
	$ git apply --stat contrib/coccinelle/cast.cocci.patch
         compat/mingw.c     |    2 +-
         compat/mingw.c     |    2 +-
         ewah/bitmap.c      |    2 +-
         ewah/ewok_rlw.h    |    6 +++---
         ewah/ewah_bitmap.c |    8 ++++----
         ewah/ewok_rlw.h    |    6 +++---
         ppc/sha1.c         |    2 +-
         wrapper.c          |    2 +-
         8 files changed, 15 insertions(+), 15 deletions(-)