gh-99761: Add _PyLong_IsPositiveSingleDigit

[eendebakpt]

Add _PyLong_IsPositiveSingleDigit to unify the usage of twos-complement in Python.

This PR was part of #99762, but split off on suggestion of @vstinner

• Issue: Consistent use of valid index check #99761

[vstinner]

Honestly, I'm not sure that this code makes the code easier to read or maintain. I don't get the "_PyLong_Negative_or_multi_digit_int" name. From what I read, I understand that the optimization only works if the number is positive or zero and has a single digit.

Maybe the function name should be the opposite, something like: _PyLong_IsPositiveSingleDigit().

The optimization would be disabled if !_PyLong_IsPositiveSingleDigit(number).

[eendebakpt]

_PyLong_IsPositiveSingleDigit

I agree _PyLong_IsPositiveSingleDigit is a better name. The method is only used in three locations (in code that I guess will only be modified by expert core devs), so if this variation does not improve the code I am fine with closing the PR.

[vstinner]

@gvanrossum @markshannon: What do you think of this change? Does it make the code more readable? The initial motivation was to put the (size_t) cast optimization in a function to give it a better name and make it less magic. The cast is used to implement "0 <= ndigits && ndigits <= 1" as a single test "(size_t)ndigits <= 1". See also #99761 for more context.

[gvanrossum]

This comment feels disconnected (two's complement is not unique to this function, it is assumed everywhere).

What requires an explanation is the trick of casting a signed value to an unsigned value and then checking whether the result is <= 1. The clever bit here is that this cast makes all negative numbers be considered very large positive numbers.

I'm also not keen on 'signed_magnitude' as the name for the variable. It makes me think of the "sign + magnitude" representation of numbers which is actually how I'd describe one's complement (!). I suggest renaming it to 'signed_size' which is just a reminder of what Py_SIZE() of a PyLong represents the size, negated if the sign of the overall number is negative. (The clever bit there is that the value zero has size 0 which is invariant if negated.)

[vstinner]

I asked to mention two's complement in this function, even if I'm not sure that this optimization relies on it.

This change motivated me to create issue #100008 to require two's complement integer representation to build Python.

[gvanrossum]

What's a cast from int to uint expected to do for negative values in one's complement? Thinking about it more, it probably still converts all negative numbers to very large positive ones, so it would still work, except for -0. Or maybe even in that case, because that's not a positive int.

So I'm still not sure that two's complement deserves being called out here.

(I do agree that we should stop believing we might support one's complement. :-)

[eendebakpt]

I agree that mentioning twos complement here is not relevant. I updated the description and included the link that was already present in the valid_index method.

I also updated the name to signed_size.

[gvanrossum]

Excellent! Future generations will thank you for that comment.

[gvanrossum]

@vstinner Do you think this requires a news entry? If not, just add the skip news label and merge.

[kumaraditya303]

I added skip news since the change is purely internal.

[gvanrossum]

Okay, @kumaraditya303 go ahead and merge.

@eendebakpt Thanks for the code!

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM Raspbian 3.x has failed when building commit 2b82c36.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/424/builds/3163) and take a look at the build logs.
4. Check if the failure is related to this commit (2b82c36) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/424/builds/3163

Summary of the results of the build (if available):

== Tests result: ENV CHANGED ==

411 tests OK.

10 slowest tests:

• test_largefile: 6 min 32 sec
• test_venv: 5 min 13 sec
• test_multiprocessing_spawn: 4 min 34 sec
• test_dbm_gnu: 3 min 24 sec
• test_asyncio: 3 min 24 sec
• test_concurrent_futures: 3 min 23 sec
• test_tokenize: 2 min 4 sec
• test_multiprocessing_forkserver: 1 min 58 sec
• test_gdb: 1 min 47 sec
• test_multiprocessing_fork: 1 min 28 sec

1 test altered the execution environment:
test_asyncio

21 tests skipped:
test_check_c_globals test_devpoll test_idle test_ioctl test_kqueue
test_launcher test_msilib test_peg_generator test_perf_profiler
test_startfile test_tcl test_tix test_tkinter test_ttk
test_ttk_textonly test_turtle test_winconsoleio test_winreg
test_winsound test_wmi test_zipfile64

Total duration: 27 min 27 sec

Click to see traceback logs

remote: Enumerating objects: 8, done.        
remote: Counting objects:  12% (1/8)        
remote: Counting objects:  25% (2/8)        
remote: Counting objects:  37% (3/8)        
remote: Counting objects:  50% (4/8)        
remote: Counting objects:  62% (5/8)        
remote: Counting objects:  75% (6/8)        
remote: Counting objects:  87% (7/8)        
remote: Counting objects: 100% (8/8)        
remote: Counting objects: 100% (8/8), done.        
remote: Compressing objects:  33% (1/3)        
remote: Compressing objects:  66% (2/3)        
remote: Compressing objects: 100% (3/3)        
remote: Compressing objects: 100% (3/3), done.        
remote: Total 8 (delta 5), reused 6 (delta 5), pack-reused 0        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to '2b82c36f17ada471e734c3ad93e6eff8b36a5ad9'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 2b82c36f17 gh-99761: Add `_PyLong_IsPositiveSingleDigit` function to check for single digit integers  (#100064)
Switched to and reset branch 'main'

Objects/obmalloc.c:776:1: warning: ‘always_inline’ function might not be inlinable [-Wattributes]
  776 | arena_map_get(pymem_block *p, int create)
      | ^~~~~~~~~~~~~

make: *** [Makefile:1895: buildbottest] Error 3