-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
PEP 7: Reword & split the "C dialect" section #4557
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
base: main
Are you sure you want to change the base?
Conversation
The motivation here is to avoid users reading PEP 7 as guarantees about what compilers/settings are required to build Python or extensions. I've also clarified a few things in line with current practice.
Common C code conventions | ||
========================= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any alliteration appropriate? Always, and all allotted alliteration allowance assured as apportioned! All adults are alliteration acclimatised as affably attested, accidental amelioration & aggravation apart. Avid alliterators adept at alliteration advance: abruptly, audaciously, ambitiously, and admirably!
Ahoy!
A
Alliterative and approved! A |
Co-authored-by: Adam Turner <[email protected]>
@warsaw & @gvanrossum, do you have thoughts on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good improvements overall!
There were two places where I didn't understand what you meant.
I also find it slightly odd to write prescriptive language (e.g. "should be compatible") while at the same time defending that with"but the intro allows exceptions". I'd rather weaken the language somewhat ("ought to"?), in the sake of clarity.
peps/pep-0007.rst
Outdated
For features that aren't in the relevant standard (like atomics or some C11 | ||
features in public headers), use CPython-specific wrappers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to follow the wikipedia link to optional features below to understand this. The point is that certain features are declared optional by the standard, and for those, if we want to use them, we must use CPython-specific headers. (But why not allow checking the standard's feature-check macros directly, if the main code just has to have two versions? Wrappers are useful if you'd be checking for the feature-check macros repeatedly (like with atomics) or if a shorthand macro could help not seeing the feature checks at all in the rest of the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not only about optional features. See #define Py_ARITHMETIC_RIGHT_SHIFT
for a nice example.
As for why wrappers:
If a wrapper already exists, you should of course use it. If it does not, you're in for a compiler compatibility study -- and the best place for the results of that is a comment on top of a macro, even if the macro is simple.
New C features tend to be added after a few implementations already exist as compiler extension, so many cases there are (almost) equivalent extensions we need to check as well. For things we add in the future, I expect that variants for GCC, MSVC & standard -- like with atomics -- will be quite normal.
I'll add examples here, they should make things clearer.
peps/pep-0007.rst
Outdated
(This is more restrictive than what we guarantee users. Remember that this PEP | ||
allows exceptions!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this means. Maybe a brief example would clarify it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a note for users, who sometimes take this out of context. I'll clarify.
C standards | ||
=========== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retain the old anchor:
C standards | |
=========== | |
.. _c_dialect: | |
C standards | |
=========== |
The motivation here is to avoid users reading PEP 7 as guarantees about what compilers/settings are required to build Python or extensions.
I've also clarified a few things in line with current practice. (And I couldn't resist some alliteration, lmk if suit & tie is required.)
📚 Documentation preview 📚: https://pep-previews--4557.org.readthedocs.build/pep-0007/