Skip to content

gh-130947: Add again PySequence_Fast() to the limited C API #130948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Mar 13, 2025
Merged

gh-130947: Add again PySequence_Fast() to the limited C API #130948

merged 2 commits into from
Mar 13, 2025

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Mar 7, 2025 •
edited by github-actions bot
Loading

Add again PySequence_Fast(), PySequence_Fast_GET_SIZE() and PySequence_Fast_GET_ITEM() to the limited C API

Add an unit tests.

📚 Documentation preview 📚: https://cpython-previews--130948.org.readthedocs.build/

@vstinner vstinner requested a review from encukou as a code owner March 7, 2025 13:50
@bedevere-app bedevere-app bot mentioned this pull request Mar 7, 2025
Closed
@vstinner
Copy link
Member Author

vstinner commented Mar 7, 2025

This PR basically does the opposite of PR #129398 which is part of Python v3.14.0a5 release. Instead of removing these APIs from the limited C API, this PR fixes the limited C API implementation of PySequence_Fast_GET_SIZE() and PySequence_Fast_GET_ITEM() macros, and it adds again PySequence_Fast() to the limited C API.

PySequence_Fast_ITEMS() remains removed, since this function should be implemented in the limited C API.

What changed since PR #129398 is that I discovered (via a bug report) that PyQt6 uses PySequence_Fast(). Moreover PyQt6 reimplements PySequence_Fast_GET_SIZE() and PySequence_Fast_GET_ITEM() macros in the limited C API.

cc @serhiy-storchaka @encukou

@serhiy-storchaka
Copy link
Member

This should be considered a new feature and discussed with the C API Workgroup. Also, I do not think that it is right to replace fast macros with slower functions.

@vstinner
Copy link
Member Author

vstinner commented Mar 7, 2025

Also, I do not think that it is right to replace fast macros with slower functions.

Fast macros are not replaced with slower functions. In the regular C API, this PR changes nothing. It only changes the limited C API implementation to use function calls.

@vstinner
Copy link
Member Author

@serhiy-storchaka: Do you still consider that my change is a new feature after my comment? Should I raise the issue to the C API Working Group?

@vstinner
pythongh-130947: Add again PySequence_Fast() to the limited C API
8226691 
Add again PySequence_Fast() to the limited C API.

Add unit tests.
@vstinner
Copy link
Member Author

I modified my PR to not add again PySequence_Fast_GET_SIZE() and PySequence_Fast_GET_ITEM() to the limited C API. The PR now only adds PySequence_Fast() back to the limited C API.

@vstinner vstinner merged commit 10cbd1f into python:main Mar 13, 2025
42 checks passed
@vstinner vstinner deleted the seq_fast branch March 13, 2025 12:01
@bedevere-bot
Copy link

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

Hi! The buildbot AMD64 FreeBSD14 3.x (tier-3) has failed when building commit 10cbd1f.

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/#/builders/1232/builds/4948) and take a look at the build logs.
  4. Check if the failure is related to this commit (10cbd1f) 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/#/builders/1232/builds/4948

Failed tests:

  • test_interpreters

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

==

Click to see traceback logs 
Traceback (most recent call last):
  File "<frozen getpath>", line 358, in <module>
ValueError: embedded null byte
Warning -- Uncaught thread exception: InterpreterError
Exception in thread Thread-209 (task):
RuntimeError: error evaluating path


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/threading.py", line 1054, in _bootstrap_inner
    self.run()
    ~~~~~~~~^^
  File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/threading.py", line 996, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/test/test_interpreters/test_stress.py", line 47, in run
    interp = interpreters.create()
  File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/test/support/interpreters/__init__.py", line 76, in create
    id = _interpreters.create(reqrefs=True)
interpreters.InterpreterError: interpreter creation failed
k


Traceback (most recent call last):
  File "<frozen getpath>", line 358, in <module>
ValueError: embedded null byte
Warning -- Uncaught thread exception: InterpreterError
Exception in thread Thread-280 (run):
RuntimeError: error evaluating path


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/threading.py", line 1054, in _bootstrap_inner
    self.run()
    ~~~~~~~~^^
  File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/threading.py", line 996, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/test/test_interpreters/test_stress.py", line 30, in task
    interp = interpreters.create()
  File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/test/support/interpreters/__init__.py", line 76, in create
    id = _interpreters.create(reqrefs=True)
interpreters.InterpreterError: interpreter creation failed
k

@bedevere-bot
Copy link

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

Hi! The buildbot iOS ARM64 Simulator 3.x (tier-3) has failed when building commit 10cbd1f.

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/#/builders/1380/builds/2860) and take a look at the build logs.
  4. Check if the failure is related to this commit (10cbd1f) 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/#/builders/1380/builds/2860

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

==

Click to see traceback logs 
Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/A5AE3DFA-7F2B-46E5-A8FE-D560FAC86C8A/data/Containers/Bundle/Application/0EA2066B-8422-4F05-A6DD-2CCB7D5AF3E6/iOSTestbed.app/python/lib/python3.14/threading.py", line 1054, in _bootstrap_inner
    self.run()
    ~~~~~~~~^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/A5AE3DFA-7F2B-46E5-A8FE-D560FAC86C8A/data/Containers/Bundle/Application/0EA2066B-8422-4F05-A6DD-2CCB7D5AF3E6/iOSTestbed.app/python/lib/python3.14/threading.py", line 996, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/A5AE3DFA-7F2B-46E5-A8FE-D560FAC86C8A/data/Containers/Bundle/Application/0EA2066B-8422-4F05-A6DD-2CCB7D5AF3E6/iOSTestbed.app/python/lib/python3.14/test/test_interpreters/test_stress.py", line 47, in run
    interp = interpreters.create()
  File "/Users/buildbot/Library/Developer/XCTestDevices/A5AE3DFA-7F2B-46E5-A8FE-D560FAC86C8A/data/Containers/Bundle/Application/0EA2066B-8422-4F05-A6DD-2CCB7D5AF3E6/iOSTestbed.app/python/lib/python3.14/test/support/interpreters/__init__.py", line 76, in create
    id = _interpreters.create(reqrefs=True)
interpreters.InterpreterError: interpreter creation failed
k

@vstinner
Copy link
Member Author

I merged a minimum version of my change: only restore PySequence_Fast() function in the limited C API.

@serhiy-storchaka
Copy link
Member

Yes, I consider it as a new feature. And I will be against it.

plashchynski pushed a commit to plashchynski/cpython that referenced this pull request Mar 17, 2025
@vstinner @plashchynski
pythongh-130947: Add again PySequence_Fast() to the limited C API (py…
dfd10c1 
…thon#130948)

Add again PySequence_Fast() to the limited C API.

Add unit tests.
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
@vstinner @seehwan80
pythongh-130947: Add again PySequence_Fast() to the limited C API (py…
ee1f3c1 
…thon#130948)

Add again PySequence_Fast() to the limited C API.

Add unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@encukou encukou Awaiting requested review from encukou encukou is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vstinner @serhiy-storchaka @bedevere-bot