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

Commit Message

Elijah Newren Oct. 13, 2020, 12:40 a.m. UTC

From: Elijah Newren <[e-mail]>

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().

Signed-off-by: Elijah Newren <[e-mail]>
---
 strmap.h | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

Comments

Jeff King Oct. 30, 2020, 2:44 p.m. UTC | #1

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

> From: Elijah Newren <[e-mail]>
> 
> 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

Elijah Newren Oct. 30, 2020, 6:02 p.m. UTC | #2

On Fri, Oct 30, 2020 at 7:44 AM Jeff King <[e-mail]> wrote:
>
> On Tue, Oct 13, 2020 at 12:40:49AM +0000, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <[e-mail]>
> >
> > 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).

Yeah, I'll just go with strset_check_and_add().  :-)

Patch

diff --git a/strmap.h b/strmap.h
index fe15e74b78..2ad6696950 100644
--- a/strmap.h
+++ b/strmap.h
@@ -178,4 +178,68 @@  static inline void strintmap_set(struct strintmap *map, const char *str,
 
 void strintmap_incr(struct strintmap *map, const char *str, intptr_t amt);
 
+/*
+ * strset:
+ *    A set of strings.
+ *
+ * Primary differences with strmap:
+ *    1) The value is always NULL, and ignored.  As there is no value to free,
+ *       there is one fewer argument to strset_clear
+ *    2) No strset_get() because there is no value.
+ *    3) No strset_put(); use strset_add() instead.
+ */
+
+struct strset {
+	struct strmap map;
+};
+
+#define strset_for_each_entry(mystrset, iter, var)	\
+	strmap_for_each_entry(&(mystrset)->map, iter, var)
+
+static inline void strset_init(struct strset *set)
+{
+	strmap_init(&set->map);
+}
+
+static inline void strset_ocd_init(struct strset *set,
+				   int strdup_strings)
+{
+	strmap_ocd_init(&set->map, strdup_strings);
+}
+
+static inline void strset_clear(struct strset *set)
+{
+	strmap_clear(&set->map, 0);
+}
+
+static inline void strset_partial_clear(struct strset *set)
+{
+	strmap_partial_clear(&set->map, 0);
+}
+
+static inline int strset_contains(struct strset *set, const char *str)
+{
+	return strmap_contains(&set->map, str);
+}
+
+static inline void strset_remove(struct strset *set, const char *str)
+{
+	return strmap_remove(&set->map, str, 0);
+}
+
+static inline int strset_empty(struct strset *set)
+{
+	return strmap_empty(&set->map);
+}
+
+static inline unsigned int strset_get_size(struct strset *set)
+{
+	return strmap_get_size(&set->map);
+}
+
+static inline void strset_add(struct strset *set, const char *str)
+{
+	strmap_put(&set->map, str, NULL);
+}
+
 #endif /* STRMAP_H */