Re: [PATCH 2/4] string-list: replace negative index encoding with "exact_match" parameter
- From
shejialuo <shejialuo@gmail.com>
- Date
- Sep 15, 2025, 12:24 UTC
- Message-ID
- <aMgFjTqYRB4A9v_4@ArchLinux>
- In-Reply-To
- <xmqqv7lsx4rb.fsf@gitster.g>
On Mon, Sep 08, 2025 at 09:56:08AM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> shejialuo <shejialuo@gmail.com> writes: > > > We would return negative index to indicate exact match by converting the > > original positive index to be "-1 - index" in > > "string_list_find_insert_index", which requires callers to decode this > > information. > > > > This is bad due to the following reasons: > > > > 1. The callers need to convert the negative index back to the original > > positive value, which requires the callers to understand the detail > > of the function. > > That has pretty much been the convention so far, not convincing that > it is "bad" at all. >
Good point, I somehow like to use the words containing emotion, which is not suitable. And after googling, I realize that this is the convention.
Show 27 quoted lines
> > 2. As we have to return negative index, we need to specify the return > > type to be `int` instead of `size_t`, which would cause sign compare > > warnings. > > That sounds more like the tail wagging the dog. > > Construct your argument the other way around, perhaps? > > - We NEED to be able to use the full range of size_t to express the > index in the array string_list holds for SUCH AND SUCH REASONS. > But string_list_find_insert_index() uses "int", which may not be > large enough to cover the range size_t covers. > > - In addition, in order to signal that the returned value for a > query is about an existing entry in the array, or a location that > an entry would be inserted at, we use a signed int and use the > bog standard "-1 - index" encoding for this purpose. This > further halves the range of valid array index. > > - To allow us to use the full range of size_t, use full size_t for > the index, and have a separate bit to tell if that index is about > an existing entry, or where the queried entry would be stored at > if we inserted it. > > Your argument does not justify the first point, your desire to use > size_t in the first place, and that is what makes it sound > backwards, I think..
Thanks for the suggestion, I will improve this in the next version.
Thanks, Jialuo