Skip to content

bpo-43502: Convert PyExceptionClass_Check from macro to static inline function #24875

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

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Mar 15, 2021

@erlend-aasland
Copy link
Contributor Author

cc. @vstinner

Should the NEWS entry include more info? For example "the macro was problematic because it reused its arguments twice".

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

If the function is not used in performance critical hot code, the macro can be converted to a regular function (not a static function).

@erlend-aasland
Copy link
Contributor Author

If the function is not used in performance critical hot code, the macro can be converted to a regular function (not a static function).

My gut feeling is to convert PyExceptionClass_Check to a static inlined function, not a regular function. Let me know if you want it to be a regular C function.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Include/ contains more than 450+ functions defined as macros. I don't want to have to review 450 PRs to change all macros.

Do you have an idea on how many macros have the double evaluation bug?

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Mar 15, 2021

Include/ contains more than 450+ functions defined as macros. I don't want to have to review 450 PRs to change all macros.

Do you have an idea on how many macros have the double evaluation bug?

I doubt all of them are clearly problematic. I'll see if I can create a list of worst cases and put it on the issue.

UPDATE There are 88 macros that reuse arguments in Include (including subdirectories):
macros-that-reuse-args.txt

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Mar 19, 2021

I'm closing this until there's some consensus regarding bpo-43502.

@erlend-aasland erlend-aasland deleted the convert-macros/PyExceptionClass_Check branch March 19, 2021 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants