Re: [PATCH v2] last-modified: implement faster algorithm

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> On Tue, Oct 21, 2025 at 10:52:11AM -0700, Junio C Hamano wrote:
>> > +		struct diff_filepair *fp = diff_queued_diff.queue[i];
>> > +		size_t k = path_idx(lm, fp->two->path);
>> > +		if (0 <= k && bitmap_get(active_c, k))
>> > +			bitmap_set(lm->scratch, k);
>> > +	}
>>
>> Earlier path_idx() wanted to signal an error by returning negative,
>> but the type is size_t that is unsigned so it cannot do so.  We
>> instead get
>>
>>     builtin/last-modified.c:307:23: error: comparison of unsigned expression in '>= 0' is always true [-Werror=type-limits]
>>       307 |                 if (0 <= k && bitmap_get(active_c, k))
>>           |                       ^~
>> ...
> Yeah, this is a true positive. I was curious if GitHub's version of the
> code also returned "-1" from a function whose return type is unsigned,
> and in fact our version of this function (called diff2idx()) returns an
> 'int'.

Yes, I think the tool is doing the right thing here, unlike "hey you
are comparing int with size_t" we saw earlier, and is giving us a
useful diagnosis.

> Practically speaking that's probably OK, since we are unlikely to have
> so many active paths anyway (or if we did, we'd likely have other
> problems to deal with ;-)), but it is gross nonetheless.

The case path_idx() returns -1 is an error case, not "there are too
many paths we are following" case.  I do not see what relevance the
number of active paths has here.