MAINT: Return size_t from num_codepoints in string ufuncs Buffer class #25571
Conversation
|
|
||
| adjust_offsets(&start, &end, len1); | ||
| if (end - start < len2) { | ||
| if (end - start < static_cast<npy_int64>(len2)) { |
There was a problem hiding this comment.
Somewhat flyby, but could one write this as end < start + len2 and avoid the distracting static_cast?
There was a problem hiding this comment.
My understanding is that the signedness of start + len2 is implementation-specific and depends on the size of size_t. For implementations where it's a 64-bit integer, the resulting type is also unsigned, so we end up with signed-unsigned comparison again.
If the unsigned type has conversion rank greater than or equal to the rank of the signed type, then the operand with the signed type is implicitly converted to the unsigned type.
- the ranks of all signed integer types are different and increase with their precision: rank of signed char < rank of short < rank of int < rank of long int < rank of long long int
- the ranks of all signed integer types equal the ranks of the corresponding unsigned integer types
There was a problem hiding this comment.
Well, maybe explicit is better, then...
|
Looks like there's a heap buffer overflow in |
|
The sanitizer actually did a really nice job on my fft via gufunc PR (#25536). Though it still took me quite a while to figure out where I was writing a byte too many! But very nice that it found it, since everything else just passed fine. |
339c6e5 to
c347ca5
Compare
c347ca5 to
67d932d
Compare
|
The test failures are unrelated as far as I can tell. |
|
Rerunning the pypy job just in case. Is it failing also for other PRs? |
|
I don't see any other failures, no, but I think it was a very early segfault in pypy code before the build even began. Could you maybe rerun the |
|
Ah, sorry, missed that there was another failure. Let's see what a rerun does. |
|
Yup, they're both okay now. |
|
There seems to be a new 60 minute build time limit we're hitting in azure, it's unrelated to this PR. |
|
Thanks for this! |
Closes #25565.