-
-
Notifications
You must be signed in to change notification settings - Fork 30.7k
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
gh-107249: Implement Py_UNUSED() for MSVC #107250
Conversation
Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4". Example with this function included by Python.h: static inline unsigned int PyUnicode_IS_READY(PyObject* Py_UNUSED(op)) { return 1; } Without this change, building a C program with "cl /W4" which just includes Python.h emits the warning: Include\cpython/unicodeobject.h(199): warning C4100: '_unused_op': unreferenced formal parameter This change fix this warning.
@zooba @serhiy-storchaka @methane: Do you want to trade a review? I consider that it's a feature change, so it requires to be approved to be merged, following new Steering Council guidelines :-) |
Do you think that it would too late to backport it to Python 3.12? We are close to Python 3.12.0 beta 4 (Tuesday, 2023-07-11). I'm not sure if it's a bugfix or a feature :-/ But I would prefer to not backport it to Python 3.11 since I'm not 100% sure that my solution works in all cases on all MSVC versions. |
+1. |
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.
(void)name;
in function name might be the simplest solution.
But existing Py_UNUSED()
macro is used in function signature.
So I don't know better approach than this PR.
Thanks for the review @methane. I merged my PR.
Is it possible to put Well, Python 3.12 is already at beta stage. I prefer to not cause any regression. Let's wait until Python 3.13 is shipped. I don't think that this change is so important that it deserves to be backported. |
C++17 supports int test([[maybe_unused]] int arg) I am not sure, but maybe it can be also written as int test(int [[maybe_unused]] arg) or int test(int arg [[maybe_unused]]) If it is so, it can be utilized by |
Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4". Example with this function included by Python.h: static inline unsigned int PyUnicode_IS_READY(PyObject* Py_UNUSED(op)) { return 1; } Without this change, building a C program with "cl /W4" which just includes Python.h emits the warning: Include\cpython/unicodeobject.h(199): warning C4100: '_unused_op': unreferenced formal parameter This change fix this warning.
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4". Example with this function included by Python.h: static inline unsigned int PyUnicode_IS_READY(PyObject* Py_UNUSED(op)) { return 1; } Without this change, building a C program with "cl /W4" which just includes Python.h emits the warning: Include\cpython/unicodeobject.h(199): warning C4100: '_unused_op': unreferenced formal parameter This change fix this warning. (cherry picked from commit 6a43cce) Co-authored-by: Victor Stinner <[email protected]>
GH-127907 is a backport of this pull request to the 3.12 branch. |
gh-107249: Implement Py_UNUSED() for MSVC (GH-107250) Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4". Example with this function included by Python.h: static inline unsigned int PyUnicode_IS_READY(PyObject* Py_UNUSED(op)) { return 1; } Without this change, building a C program with "cl /W4" which just includes Python.h emits the warning: Include\cpython/unicodeobject.h(199): warning C4100: '_unused_op': unreferenced formal parameter This change fix this warning. (cherry picked from commit 6a43cce) Co-authored-by: Victor Stinner <[email protected]>
Fix warnings C4100 in Py_UNUSED() when Python is built with "cl /W4".
Example with this function included by Python.h:
Without this change, building a C program with "cl /W4" which just includes Python.h emits the warning:
This change fix this warning.