On Fri, Oct 05, 2018 at 12:59:02AM +0200, René Scharfe wrote:
> We could also do something like this to reduce the amount of manual
> casting, but do we want to? (Macro at the bottom, three semi-random
> examples at the top.)
> [...]
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 5f2e90932f..f9e78d69a2 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -1066,6 +1066,18 @@ static inline void sane_qsort(void *base, size_t nmemb, size_t size,
> qsort(base, nmemb, size, compar);
> }
>
> +#define DEFINE_SORT(name, elemtype, one, two, code) \
> +static int name##_compare(const void *one##_v_, const void *two##_v_) \
> +{ \
> + elemtype const *one = one##_v_; \
> + elemtype const *two = two##_v_; \
> + code; \
> +} \
> +static void name(elemtype *array, size_t n) \
> +{ \
> + QSORT(array, n, name##_compare); \
> +}
Interesting. When I saw the callers of this macro, I first thought you
were just removing the casts from the comparison function, but the real
value here is the matching QSORT() wrapper which provides the type
safety.
I'm not wild about declaring functions inside macros, just because it
makes tools like ctags like useful (but I have certainly been guilty of
it myself). I'd also worry that taking "code" as a macro parameter might
not scale (what happens if the code has a comma in it?)
I think we can address that last part by switching the definition order.
Like:
#define DEFINE_SORT(name, elemtype, one, two) \
static int name##_compare(const void *, const void *); \
static void name(elemtype *array, size_t n) \
{ \
QSORT(array, n, name##_compare); \
} \
static int name##_compare(const void *one##_v_, const void *two##_v_) \
{ \
elemtype const *one = one##_v_; \
elemtype const *two = two##_v_; \
And then expecting the caller to do:
DEFINE_SORT(foo, struct foo, a, b)
/* code goes here */
}
The unbalanced braces are nasty, though (and likely to screw up editor
formatting, highlighting, etc).
I wonder if it would be possible to just declare the comparison function
with its real types, and then teach QSORT() to do a type check. That
would require typeof() at least, but it would be OK for the type-check
to be available only to gcc/clang users, I think.
I'm not quite sure what that type-check would look like, but I was
thinking something along the lines of (inside the QSORT macro):
do {
/* this will yield a type mismatch if fed the wrong function */
int (*check)(const typeof(array), const typeof(array)) = compar;
sane_qsort(array, n, sizeof(*array), n);
} while (0)
I have no idea if that even comes close to compiling, though.
-Peff
