Skip to content

Conversation

@vstinner
Copy link
Member

@vstinnervstinner commented Feb 8, 2022

When a static inline function is wrapped by a macro which casts its
arguments to the expected type, there is no need that the function
has a different name than the macro. Use the same name for the macro
and the function to avoid confusion.

Rename _PyUnicode_get_wstr_length() to PyUnicode_WSTR_LENGTH().

Don't rename static inline _Py_NewRef() and _Py_XNewRef() functions,
since the C API exports Py_NewRef() and Py_XNewRef() functions as
regular functions. The name cannot be reused in this case.

https://bugs.python.org/issue45490

When a static inline function is wrapped by a macro which casts its arguments to the expected type, there is no need that the function has a different name than the macro. Use the same name for the macro and the function to avoid confusion. Rename _PyUnicode_get_wstr_length() to PyUnicode_WSTR_LENGTH(). Don't rename static inline _Py_NewRef() and _Py_XNewRef() functions, since the C API exports Py_NewRef() and Py_XNewRef() functions as regular functions. The name cannot be reused in this case.
@vstinner
Copy link
MemberAuthor

cc @erlend-aasland@encukou

@encukou
Copy link
Member

Hm, this does look a bit better at first glance. Are there any downsides?

@vstinner
Copy link
MemberAuthor

Are there any downsides?

I'm not aware of any downside.

Copy link
Contributor

@erlend-aaslanderlend-aasland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this change.

@vstinnervstinner merged commit e0bcfd0 into python:mainFeb 11, 2022
@vstinnervstinner deleted the rename_static_inline branch February 11, 2022 16:01
@vstinner
Copy link
MemberAuthor

Merged, thanks for the review @erlend-aasland.

More background on why I chose a different name initially and why I want to change it now: https://mail.python.org/archives/list/python-dev@python.org/message/XOLV4BVJDZH3RJRLV3UKDYKA664IANZO/

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vstinner@encukou@erlend-aasland@the-knights-who-say-ni@bedevere-bot