Re: [PATCH] Use MOVE_ARRAY

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 22, 2018, 23:26 UTC
Message-ID: <xmqq372xjwzg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20180122224410.GA21604@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

> Most of these are "shift part of the array". I wonder if it would make
> sense to encapsulate that pattern in a helper, like:
>
>   #define SHIFT_ARRAY(a, nr, pos, slots) \
>     MOVE_ARRAY(a + pos + slots, a + pos, nr - pos - slots)
>   ...
>   SHIFT_ARRAY(it->down, it->subtree_nr, pos, 1);

Exactly my thought when I was queuing it, but I was wondering about this more from "can we use the higher level abstraction for reducing errors?" point of view. If we are shifting an array by 3 slots to the right, we should at least have enough slots allocated to hold them (i.e. a->nr - a->alloc must be 3 or more). But after realizing that the level these macros operate at is still a bit too low to do something like that, I quickly lost interest ;-)

> I'm not sure if that's more readable because it describes a higher-level
> operation, or if it's less because it adds yet another non-standard
> helper for the reader to learn.

Yeah, conflicting goals. It indeed is easier to see what is going on when reading the code, once the reader gets used to them.

Thanks.