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

Python.h doesn't follow C99 standard. #120293

Closed
ndparker opened this issue Jun 9, 2024 · 19 comments
Closed

Python.h doesn't follow C99 standard. #120293

ndparker opened this issue Jun 9, 2024 · 19 comments
Labels
build The build process and cross-build topic-C-API type-bug An unexpected behavior, bug, or error

Comments

@ndparker
Copy link
Contributor

ndparker commented Jun 9, 2024

Bug report

Bug description:

Hi,

I'm trying to test my C extensions with Python 3.13. For testing I usually enable as many error checks as possible. I'm seeing a couple of them. Python version is:

$ python3.13 -V
Python 3.13.0b1

Some errors are related to losing const qualifiers:

    /usr/include/python3.13/cpython/pyatomic_gcc.h: In function '_Py_atomic_load_ptr':
    /usr/include/python3.13/cpython/pyatomic_gcc.h:300:34: error: cast discards 'const' qualifier from pointer target type [-Werror=cast-qual]
      300 | { return (void *)__atomic_load_n((void **)obj, __ATOMIC_SEQ_CST); }
          |                                  ^
    /usr/include/python3.13/cpython/pyatomic_gcc.h: In function '_Py_atomic_load_ptr_relaxed':
    /usr/include/python3.13/cpython/pyatomic_gcc.h:359:34: error: cast discards 'const' qualifier from pointer target xt (line 8))

Some related to C99 compatibility (not sure what the current expected compat version is):

    /usr/include/python3.13/cpython/code.h:32:10: error: ISO C99 doesn't support unnamed structs/unions [-Werror=pedantic]
       32 |         };

Thanks,

CPython versions tested on:

3.13

Operating systems tested on:

Linux

@ndparker ndparker added the type-bug An unexpected behavior, bug, or error label Jun 9, 2024
@eli-schwartz
Copy link
Contributor

Previously: #105059

git blame fingers 060a96f / #117144

@eli-schwartz
Copy link
Contributor

It's trivial to catch.

$ cat /tmp/foo.c 
#include <Python.h>
$ 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

If the objective is to continue supporting external users of Python.h that unlike cpython, haven't upgraded to c11 -- then this seems like a useful unittest to add.

@sobolevn
Copy link
Member

cc @vstinner as of #105059

@sobolevn sobolevn added the build The build process and cross-build label Jun 11, 2024
@vstinner
Copy link
Member

The Python C API doesn't support -Werror=pedantic compiler flag. I don't think that it's a flag that we want to support. I suggest closing the issue as "WONT FIX".

@vstinner
Copy link
Member

If I try to build test_cext with that flag, the compiler fails early:

building 'cext' extension
gcc -fno-strict-overflow -Wsign-compare -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -Og -Wall -fPIC -I/home/vstinner/python/main/env/include -I/home/vstinner/python/main/Include -I/home/vstinner/python/main -c extension.c -o build/temp.linux-x86_64-cpython-314-pydebug/extension.o -Werror -Werror=pedantic -Werror=declaration-after-statement -DMODULE_NAME=cext
extension.c:56:19: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
   56 |     {Py_mod_exec, (void*)_testcext_exec},
      |                   ^

@vstinner vstinner changed the title C extension compilation errors with various checks enabled C extension compilation errors with various checks enabled (-Werror=pedantic) Jun 11, 2024
@ndparker
Copy link
Contributor Author

Hmm. -pedantic support is not documented in PEP 7. Any pointers for a documentation here?

That being said - the change of title is not right. It was one -pedantic error (with C99 - it does work with C11, thatswhy the question about version compat). However, the first error is related to -Wcast-qual (and definitely seems valid to me).

@ndparker
Copy link
Contributor Author

If I try to build test_cext with that flag, the compiler fails early:

building 'cext' extension
gcc -fno-strict-overflow -Wsign-compare -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -Og -Wall -fPIC -I/home/vstinner/python/main/env/include -I/home/vstinner/python/main/Include -I/home/vstinner/python/main -c extension.c -o build/temp.linux-x86_64-cpython-314-pydebug/extension.o -Werror -Werror=pedantic -Werror=declaration-after-statement -DMODULE_NAME=cext
extension.c:56:19: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
   56 |     {Py_mod_exec, (void*)_testcext_exec},
      |                   ^

I'd like to add that it's not about compiling my extension, I can deal with that. It's simply about including Python.h.

@vstinner
Copy link
Member

I'd like to add that it's not about compiling my extension, I can deal with that.

What I mean is that it seems impossible to build any non-trivial C extension with -Werror=pedantic, since Python requires syntax which is denied by -Werror=pedantic. Like my example.

My example comes from Lib/test/test_cext/extension.c which is a trivial C extension which does basically nothing.

@encukou
Copy link
Member

encukou commented Jun 12, 2024

IMO, the headers should definitely compile in pedantic mode with -std=c11, since that checks we actually use the standard we chose.
Possibly with C99 too. IMO, having an anonymous union in the public headers is a regression in 3.13.

@ndparker
Copy link
Contributor Author

FWIW, I'm casting the member functions like this and gcc doesn't complain:

#define EXT_CFUNC(func) ((PyCFunction)(void (*) (void))(func))

[...]
PyDoc_STRVAR(myfunc__doc__,
"myfunc(self)...");
[...]
static PyObject *
myfunc(mytype_t *self)
{
    ...
    return someobject;
}

[...]
static PyMethodDef mytype_methods[] = {
[...]
    {"somename", EXT_CFUNC(myfunc), METH_NOARGS, myfunc__doc__},
[...]

I looked into the headers, to verify what I'm doing here and I might have just stolen that from _PyCFunction_CAST (methodobject.h) a long time ago. I don't remember.

@ndparker
Copy link
Contributor Author

And again, to make sure it doesn't get lost in the discussion: I've started off with another error not related to -pedantic:

    /usr/include/python3.13/cpython/pyatomic_gcc.h: In function '_Py_atomic_load_ptr':
    /usr/include/python3.13/cpython/pyatomic_gcc.h:300:34: error: cast discards 'const' qualifier from pointer target type [-Werror=cast-qual]
      300 | { return (void *)__atomic_load_n((void **)obj, __ATOMIC_SEQ_CST); }
          |                                  ^
    /usr/include/python3.13/cpython/pyatomic_gcc.h: In function '_Py_atomic_load_ptr_relaxed':
    /usr/include/python3.13/cpython/pyatomic_gcc.h:359:34: error: cast discards 'const' qualifier from pointer target xt (line 8))

Should I open another issue for that one?

@vstinner
Copy link
Member

Good idea, please open a separated issue for the cast issue in pyatomic_gcc.h.

@ndparker
Copy link
Contributor Author

Good idea, please open a separated issue for the cast issue in pyatomic_gcc.h.

Done.

@ndparker ndparker changed the title C extension compilation errors with various checks enabled (-Werror=pedantic) Python.h doesn't follow C99 standard. Jun 16, 2024
@ndparker
Copy link
Contributor Author

I've changed the title to reflect the actual issue I was seeing (modulo the const messup).

@vstinner
Copy link
Member

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

I fixed this issue with 9e4a81f by moving private PyCode and PyOptimizer types to the internal C API.

@vstinner
Copy link
Member

Good idea, please open a separated issue for the cast issue in pyatomic_gcc.h.
Done.

It's the issue #120593

@vstinner
Copy link
Member

I fixed this issue with 9e4a81f by moving private PyCode and PyOptimizer types to the internal C API.

I tested manually and I confirm that the "ISO C99 doesn’t support unnamed structs/unions" warnings have been fixed (in code.h). I close the issue.

See the remaining issue #120593 for the const qualifier.

@ndparker
Copy link
Contributor Author

Thank you!

@vstinner
Copy link
Member

Thanks for your report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build The build process and cross-build topic-C-API type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants