Hi Hannes,

On Wed, 9 Oct 2019, Johannes Sixt wrote:

> Am 09.10.19 um 03:19 schrieb Junio C Hamano:
> > Johannes Sixt <j...@kdbg.org> writes:
> >  After all, the primary purpose of
> > inventing the encoder was to catch the arith overflow, wasn't it?
> That was *your* motivation for the helper function. But IMO it is a
> wrong design decision.

I wish that comment, and the argument following it, would have come as
part of the review of the patch series that already made it to `next`.

FWIW I actually agree with Junio about the helper, but in hindsight I
could have used a better name (not one that is tied to the "index").
Something like `unsigned_one_complement()`. But of course, that would
say _what_ it does, not _why_.

And yes, I would wish we had C++ templates so that the helper could use
the exact type of the caller.


> Whether or not the index calculation overflows is a matter of the type
> that is used for the index, and that in turn is dicated by the
> possible sizes of the collections that are indexed. IOW, the index
> overflow check is (*if* it is necessary) a policy decision that must
> be made at a higher level and must not be hidden away in a helper
> function whose purpose (as suggested by its name) is something
> entirely different.
> Unless, of course, we declare "all our indexes are of type int". But
> that ship has sailed long ago, because there are too many cases where we
> are forced to use size_t as index (strlen, sizeof...).
> Meta note: We know that we are painting a tiny shed here (Replacing a
> one-liner by a one-liner, huh?) If anyone of you has better things to
> do, please move on. My interest in this discussion are just the design
> decisions that are made, not the actual outcome of this particular case.
> -- Hannes

Reply via email to