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

Submission for SC consideration: PEP 670 -- Convert macros to functions in the Python C API #90

Closed
vstinner opened this issue Nov 29, 2021 · 12 comments
Labels
PEP Python Enhancement Proposal

Comments

@vstinner
Copy link
Member

Hi,

The PEP 670 "Convert macros to functions in the Python C API" has been discussed for 1 month on python-dev. Erlend and me updated the PEP multiple times to address comments of Antoine Pitrou and Petr Viktorin.

The PEP is also based on past discussions on the bug tracker and pull requests. I tried to reply to arguments of Marc Andre Lemburg and Raymond Hettinger, and I added "Rejected Ideas: Keep macros, but fix some macro issues" which tries to describe Lemburg's point of view. I included benchmarks to answer to Raymond's worries about a possible performance slowdown.

Victor and Erlend @erlend-aasland

@vstinner
Copy link
Member Author

@brettcannon
Copy link
Member

I've added this to our rolling agenda.

@vstinner
Copy link
Member Author

I've added this to our rolling agenda.

Hi, is there any update on this PEP?

@pablogsal
Copy link
Member

I've added this to our rolling agenda.

Hi, is there any update on this PEP?

No but is scheduled to be discussed next week.

@vstinner
Copy link
Member Author

Hi, did you discuss this PEP in latest meetings? Do you have questions about it?

@brettcannon
Copy link
Member

We did discuss it, but we aren't done yet.

@vstinner
Copy link
Member Author

One of my motivation to push the PEP 670 is my plan to Slowly bend the C API towards the limited API to get a stable ABI for everyone. I didn't put it in the PEP because the relationship between the PEP 670 and the limited API/stable ABI is indirect, and the PEP alone is not enough to immediately make the whole default C API usable to get the stable ABI.

@erlend-aasland
Copy link

erlend-aasland commented Feb 8, 2022

The SC posted an announcement on python-dev recently. A copy of the announcement is inlined in this message for convenience.

https://mail.python.org/archives/list/python-dev@python.org/message/IJ3IBVY3JDPROKX55YNDT6XZTVTTPGOP/

Thank you for submitting PEP 670 (Convert macros to functions in the Python C API)! The steering council is still discussing it. We're not ready to accept the PEP as a whole, but there are parts on which we agree unanimously. To unblock the work, we're making the following pronouncements:

All other things being equal, static inline functions are better than macros.
Specifically, inline static functions should be preferred over function-like macros in new code. Please add a note about this to PEP 7.

The performance impact of changing macros to static inline functions is negligible.
The performance changes are indistinguishable from noise caused by other factors, such as layout changes and specific compiler optimizations.
It's possible that this isn't true in all cases. If any are found, they should be kept as macros and added to the benchmark suite to avoid regressions.
More generally, if there is a non-obvious reason for a conscious design decision, it should be explained (e.g. in a comment) to help future developers.

When there is no backwards compatibility issue, macros can be changed to static inline functions.
In effect, the SC accepts the first part of the PEP, except cases where:
a macro is converted to macro and function ("Cast to PyObject*"), and where the return value is removed.
Please do the changes in several smaller pull requests, don't skip reviews, and be prepared to revert if any issues are found.

The straightforward macros should be converted first. When they are, we hope that it will become easier to evaluate the remaining ones.

  • Petr, on behalf of the Steering Council

@brettcannon brettcannon added the PEP Python Enhancement Proposal label Feb 16, 2022
@vstinner
Copy link
Member Author

We posted a second version of the PEP which takes in account all the feedback that we received so far: https://mail.python.org/archives/list/python-dev@python.org/thread/VM6I3UHVMME6QRSUOYLK6N2OZHP454W6/ It better explains the requirement for a macro to not add new compiler warnings, and it restricts the PEP scope (even more) to really avoid any risk of backward compatibility.

@brettcannon
Copy link
Member

I've put the PEP back on our agenda.

@vstinner
Copy link
Member Author

@encukou reorganized the PEP (merge the two sections about performance) and clarified a few points: python/peps#2461

PEP 670 can be read at: https://peps.python.org/pep-0670/

@encukou
Copy link
Member

encukou commented Jun 20, 2022

The PEP is Accepted Final now, I just forgot to close the issue.
Acceptance mail: https://mail.python.org/archives/list/python-dev@python.org/thread/QQFCJ7LR36RUZSC3WI6WZZMQVQ3ZI4MS/

@encukou encukou closed this as completed Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PEP Python Enhancement Proposal
Projects
None yet
Development

No branches or pull requests

5 participants