Skip to content

C standard for public headers #30

Closed

Description

@encukou

Previous discussion: capi-workgroup/problems#42 & capi-workgroup/api-evolution#22

It seems the consensus of the people involved is that Python.h should be usable with:

  • C99 or newer
  • C++11 or newer

Now that there's a report about the 3.13 headers containing a C11 feature (anonymous structs/unions), I guess it's time to make an official guideline.

Activity

  1. vstinner commented on Jun 13, 2024

    @vstinner

    The C API is now tested by test_cext and test_cppext: these tests checks that including <Python.h> does not issue any compiler warning.

    test_cext has 5 tests using -Werror -Werror=declaration-after-statement:

    • C API (with no compiler flags)
    • C API with C99
    • C API with C11
    • Limited C API (with no compiler flags)
    • Limited C API with C11

    test_cppext has 4 tests using -Werror:

    • C API (with no compiler flags)
    • C API with C++03
    • C API with C++11
    • C API with C++14 (only on Windows)

    Without extra compiler flags, we're good. The problem of the issue gh-120293 are the usage of -Werror=pedantic and -Werror=cast-qual. I don't think that we want to support these flags.

  2. encukou commented on Jun 14, 2024

    @encukou
    CollaboratorAuthor

    This issue is not about the tests, but about the C standard we want the headers to support.
    After we decide that, we can test as much of it as we can, and make triage easy for issues about untestable things. But IMO we should write this down explicitly; it shouldn't be “whatever the tests do”.

    Again, the tests currently pass even though the headers use a feature that's not in the C99 standard. If the intent is to follow the standard, then the test should change.

  3. vstinner commented on Jun 14, 2024

    @vstinner

    Again, the tests currently pass even though the headers use a feature that's not in the C99 standard.

    Well, "standard" is one thing, "what's being used in practice" is something else. No C compiler respect the standard by default, they add "compiler extensions", and require special compiler options to respect the standard.

    For example, unnamed unions are supposed by all C compilers used by Python (GCC, clang, MSC) in C99 mode.

    So the question is if we care about "strict standard" or "what's being used in practice by C compilers supported by Python" (so including compiler extensions).

    In practice, the question is if we want to support -Werror=pedantic or not.

    I proposed to not support -Werror=pedantic nor -Werror=cast-qual options. It's too much work, Python has more and more static inline functions, and it's tricky to respect the "pedantic" mode.

  4. encukou commented on Jun 14, 2024

    @encukou
    CollaboratorAuthor

    So, you're saying that we should only try to be compatible with specific compilers (and compiler options), rather than C standards?

    Python has more and more static inline functions

    I don't think supporting C89 is on the table, so static inline functions aren't really relevant.

  5. vstinner commented on Jun 14, 2024

    @vstinner

    So, you're saying that we should only try to be compatible with specific compilers (and compiler options), rather than C standards?

    Yes.

    I don't think supporting C89 is on the table, so static inline functions aren't really relevant.

    I was referring to the fact that static inline function bodies expose a lot of code and usually compiler problems come from such code. For example, of pyatomic functions would be regular opaque functions, we wouldn't have this discussion about -Werror=cast-qual. In the past, we also got many cast issues with C++. I also had to introduce _Py_NULL to use nullptr in C++ in our Python C API :-)

  6. encukou commented on Jun 14, 2024

    @encukou
    CollaboratorAuthor

    Well, I don't see why you brought up -Werror=cast-qual here. If we support specific compiler options, adding that one should be a separate conversation.

    Anyway, I think I understand your opinion now; I just disagree with it. I guess it's time for others to chime in.

  7. gvanrossum commented on Jun 14, 2024

    @gvanrossum

    I am confused though. Can you cleanly summarize the two opinions on the table?

  8. eli-schwartz commented on Jun 16, 2024

    @eli-schwartz
    $ cat /tmp/foo.c
    #include <Python.h>
    

    With c99:

    $ gcc -std=c99 -Werror=pedantic -I/usr/include/python3.13 -c /tmp/foo.c -o /tmp/foo.o
    In file included from /usr/include/python3.13/Python.h:92,
                     from /tmp/foo.c:1:
    /usr/include/python3.13/cpython/code.h:32:10: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       32 |         };
          |          ^
    /usr/include/python3.13/cpython/code.h:34:6: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       34 |     };
          |      ^
    /usr/include/python3.13/cpython/code.h:27:9: error: struct has no named members [-Werror=pedantic]
       27 | typedef struct {
          |         ^~~~~~
    In file included from /usr/include/python3.13/Python.h:128:
    /usr/include/python3.13/cpython/optimizer.h:60:14: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       60 |             };
          |              ^
    /usr/include/python3.13/cpython/optimizer.h:62:10: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       62 |         };
          |          ^
    /usr/include/python3.13/cpython/optimizer.h:63:6: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       63 |     };
          |      ^
    cc1: some warnings being treated as errors
    

    With gnu99:

    $ gcc -std=gnu99 -Werror=pedantic -I/usr/include/python3.13 -c /tmp/foo.c -o /tmp/foo.o
    In file included from /usr/include/python3.13/Python.h:92,
                     from /tmp/foo.c:1:
    /usr/include/python3.13/cpython/code.h:32:10: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       32 |         };
          |          ^
    /usr/include/python3.13/cpython/code.h:34:6: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       34 |     };
          |      ^
    /usr/include/python3.13/cpython/code.h:27:9: error: struct has no named members [-Werror=pedantic]
       27 | typedef struct {
          |         ^~~~~~
    In file included from /usr/include/python3.13/Python.h:128:
    /usr/include/python3.13/cpython/optimizer.h:60:14: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       60 |             };
          |              ^
    /usr/include/python3.13/cpython/optimizer.h:62:10: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       62 |         };
          |          ^
    /usr/include/python3.13/cpython/optimizer.h:63:6: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]
       63 |     };
          |      ^
    cc1: some warnings being treated as errors
    

    With c11:

    $ gcc -std=c11 -Werror=pedantic -I/usr/include/python3.13 -c /tmp/foo.c -o /tmp/foo.o
    $
    

    @vstinner,

    Well, "standard" is one thing, "what's being used in practice" is something else. No C compiler respect the standard by default, they add "compiler extensions", and require special compiler options to respect the standard.

    This has nothing to do with "compiler extensions". It is about the fact that some compilers, like GCC, will allow any code that is legal by a later standard to also compile with a newer standard if the alternative is an error. The theory is that "well you know, your meaning is obvious, you wrote code that is legal in c23 but not c17, so clearly you meant to build with c17". This doesn't actually mean it's safe to ignore the fact that it compiles by default.

    You can get the same issue if you upgrade to future gcc 15, and start using c23 features. As long as there isn't an incompatible alternative meaning in earlier standards, the compiler typically accepts it. Hopefully with a warning that you're doing something which isn't ISO C99.

    ... which brings us back to using -Werror=pedantic, since that offers a chance to catch cases where you're using code that isn't legal for a std but the compiler is accepting it anyways "to be nice".

    If it was a compiler extension it would be allowed when compiling with "GNU C", the collection of GCC vendor extensions on top of standard C. But it is not.

  9. eli-schwartz commented on Jun 16, 2024

    @eli-schwartz

    I am confused though. Can you cleanly summarize the two opinions on the table?

    If I understand correctly, the options are:

    • require per policy that the headers to be compatible with any conforming C99 compiler, and check this as best one can, using the compilers available in CI (-Werror=pedantic can give you advance warning in many cases)
    • require per policy that the headers be compatible with officially supported compilers (gcc, clang, MSVC) when given the -std=c99 option to tell the compiler to perform any dealbreaker decisions by conforming with the c99 interpretation rather than the c23 interpretation.

    Option 2 requires mandating specific compilers, and also specific compiler versions (e.g. require that GCC is at least "whichever version of GCC first implemented c11 unnamed unions").

  10. vstinner commented on Jun 17, 2024

    @vstinner

    code.h:32:10: error: ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]

    The problem is not specific to code.h. I added __extension__ (GCC/clang) and __pragma(warning(disable: 4201)) (MSC) to object.h to make a similar warning quiet in the Free Threading mode, on the unnamed union:

    #ifndef Py_GIL_DISABLED
    struct _object {
    #if (defined(__GNUC__) || defined(__clang__)) \
            && !(defined __STDC_VERSION__ && __STDC_VERSION__ >= 201112L)
        // On C99 and older, anonymous union is a GCC and clang extension
        __extension__
    #endif
    #ifdef _MSC_VER
        // Ignore MSC warning C4201: "nonstandard extension used:
        // nameless struct/union"
        __pragma(warning(push))
        __pragma(warning(disable: 4201))
    #endif
        union {
           Py_ssize_t ob_refcnt;
    #if SIZEOF_VOID_P > 4
           PY_UINT32_T ob_refcnt_split[2];
    #endif
        };
    #ifdef _MSC_VER
        __pragma(warning(pop))
    #endif
    
        PyTypeObject *ob_type;
    };
    #else
    (...)
    #endif
    

    So it seems like C11 is what we need/want here.

  11. vstinner commented on Jun 17, 2024

    @vstinner

    Now that there's a python/cpython#120293 about the 3.13 headers containing a C11 feature (anonymous structs/unions), I guess it's time to make an official guideline.

    I suggest to require C11 and C++03 at minimum to use the Python C API.

  12. encukou commented on Jun 17, 2024

    @encukou
    CollaboratorAuthor

    @guido, sorry for the delay.

    I am confused though. Can you cleanly summarize the two opinions on the table?

    1. Document the standards we support.

    The C API guidelines will list the C/C++ standards that #include <Python.h> is meant to comply with. The list is open for nitpicking; I propose:

    • C99 or newer
    • C++11 or newer
    • (possibly: C89 with several select C99 features, as in PEP 7)

    We'd welcome PRs to improve conformance, and to make the tests better in verifying conformance. (Within reason of course -- the guidelines can always be changed.)

    2. Don't.

    We support specific compilers, as listed in PEP 11. The supported compiler options are defined in the tests/CI.

    (That's my understanding; I'll let Victor clarify.)

  13. vstinner commented on Jun 17, 2024

    @vstinner
    1. Don't.
      We support specific compilers, as listed in PEP 11. The supported compiler options are defined in the tests/CI.
      (That's my understanding; I'll let Victor clarify.)

    That's not my intent. I mean that in practice, we can support C99 with compiler extensions, such as __extension__.

  14. vstinner commented on Jun 17, 2024

    @vstinner

    I mean that in practice, we can support C99 with compiler extensions, such as extension.

    Anyway, as I wrote previously, I agree with Petr and I suggest to require C11 to get unnamed unions and other nice C11 features.

  15. zooba commented on Jun 17, 2024

    @zooba

    I suggest to require C11 to get unnamed unions and other nice C11 features

    I don't think we can reasonably require it in our headers (any of the headers that we distribute, "internal" or not), until the build backends used by package publishers have released stable versions that automatically add in the compiler options necessary to enable these modes.

    This isn't a decision that affects us - it affects our users, who will suddenly find that they cannot compile with our newest releases due to code that they can't change.

    I'd prefer to jump through as many hoops as possible to avoid breaking our users like that. I think it's worth the effort to not use these features in public header files (again, whether "internal" or not) so that we don't put the burden on our users. As far as I'm aware, nothing is broken for us, we just have to do a little bit more typing in our own code.

  16. 32 remaining items

  17. zooba commented on Jan 24, 2025

    @zooba
    • I'd rather not make promises that can be broken with a compiler update.

    It's more a promise that we'll fix it (and a reminder to whoever is editing headers right now that they've got to suppress warnings they may introduce). And Victor has added tests already.

    • “Max settings” is vague

    Deliberately, yeah. For MSVC I'd mean /W4, but I'm pretty sure that isn't universal.

  18. encukou commented on Jan 27, 2025

    @encukou
    CollaboratorAuthor

    Deliberately, yeah.

    For the purpose of winning arguments, is there anything in “Max settings” that “C11 and C++11” doesn't catch?

    “Max settings” makes sense in this conversation, but I think the public version would need to be much longer. Or do you have a reasonable wording in mind?
    Ideally one that makes it clear that we don't mean options like -Wpadded.

    And Victor has added tests already.

    Not the /W4 and -pedantic-errors/-Wextra ones, AFAIK. (edit: Oh, I see the /W4 now)

  19. zooba commented on Jan 27, 2025

    @zooba

    Ideally one that makes it clear that we don't mean options like -Wpadded.

    But I do mean options like -Wpadded :) We shouldn't produce that warning if it's turned on, and if someone reports that we do, then we should fix it.

    I'm not massively concerned about eagerly going through and getting warning clean - if we state a policy then people will do that for us 😉 - but we should decide in advance what our response will be if someone comes to us asking to prevent that warning.

    As for wording, I think we can say something like "most strict warning preset offered by the compiler, such as ... on <compiler>".

  20. encukou commented on Jan 27, 2025

    @encukou
    CollaboratorAuthor

    I disagree here: I think that if someone asks for -Wpadded, they should get the warnings for Python headers, because our structs do include padding, and I don't think we want to change that. IMO, the correct reaction would be disabling the warning for Python.h, from the user's code.

    But either way, I think this is out of scope for this issue (“C standard for public headers”). Would you be OK doing it in a future step, or do you think “both of C11 and C++11” is unacceptable?

  21. zooba commented on Jan 27, 2025

    @zooba

    they should get the warnings for Python headers, because our structs do include padding, and I don't think we want to change that.

    They can't fix it, because we've already built with padding, and so the structs have to reflect reality at the point where a consumer is using our headers. Anything that would require edits to our code (including our headers) should be suppressed, and we shouldn't force users to figure that out themselves (how many places are they going to #include <Python.h>?).

    do you think “both of C11 and C++11” is unacceptable?

    I think it's too easy to [mis]interpret in ways that people can use to justify making changes to the headers that will break users. We aren't requiring those language specs literally, we're requiring the feature sets, and if default compiler extensions to C99 or C++03 make things work (as they currently do) then they should be allowed to continue working.

    If we want to get to those language versions specifically (i.e. require /std:c11 to compile) then we should add deprecation warnings now and schedule a changeover for 3.16. If that's not the goal, then we should find wording that doesn't give contributors permission to assume those language versions are definitely active.

  22. encukou commented on Jan 28, 2025

    @encukou
    CollaboratorAuthor

    OK, I now see where we disagree.
    The main reason I want to test -pedantic &c. is because, as a standard-checking mode, it approximates compilers that we don't have in the CI. Maybe it's a questionable approximation, but it's a testable one.

    However, if the user requests additional warnings, that's a different situation in my eyes. I don't think we should be silently overriding that preference.

    Well, I think we're out of scope of a “decision”. Do you want to start a Discourse discussion for “max settings”? I might not be able to word your proposal well.


    I think it's too easy to [mis]interpret in ways that people can use to justify making changes to the headers that will break users.

    We have tests for that.
    We really need to separate public docs from contribution guidelines; mixing them is not working well at all.

    Anything that would require edits to our code (including our headers) should be suppressed

    Ideally no edit would be needed, you should be OK with a pragma push/pop, marking code where you don't want the additional diagnostics.

    (how many places are they going to #include <Python.h>?).

    If you ask me: ideally, one :)

  23. zooba commented on Jan 28, 2025

    @zooba

    However, if the user requests additional warnings, that's a different situation in my eyes. I don't think we should be silently overriding that preference.

    The problem is that they'll be requesting them for their own code, but due to how includes work, our code inevitably is covered by that. But they don't want to know about issues in our code, just their own. That's why ours should override those preferences (in places where we've already seen the warning ourselves and verified that we shouldn't just fix our own code, which is why I'm not keen on the idea of broad suppressions for all of our headers, even though I suspect that's inevitably where we'll end up).

    We have tests for that.
    We really need to separate public docs from contribution guidelines; mixing them is not working well at all.

    So the public docs will say "our headers don't use anything later than C11 or C++11, and if your compiler has a mode for these language versions you don't have to enable it", and our contribution guidelines will list the exact features we're allowed to use? That works for me, but I'm not sure it's what you're proposing.

    Ideally no edit would be needed, you should be OK with a pragma push/pop, marking code where you don't want the additional diagnostics.

    Provided we write the push/pop into our own code, yes, that's fine. We shouldn't make our users write that themselves around our header.

    If you ask me: ideally, one :)

    Well, guess who's going to be disappointed then 😉

  24. encukou commented on Jan 28, 2025

    @encukou
    CollaboratorAuthor

    So the public docs will say "our headers don't use anything later than C11 or C++11, and if your compiler has a mode for these language versions you don't have to enable it", and our contribution guidelines will list the exact features we're allowed to use? That works for me, but I'm not sure it's what you're proposing.

    Yes, that's what I meant for the vote.
    Where possible, the contribution guidelines should be backed by tests. (Specifically, I'm thinking about c99 -pedantic or equivalent with individual warnings disabled in the headers themselves -- but let's save the details for the review.)


    The “max settings” conversation is too interesting; can we split it out of here?

  25. zooba commented on Jan 29, 2025

    @zooba

    Sure. I don't have a particular place to split it to, though. Is it something we need to decide (and put... where?)

  26. encukou commented on Feb 4, 2025

    @encukou
    CollaboratorAuthor

    Thanks for the vote, everyone! I opened a CPython issue python/cpython#129666.

    Sure. I don't have a particular place to split it to

    I think Discourse would be best.
    I thought this issue (C standard) is mostly a technicality, plus it was already discussed publicly in https://discuss.python.org/t/requiring-compilers-c11-standard-mode-to-build-cpython/26481/25, so I went directly to a vote. In hindsight, a Discourse topic would have been better.

    Silencing all known warnings probably will be more controversial (at least I hope so); IMO we should talk it out in public.
    Let me know if you want me to start the topic.

  27. zooba commented on Feb 6, 2025

    @zooba

    Let me know if you want me to start the topic.

    Go ahead. Just present it as "avoiding all known warnings" rather than "silencing" 😉 The point is to write reliable, portable code that can be used in a wide variety of circumstances, not merely to shut up the complaints.

  28. vstinner commented on May 28, 2025

    @vstinner

    Can we close this issue? The decision was taken, no?

  29. encukou commented on Aug 12, 2025

    @encukou
    CollaboratorAuthor

    Yeah.
    Sorry @zooba, I couldn't find a wording that doesn't sound silly or vague. Please start the discussion if you have a good wording in mind.
    But, since we already test with /W4 and -pedantic-errors, I don't really see a practical change to discuss.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    voteThe WG is voting (or has voted)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions