Fix invalid codegen for #7451. - #7456

6562250 extends a current unit test to include prescribing a negative axis value to trigger the bug described here: #7451 (comment). This will manifest as not being able to broadcast.

The addition of f3755f2 fixes the problem demonstrated in 6562250.

Buildfarm ID: numba_smoketest_cpu_yaml_50.

This fixes this particular issue, but should we instead change tuple_setitem so that it supports negative indexing? What if there are other latent instances of this issue in the codebase?

This fixes this particular issue, but should we instead change tuple_setitem so that it supports negative indexing? What if there are other latent instances of this issue in the codebase?

Or maybe "also", since I see why the change is needed to allow broadcasting here.

This fixes this particular issue, but should we instead change tuple_setitem so that it supports negative indexing? What if there are other latent instances of this issue in the codebase?

Thanks for checking the patch. Suggest talking about this at the public meeting today. The function is "unsafe" by design, I guess the question is how unsafe that should be.

Buildfarm ID: numba_smoketest_cpu_yaml_50.

Passed on f3755f2

The function is "unsafe" by design, I guess the question is how unsafe that should be.

I think at the moment it's a bit of a footgun, because a user would reasonably expect it to handle negative indexing (consistent with lots of other indexing use cases) but its documentation makes no mention of it not supporting negative indexing. If we decide not to support negative indexing, then I think it would be good to mention that it doesn't support negative indexing in the docs.

The function is "unsafe" by design, I guess the question is how unsafe that should be.

I think at the moment it's a bit of a footgun, because a user would reasonably expect it to handle negative indexing (consistent with lots of other indexing use cases) but its documentation makes no mention of it not supporting negative indexing. If we decide not to support negative indexing, then I think it would be good to mention that it doesn't support negative indexing in the docs.

Agree, it either needs the checks or some big warning about what "in-bounds" means, i.e. not the Python version.

I think given the current PR backlog, as this fixes the immediate issue it should be merged in as it makes mainline stable again and permits merges of other PRs. However, I'll raise the question of "how safe is numba.*.unsafe.*?" at the public meeting today. This also makes me wonder if there should be a "safe" mode where things like this are checked.

Looks good!

Thanks for the review @gmarkall

I see all unsafe intrinsics as the same level of safety as inlineasm; thus, no safety checks at all. We can provide a "safe" version of it though but leave dev a choice to opt into the unsafe and faster one.