-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
gh-142447: Fix cast warning in pycore_backoff.h #142465
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
Conversation
MAKE_VALUE_AND_BACKOFF() macro casts its result to uint16_t. Add pycore_backoff.h header to test_cppext tests.
colesbury
left a comment
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.
LGTM, but consider comment about test includes.
|
|
||
| #ifdef TEST_INTERNAL_C_API | ||
| // gh-135906: Check for compiler warnings in the internal C API | ||
| # include "internal/pycore_backoff.h" |
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.
Maybe better to include the headers greenlet actually uses:
internal/pycore_interpframe_structs.h
internal/pycore_interpframe.h
internal/pycore_tstate.h
pycore_backoff.h isn't included directly in greenlet.
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.
The problem is that including pycore_interpframe.h (for example) produces tons of compiler warnings from mimalloc, and I have no idea how to fix them. Examples of warning:
In file included from /usr/include/features.h:524,
from /usr/include/assert.h:35,
from /home/vstinner/python/main/Include/Python.h:20,
from extension.cpp:13:
/home/vstinner/python/main/Include/internal/mimalloc/mimalloc/types.h:73:33: error: expected unqualified-id before ‘noexcept’
73 | #define mi_decl_throw __THROW
| ^~~~~~~
/home/vstinner/python/main/Include/internal/mimalloc/mimalloc/types.h:611:31: note: in expansion of macro ‘mi_decl_throw’
611 | mi_decl_noreturn mi_decl_cold mi_decl_throw
| ^~~~~~~~~~~~~
In file included from /home/vstinner/python/main/Include/internal/pycore_mimalloc.h:44,
from /home/vstinner/python/main/Include/internal/pycore_tstate.h:14,
from /home/vstinner/python/main/Include/internal/pycore_code.h:13,
from /home/vstinner/python/main/Include/internal/pycore_interpframe.h:8,
from extension.cpp:18:
/home/vstinner/python/main/Include/internal/mimalloc/mimalloc/internal.h: In function ‘bool _mi_is_aligned(void*, size_t)’:
/home/vstinner/python/main/Include/internal/mimalloc/mimalloc/types.h:613:49: error: ‘_mi_assert_fail’ was not declared in this scope; did you mean ‘__assert_fail’?
613 | #define mi_assert(expr) ((expr) ? (void)0 : _mi_assert_fail(#expr,__FILE__,__LINE__,__func__))
| ^~~~~~~~~~~~~~~
/home/vstinner/python/main/Include/internal/mimalloc/mimalloc/types.h:619:31: note: in expansion of macro ‘mi_assert’
619 | #define mi_assert_internal mi_assert
| ^~~~~~~~~
/home/vstinner/python/main/Include/internal/mimalloc/mimalloc/internal.h:273:3: note: in expansion of macro ‘mi_assert_internal’
273 | mi_assert_internal(alignment != 0);
| ^~~~~~~~~~~~~~~~~~
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 going to be a problem in greenlet.
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.
@tacaswell: Do you get compiler warnings, or even compiler errors, from Python internal/mimalloc/ headers in greenlet?
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 only see one warning with cpython main after this PR was merged:
running build_ext
building 'greenlet._greenlet' extension
creating build/temp.linux-x86_64-cpython-315/src/greenlet
clang++ -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall -fPIC -I/home/tcaswell/.virtualenvs/cp315-clang/include -I/home/tcaswell/.pybuild/cp315-clang/include/python3.15 -c src/greenlet/greenlet.cpp -o build/temp.linux-x86_64-cpython-315/src/greenlet/greenlet.o
In file included from src/greenlet/greenlet.cpp:43:
src/greenlet/PyGreenlet.cpp:62:23: warning: unused variable 'c' [-Wunused-variable]
62 | UserGreenlet* c = new UserGreenlet(o, GET_THREAD_STATE().state().borrow_current());
| ^
1 warning generated.
clang++ -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall -shared -Wl,-rpath,/home/tcaswell/.pybuild/cp315-clang/lib build/temp.linux-x86_64-cpython-315/src/greenlet/greenlet.o -L/home/tcaswell/.pybuild/cp315-clang/lib -o build/lib.linux-x86_64-cpython-315/greenlet/_greenlet.cpython-315-x86_64-linux-gnu.so
building 'greenlet.tests._test_extension' extension
creating build/temp.linux-x86_64-cpython-315/src/greenlet/tests
clang -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall -fPIC -Isrc/greenlet/ -I/home/tcaswell/.virtualenvs/cp315-clang/include -I/home/tcaswell/.pybuild/cp315-clang/include/python3.15 -c src/greenlet/tests/_test_extension.c -o build/temp.linux-x86_64-cpython-315/src/greenlet/tests/_test_extension.o
clang -shared -Wl,-rpath,/home/tcaswell/.pybuild/cp315-clang/lib build/temp.linux-x86_64-cpython-315/src/greenlet/tests/_test_extension.o -L/home/tcaswell/.pybuild/cp315-clang/lib -o build/lib.linux-x86_64-cpython-315/greenlet/tests/_test_extension.cpython-315-x86_64-linux-gnu.so
building 'greenlet.tests._test_extension_cpp' extension
clang++ -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall -fPIC -Isrc/greenlet/ -I/home/tcaswell/.virtualenvs/cp315-clang/include -I/home/tcaswell/.pybuild/cp315-clang/include/python3.15 -c src/greenlet/tests/_test_extension_cpp.cpp -o build/temp.linux-x86_64-cpython-315/src/greenlet/tests/_test_extension_cpp.o
clang++ -fno-strict-overflow -Wsign-compare -Wunreachable-code -DNDEBUG -g -O3 -Wall -shared -Wl,-rpath,/home/tcaswell/.pybuild/cp315-clang/lib build/temp.linux-x86_64-cpython-315/src/greenlet/tests/_test_extension_cpp.o -L/home/tcaswell/.pybuild/cp315-clang/lib -o build/lib.linux-x86_64-cpython-315/greenlet/tests/_test_extension_cpp.cpython-315-x86_64-linux-gnu.so
installing to build/bdist.linux-x86_64/wheel
greenlet also includes internal/pycore_frame.h (the GREENLET_PYXXX guards are "that version or newer"), not sure if that is relevant.
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.
Ok, thanks for checking!
greenlet also includes internal/pycore_frame.h (the GREENLET_PYXXX guards are "that version or newer"), not sure if that is relevant.
test_cppext already checks that pycore_frame.h doesn't emit compiler warnings.
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.
greenlets imports pycore_farem.h first.
MAKE_VALUE_AND_BACKOFF() macro casts its result to uint16_t.
Add pycore_backoff.h header to test_cppext tests.