Re: [PATCH v2 09/10] strmap: add a strset sub-type

From: Jeff King <peff@peff.net>
Date: Oct 30, 2020, 14:44 UTC
Message-ID: <20201030144418.GI3277724@coredump.intra.peff.net>
In-Reply-To: <490d3a42add2cc5f0d30db8f2351614294e00121.1602549650.git.gitgitgadget@gmail.com>

On Tue, Oct 13, 2020 at 12:40:49AM +0000, Elijah Newren via GitGitGadget wrote:

Show 12 quoted lines

> From: Elijah Newren <newren@gmail.com>
> 
> Similar to adding strintmap for special-casing a string -> int mapping,
> add a strset type for cases where we really are only interested in using
> strmap for storing a set rather than a mapping.  In this case, we'll
> always just store NULL for the value but the different struct type makes
> it clearer than code comments how a variable is intended to be used.
> 
> The difference in usage also results in some differences in API: a few
> things that aren't necessary or meaningful are dropped (namely, the
> free_util argument to *_clear(), and the *_get() function), and
> strset_add() is chosen as the API instead of strset_put().

That all makes sense.

We're wasting 8 bytes of NULL pointer for each entry, but it's unlikely to be all that important. If we later find a case where we think it matters, we can always refactor the type not to depend on strmap.

I'd want a strset_check_and_add() to match what I used recently in shortlog.h. Maybe strset_contains_and_add() would be a better name to match the individual functions here. I dunno (it actually seems clunkier).

-Peff