Skip to content
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.

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

gh-106320: Remove private _PyLong_IsCompact() function #108742

Closed
wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Sep 1, 2023

Move the private _PyLong_IsCompact() and _PyLong_CompactValue() functions to the internal C API (pycore_long.h). Public PyUnstable_Long_IsCompact() and PyUnstable_Long_CompactValue() functions can be used instead.

Remove "const" qualifier from PyUnstable_Long_IsCompact() and PyUnstable_Long_CompactValue() parameter type.


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

@vstinner
Copy link
Member Author

vstinner commented Sep 1, 2023

With this change, PyUnstable_Long_IsCompact() and PyUnstable_Long_CompactValue() become regular opaque function calls. They are no longer implemented as static inline functions. That's needed to be able remove the 2 associated static inline functions from the public C API.

Remove "const" qualifier from PyUnstable_Long_IsCompact() and PyUnstable_Long_CompactValue() parameter type.

When I tried to use the const qualified in the public C API in the past, I had issues :-( I prefer to avoid it. For the internal C API with static inline functions, that's fine.

See:

Move the private _PyLong_IsCompact() and _PyLong_CompactValue()
functions to the internal C API (pycore_long.h). Public
PyUnstable_Long_IsCompact() and PyUnstable_Long_CompactValue()
functions can be used instead.

Remove "const" qualifier from PyUnstable_Long_IsCompact() and
PyUnstable_Long_CompactValue() parameter type.
@vstinner vstinner force-pushed the remove_pylong_iscompat branch from 1969769 to c6be6f8 Compare September 1, 2023 07:15
@vstinner vstinner added the needs backport to 3.12 bug and security fixes label Sep 1, 2023
@vstinner
Copy link
Member Author

vstinner commented Sep 1, 2023

@gvanrossum @markshannon: Are the private _PyLong_IsCompact() and _PyLong_CompactValue() functions exposed by <Python.h> on purpose? Or are you are ok to move them to the internal C API?

I don't know the impact on performance. I'm more in favor of hiding implementation details.

If these functions are moved to the internal C API, are you ok to also make this change in Python 3.12?

Remove "const" qualifier from PyUnstable_Long_IsCompact() and PyUnstable_Long_CompactValue() parameter type.

I don't have a strong opinion on that, I'm also fine with keeping const if you prefer :-) It's just that in the past, I tried to use const in the public C API and "I got issues".

@gvanrossum
Copy link
Member

Please don’t touch these. They are meant to be exported.

@gvanrossum
Copy link
Member

Also it’s too late for 3.12.

@vstinner
Copy link
Member Author

vstinner commented Sep 1, 2023

Please don’t touch these. They are meant to be exported.

Ok, let's keep them.

@vstinner vstinner closed this Sep 1, 2023
@vstinner vstinner deleted the remove_pylong_iscompat branch September 1, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants