stb_ds: arrins and -Wsign-compare

[ennorehling]

After upgrading from version 0.65 to 0.67, when compiling with -Wsigned-compare, I get this warning from stb_ds.h:

enno@erdbaer:~/eressea/git@develop$ gcc -Wextra -Werror stbtest.c
In file included from stbtest.c:2:
stbtest.c: In function ‘main’:
include/stb_ds.h:541:32: error: operand of ‘?:’ changes signedness from ‘long int’ to ‘size_t’ {aka ‘long unsigned int’} due to unsignedness of other operand [-Werror=sign-compare]
  541 | #define stbds_arrlen(a)        ((a) ? (ptrdiff_t) stbds_header(a)->length : 0)
      |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/stb_ds.h:548:128: note: in expansion of macro ‘stbds_arrlen’
  548 | ds_header(a)->length += (n), stbds_header(a)->length-(n)) : stbds_arrlen(a))
      |                                                             ^~~~~~~~~~~~

include/stb_ds.h:546:40: note: in expansion of macro ‘stbds_arraddnindex’
  546 | #define stbds_arraddn(a,n)     ((void)(stbds_arraddnindex(a, n)))    // deprecated, use one of the following instead:
      |                                        ^~~~~~~~~~~~~~~~~~
include/stb_ds.h:555:33: note: in expansion of macro ‘stbds_arraddn’
  555 | #define stbds_arrinsn(a,i,n)   (stbds_arraddn((a),(n)), memmove(&(a)[(i)+(n)], &(a)[i], sizeof *(a) * (stbds_header(a)->length-(n)-(i))))
      |                                 ^~~~~~~~~~~~~
include/stb_ds.h:556:33: note: in expansion of macro ‘stbds_arrinsn’
  556 | #define stbds_arrins(a,i,v)    (stbds_arrinsn((a),(i),1), (a)[i]=(v))
      |                                 ^~~~~~~~~~~~~
include/stb_ds.h:409:21: note: in expansion of macro ‘stbds_arrins’
  409 | #define arrins      stbds_arrins
      |                     ^~~~~~~~~~~~
stbtest.c:7:5: note: in expansion of macro ‘arrins’
    7 |     arrins(arr, 0, 42);
      |     ^~~~~~
cc1: all warnings being treated as errors

My compiler is gcc 10.2:

enno@erdbaer:~/eressea/git@develop$ gcc --version
gcc (Debian 10.2.1-6) 10.2.1 20210110

This is my sample to reproduce the problem:

enno@erdbaer:~/eressea/git@develop$ cat stbtest.c
#define STB_DS_IMPLEMENTATION
#include "include/stb_ds.h"

int main(void)
{
    int *arr = 0;
    arrins(arr, 0, 42);
    return 0;
}

[nothings]

i don't understand the error. ptrdiff_t is a signed type, and 0 is a signed type. Furthermore, the definition of arrlen() is unchanged from 0.65. The intermediate macros have changed, but apparently only to convert the name from arraddindex from arraddoff.

[mmozeiko]

I think gcc does not like ternary op in stbds_arraddnindex, which expands to cond ? A : B where A has size_t type, but B has ptrdiff_t (from stbds_arrlen). It wants those types to be the same signedness. Here's the same warning: https://godbolt.org/z/Ees4YrKMv

[nothings]

Ah, yes, I see. I understood the error, I just misunderstood which ternary was causing it.

Also, while looking at the code, looks like arraddnptr has a bug in the case where n=0, it doesn't offset to the end of the array. I guess the author imagined because that's not a valid place to write to, but the inconsistency with all the other cases doesn't make sense to me.

[ennorehling]

So I guess replacing the stbds_arrlen in line 548 with stbds_arrlenu does the trick. Incidentally, why is arrlen defined as ptrdiff_t, not size_t? Negative array sizes don't make any sense, and having both arrlen and arrlenu seems bizarre.

[nothings]

unsigned integer indices are terrible for e.g. downward counted loops, therefore generally all indices should be signed integers, therefore all sizes should be signed to avoid warnings when used with signed indices

arrlenu is available for people who disagree with me