gh-125889: Fix bounds check of hi argument in bisect_left|right functions

[rruuaanng]

• Issue: python versions of bisect_right and bisect_left can fail guarantees #125889

[nineteendo]

Can you add a test?

[rruuaanng]

I'm editing it, this may take a while, please excuse me.

[rruuaanng]

For the two solutions mentioned by

1. We could either make it an error to pass a negative value as is the case with lo currently.
2. fix it to work with negative values.

I chose the latter because almost all test functions in the test assume that when hi is a negative number, there will be a default processing. But I don't want to refactor this test on a large scale, so I chose the latter, and in the test, it mainly judges that when its hi is a negative number, it can automatically handle it. Is negative and it is not negative 1, it can be negative infinity.

[mike-neergaard]

I see a modification to
Modules/_bisectmodule.c

If someone somehow winds up running the python code in Lib/bisect.py, would it still have the bug?

[rruuaanng]

Maybe not. You can read the test I added.It described that under normal use, when the hi parameter is negative, there will be no abnormality. I'm not sure if this is what you expected.

Edit

In fact, the c file I changed corresponds to the implementation in python. Generally, python calls the c function.

[picnixz]

In fact, the c file I changed corresponds to the implementation in python. Generally, python calls the c function.

This is incorrect. The C implementation is used if it is available. I think we should first discuss more on whether negative indices should be supported at all.

Currently hi == -1 means for the C implementation "not given" but we should decide first whether hi < 0 means "count from the end". And we should synchronize both the Python and the C implementation so that they consider hi < 0 the same way.

[rruuaanng]

Oh, forgive me, the description seems to be a little failed. I want to say c implementation, but what it mentions is py implementation. I will continue to change it later.

[rhettinger]

FWIW, the hi < 0 check had been intentionally omitted and there was a previous discussion about it. The thought was that it slowed down the code but provided no real value in real code. This "solves" a problem that no one seems to have in practice.

[rruuaanng]

Therefore, there are two points of focus on the current discussion.

1. hi isn't allowed to be a negative.
2. hi makes it work normally when it's negative.

[rruuaanng]

I will close this RP, maybe we need to wait until that unified proposal before moving forward, then we can reopen it. Thank you for your attention to this.