On Fri, Oct 30, 2020 at 10:28:51AM -0700, Elijah Newren wrote:
> > You might want to mention that this _could_ be done as just accessors to
> > strmap, but using a separate struct provides type safety against
> > misusing pointers as integers or vice versa.
>
> If I just did it as accessors, it makes it harder for myself and
> others to remember what my huge piles of strmaps in merge-ort do; I
> found that it became easier to follow the code and remember what
> things were doing when some were marked as strmap, some as strintmap,
> and some as strset.
Oh, I'm definitely on board with that argument. I was just suggesting
you might want to put it in the commit message for posterity.
> > > +/*
> > > + * strintmap:
> > > + * A map of string -> int, typecasting the void* of strmap to an int.
> >
> > Are the size and signedness of an int flexible enough for all uses?
>
> If some users want signed values and others want unsigned, I'm not
> sure how we can satisfy both. Maybe make a struintmap?
Right, that was sort of my question: do your users actually want it
signed or not. Sounds like they do want it signed, and don't mind the
loss of range.
> Perhaps that could be added later if uses come up for it? Some of my
> uses need int, the rest of them wouldn't care about int vs unsigned.
Yeah, if you don't have any callers which care, I'd definitely punt on
it for now.
> If someone does care about the full range of bits up to 64 on relevant
> platforms, I guess I should make it strintptr_t_map.
Yeah, that's what I was wondering. I suspect the use case for that is
pretty narrow, though. If you really care about having a 64-bit value
for some data, then you probably want it _everywhere_, not just on
64-bit platforms. I guess the exception would be if you're mapping into
size_t's or something.
I think my question was as much "did you think about range issues for
your intended users" as "should we provide more range in this map type".
And it sounds like you have thought about that, so I'm happy proceeding.
> But besides the
> egregiously ugly name, one advantage of int over intptr_t (or unsigned
> over uintptr_t) is that you can use it in a printf easily:
> printf("Size: %d\n", strintmap_get(&foo, 0));
> whereas if it strintmap_get() returns an intptr_t, then it's a royal
> mess to attempt to portably use it without adding additional manual
> casts. Maybe I was just missing something obvious, but I couldn't
> figure out the %d, %ld, %lld, PRIdMAX, etc. choices and get the
> statement to compile on all platforms, so I'd always just cast to int
> or unsigned at the time of calling printf.
The right way is:
printf("Size: %"PRIdMAX", (intmax_t) your_intptr_t);
which will always do the right thing no matter the size (at the minor
cost of passing a larger-than-necessary parameter, but if you're
micro-optimizing then calling printf at all is probably already a
problem).
But yeah, in general using a real "int" is much more convenient and if
there's no reason to avoid it for range problems, I think it's
preferable.
-Peff
