Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-65210: Add const qualifiers in PyArg_VaParseTupleAndKeywords()#105958
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
gh-65210: Add const qualifiers in PyArg_VaParseTupleAndKeywords()#105958
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Change the declaration of the keywords parameter in functionsPyArg_ParseTupleAndKeywords() and PyArg_VaParseTupleAndKeywords() from `char **`to `char * const *` in C and `const char * const *` in C++.It makes these functions compatible with argument of type `const char * const *`,`const char **` or `char * const *` in C++ and `char * const *` in Cwithout explicit type cast.
Doc/c-api/arg.rst Outdated
| The *keywords* parameter declaration is :c:expr:`char * const *` in C and | ||
| :c:expr:`const char * const *` in C++. | ||
| This can be overridden by defining the macro :c:macro:`PY_CXX_CONST` |
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.
Please document this new macro somewhere with.. c:macro:: PY_CXX_CONST.
That will fix this warning and the one in3.13.rst and the CI.
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 do not know how to document it better than I did. I will appreciate your suggestions. For now I just removed the role.
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.
@CAM-Gerlach Any suggestions?
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.
@serhiy-storchaka To properly document a public entity (macro, function, class, method, etc), you can use the the appropriate directive—in this case,.. c:macro:: as@hugovk mentioned. This could be something like (placed, perhaps, at the top or bottom of theAPI Functions subsection):
..macro::PY_CXX_CONST The value to be inserted, if any, before:c:expr:`char * const *` in the *keywords* parameter declaration of:c:func:`PyArg_ParseTupleAndKeywords` and:c:func:`PyArg_VaParseTupleAndKeywords`. Default empty for C and ``const`` for C++ (:c:expr:`const char * const *`). To override, define it to the desired value before including:file:`Python.h`. ..versionadded::3.13
You can then simplify thisnote accordingly, per my other suggestion (in a separate comment).
Doc/whatsnew/3.13.rst Outdated
| @@ -433,7 +433,7 @@ New Features | |||
| :c:expr:`const char * const *`, :c:expr:`const char **` or | |||
| :c:expr:`char * const *` in C++ and :c:expr:`char * const *` in C | |||
| without explicit type cast. | |||
| This can be overridden by defining the macro:c:macro:`PY_CXX_CONST` | |||
| This can be overridden by defining the macro``PY_CXX_CONST`` | |||
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.
Can we use this?
| This can be overridden by defining the macro``PY_CXX_CONST`` | |
| This can be overridden by defining the macro:c:macro:`!PY_CXX_CONST` |
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.
Yes, both:c:macro:`!PY_CXX_CONST` and``PY_CXX_CONST`` would work.
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.
To note, while they both silence the warning, they have different semantics in the source and rendering in the output.
However, with the changes suggested above, it is a moot point as you can link this and also simplify this What's New entry by referring readers to the macro description, which explains this (instead of duplicating the explanation here).
Doc/c-api/arg.rst Outdated
| The *keywords* parameter declaration is :c:expr:`char * const *` in C and | ||
| :c:expr:`const char * const *` in C++. | ||
| This can be overridden by defining the macro :c:macro:`PY_CXX_CONST` |
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.
@serhiy-storchaka To properly document a public entity (macro, function, class, method, etc), you can use the the appropriate directive—in this case,.. c:macro:: as@hugovk mentioned. This could be something like (placed, perhaps, at the top or bottom of theAPI Functions subsection):
..macro::PY_CXX_CONST The value to be inserted, if any, before:c:expr:`char * const *` in the *keywords* parameter declaration of:c:func:`PyArg_ParseTupleAndKeywords` and:c:func:`PyArg_VaParseTupleAndKeywords`. Default empty for C and ``const`` for C++ (:c:expr:`const char * const *`). To override, define it to the desired value before including:file:`Python.h`. ..versionadded::3.13
You can then simplify thisnote accordingly, per my other suggestion (in a separate comment).
Doc/c-api/arg.rst Outdated
| This can be overridden by defining the macro :c:macro:`PY_CXX_CONST` | ||
| before including :file:`Python.h` as ``const`` for the latter and as | ||
| empty value for the former. |
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.
| This can be overridden by defining the macro:c:macro:`PY_CXX_CONST` | |
| before including:file:`Python.h` as ``const`` for the latter and as | |
| empty value for the former. | |
| This can be overridden with the:c:macro:`PY_CXX_CONST` macro. |
Refer to the newly-added macro's documentation instead of needing to repeat these details here.
Doc/whatsnew/3.13.rst Outdated
| @@ -433,7 +433,7 @@ New Features | |||
| :c:expr:`const char * const *`, :c:expr:`const char **` or | |||
| :c:expr:`char * const *` in C++ and :c:expr:`char * const *` in C | |||
| without explicit type cast. | |||
| This can be overridden by defining the macro:c:macro:`PY_CXX_CONST` | |||
| This can be overridden by defining the macro``PY_CXX_CONST`` | |||
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.
To note, while they both silence the warning, they have different semantics in the source and rendering in the output.
However, with the changes suggested above, it is a moot point as you can link this and also simplify this What's New entry by referring readers to the macro description, which explains this (instead of duplicating the explanation here).
Doc/whatsnew/3.13.rst Outdated
| This can be overridden by defining the macro ``PY_CXX_CONST`` | ||
| before including :file:`Python.h` as ``const`` for the latter and as | ||
| empty value for the former. |
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.
| This can be overridden by defining the macro ``PY_CXX_CONST`` | |
| before including:file:`Python.h` as ``const`` for the latter and as | |
| empty value for the former. | |
| This can be overridden with the:c:macro:`PY_CXX_CONST` macro. |
Now that this is documented, simply refer interested readers here directly instead of duplicating the whole explanation.
Doc/whatsnew/3.13.rst Outdated
| * The *keywords* parameter of :c:func:`PyArg_ParseTupleAndKeywords` and | ||
| :c:func:`PyArg_VaParseTupleAndKeywords` has now type :c:expr:`char * const *` | ||
| in C and :c:expr:`const char * const *` in C++, instead of :c:expr:`char **`. | ||
| It makes these functions compatible with argument of type |
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.
| It makes these functions compatible withargument of type | |
| It makes these functions compatible witharguments of type |
Fix grammar error
Doc/whatsnew/3.13.rst Outdated
| It makes these functions compatible with argument of type | ||
| :c:expr:`const char * const *`, :c:expr:`const char **` or | ||
| :c:expr:`char * const *` in C++ and :c:expr:`char * const *` in C | ||
| without explicit type cast. |
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.
| without explicit type cast. | |
| withoutanexplicit type cast. |
Fix grammar error
Standard reminder: You can directly apply all the suggestions you want in one go by going to |
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
4ff9e8f to0e4dc59Compare@CAM-Gerlach Thank you, I applied all your suggestions. |
…() (pythonGH-105958)Change the declaration of the keywords parameter in functionsPyArg_ParseTupleAndKeywords() and PyArg_VaParseTupleAndKeywords() from `char **`to `char * const *` in C and `const char * const *` in C++.It makes these functions compatible with argument of type `const char * const *`,`const char **` or `char * const *` in C++ and `char * const *` in Cwithout explicit type cast.Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
…() (pythonGH-105958)Change the declaration of the keywords parameter in functionsPyArg_ParseTupleAndKeywords() and PyArg_VaParseTupleAndKeywords() from `char **`to `char * const *` in C and `const char * const *` in C++.It makes these functions compatible with argument of type `const char * const *`,`const char **` or `char * const *` in C++ and `char * const *` in Cwithout explicit type cast.Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Uh oh!
There was an error while loading.Please reload this page.
Change the declaration of the keywords parameter in functions PyArg_ParseTupleAndKeywords() and PyArg_VaParseTupleAndKeywords() from
char **tochar * const *in C andconst char * const *in C++.It makes these functions compatible with argument of type
const char * const *,const char **orchar * const *in C++ andchar * const *in C without explicit type cast.📚 Documentation preview 📚:https://cpython-previews--105958.org.readthedocs.build/