python / peps

Python Enhancement Proposals
https://peps.python.org
4.35k stars 1.5k forks source link

PEP 7: Public C API should be compatible with C99 #3862

Closed vstinner closed 4 days ago

vstinner commented 1 month ago

đź“š Documentation preview đź“š: https://pep-previews--3862.org.readthedocs.build/

colesbury commented 1 month ago

Huh? The public headers are not C99 or C++03 compatible:

warsaw commented 1 month ago

@colesbury are you saying that mimalloc is a blocker for this PR?

colesbury commented 1 month ago

@warsaw, mimalloc is not part of the public headers so I don't think it's relevant.

The pyatomic.h header is used by the refcounting functions in the free-threaded build. These are mostly static inline functions exposed by the public C API headers. There isn't a ISO C99 replacement here.

The PyObject struct definition relies on either C11 or non-standard extensions. Changing this to a non-anonymous union risk breaking backwards compatibility.

If the intention is that new code in public headers should be ISO C99 and ISO C++03 compatible, that's probably fine, but I think it'd be worth saying so explicitly in the PEP.

(See also https://github.com/capi-workgroup/decisions/issues/30)

vstinner commented 1 month ago

@colesbury: I updated the PEP to apply @encukou's suggestion: https://github.com/capi-workgroup/decisions/issues/30#issuecomment-2236043973

vstinner commented 1 month ago

cc @encukou

encukou commented 1 month ago

Looks good to me, though the PEP authors have the final say here. Long-term, let's move this out of the style guide altogether, into the C API PEP. This is not a rule you should break if it “would make the code less readable” :)

vstinner commented 1 month ago

@warsaw @gvanrossum: Are you ok with these changes?

gvanrossum commented 1 month ago

I've lost track of the discussion -- why can't we just say "we require C11"? (And the equivalent C++, which is what?) Maybe the reason for this should be spelled out in the PEP so it will be easier to revise in the future.

warsaw commented 1 month ago

Good question @gvanrossum . Also, I wonder if the "C dialect" section could be better represented as a table, with Python version along one dimension, and C dialect, C++ dialect, and additional notes along the other?

encukou commented 1 month ago

This PR is meant to document/formalize the status quo, not to be progressive.

Practically, in header code, the intersection of C99, C++09, and C11-without-optional-features is very close to “C89 with several select C99 features” we had for 3.6-3.10. But it's much easier to test. Whenever we break compatibility here, someone complains, and in each individual case it's usually easier to revert than to push for C11.

There was a discussion about this on Discourse; also see the hoops we currently jump through to have a C11 feature in the headers.

Enabling a compiler feature changes the behaviour of my entire program, not just the CPython headers. As it turns out, there exist some huge beasts that use Python as a library, and are quite sensitive to compiler options. (Steve might nowadays be free to say what his motivating example was? Just to fuel speculation, Python in MS Excel was announced shortly after the discussion.)

gvanrossum commented 1 month ago

Just to fuel speculation, Python in MS Excel was announced shortly after the discussion.

I don't think that's it -- in that architecture Python runs in a separate container. But I trust there are Microsoft projects not owned by Steve that are very sensitive to this stuff and unwilling/unable to change.

OTOH Victor's argument sounds more like speculation (exactly what Greg challenged earlier in that thread).

I am still uncomfortable with leaving the list of C11 features we use beyond C99/C++03 unspecified. I think we should either bite the bullet and stop using those, or otherwise document a timeline (say, 5 years) for changing the required standard level to C11/C++11 without optional features (or using the optional features only when they are available).

As the PR is currently phrased, it doesn't give enough (enforceable) guidelines to either the users of the public C API or its maintainers.

encukou commented 1 month ago

I agree that we should switch to C11 eventually. Let's have that discussion again; you mainly need to convince Steve. (But if we document a timeline so that users can prepare for a migration, let's not put it in a C style guide...)

As the PR is currently phrased, it doesn't give enough (enforceable) guidelines to either the users of the public C API or its maintainers.

AFAIK, PEP 7 is meant for the maintainers. Users expect their builds to not break with new Python versions; this is how we give them that.

The rules are already enforced -- they're tested [c99, c++03], and regressions that aren't caught by the tests have been reverted (since we don't want to break users).

I am still uncomfortable with leaving the list of C11 features we use beyond C99/C++03 unspecified.

That would mainly be useful for users. Those can try compiling <Python.h> with whatever compiler/flags they need. If we want to promise something to users, let's do that separately from guidelines for core devs.

vstinner commented 1 month ago

I created a separated PR to require C11 and C++11: https://github.com/python/peps/pull/3896

vstinner commented 4 days ago

The discussion https://github.com/capi-workgroup/decisions/issues/30 is still on-going, I prefer to close my PR for now.