ENH: Add overflow handling for negative integers scalar multiplication

[Micky774]

Related Issues/PRs

Related to #21506

Follow-up to #21648

Changes

Adds checks for multiplication overflow for signed integers, in particular when they are negative.

Comments

N/A

[seberg]

I am wondering slightly whether we should care that npy_mul_with_overflow_intp is currently almost only used in places where we really don't care about negative values, because its main purpose is/was for shape calculations.
Likely, it just doesn't matter, though, and this is good.

[Micky774]

I am wondering slightly whether we should care that npy_mul_with_overflow_intp is currently almost only used in places where we really don't care about negative values, because its main purpose is/was for shape calculations. Likely, it just doesn't matter, though, and this is good.

I currently have it treated as an "unsigned" value for that exact reason. It doesn't break any tests so hopefully it isn't problematic.

[seberg]

Ah, I had missed that. That works, since intp is only an alias (so it doesn't exist as a "scalar"). However, I it also seems confusing to use the same name/pattern.

I don't have a super good idea right away though, one would be to rename the intp version to npy_mul_sizes_with_overflow (and maybe add assert(a >= 0 && b >= 0). The use in scalarmath.c is already an oversight due to the >= 0 constraint, and that was added by Julian, who wrote the original I think :).

[seberg]

@Micky774 did you prefer keeping it as is, or rather introduce a new name for the intp version? I am slightly leaning to that right now. (even if it is a bit annoying code wise.)

[Micky774]

@Micky774 did you prefer keeping it as is, or rather introduce a new name for the intp version? I am slightly leaning to that right now. (even if it is a bit annoying code wise.)

I can adopt your suggestion and write a separate function for it. Apologies for the delays, I usually work on numpy stuff towards the end of the week :)

[Micky774]

@seberg I think this is ready now, there's just a problem w/ Travis CI

[seberg]

Agreed, thanks @Micky774!