Hi Hannes, On Wed, 9 Oct 2019, Johannes Sixt wrote: > Am 09.10.19 um 03:19 schrieb Junio C Hamano: > > Johannes Sixt <j6t@xxxxxxxx> 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. Ciao, Dscho > 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 >
