MAINT: fix new compiler warnings on clang#16857
Conversation
|
I am going to self-merge to clear the build failures. |
| (abs(steps[2]) < MAX_STEP_SIZE) && \ | ||
| ((labs(steps[0]) < MAX_STEP_SIZE) && \ | ||
| (labs(steps[1]) < MAX_STEP_SIZE) && \ | ||
| (labs(steps[2]) < MAX_STEP_SIZE) && \ |
There was a problem hiding this comment.
Was good to merge, question though, doesn't this have to be llabs on 64bit windows?
There was a problem hiding this comment.
I would think steps is intp here, which may be a long long on windos 64. Its a corner case, but I expect we should change that. Or just use a macro x < 0 ? -x : x compilers probably make that a single instruction anyway?
There was a problem hiding this comment.
I'm with @seberg, this isn't a complete fix for platforms where long != long long (but is for other platforms)
Another option would be to use the C99 syntax,
#define np_abs(x) _Generic((x), int: abs, long: labs, long long: llabs, intmax_t: imaxabs)(x)There was a problem hiding this comment.
I am wondering if we should just write it as steps[0] < MAX_STEP_SIZE && -MAX_STEP_SIZE < steps[0]?
Is the nomemoverlap call incorrect, in that it needs to use steps[0] * (dimensions[0]-1), although, I suppose that does not matter, it is very unlikely to misfire. It may be that if it misfires, it will also always be fine. But, I am not quite sure it is right now, because I suppose overflows could happen inside nomemoverlap.
EDIT: Hmmm, this all only works if we assume identical item/element sizes, right? Which I guess should generally be fine
gh-16605 passed local CI but after merge CI is broken. Maybe an interaction with some other branch that was merged after it was last run through CI