Skip to content

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

Closed
erlend-aasland wants to merge 3 commits intopython:masterfrom
erlend-aasland:convert-macros/PyExceptionClass_Check
Closed

bpo-43502: Convert PyExceptionClass_Check from macro to static inline function#24875
erlend-aasland wants to merge 3 commits intopython:masterfrom
erlend-aasland:convert-macros/PyExceptionClass_Check

Conversation

@erlend-aasland
Copy link
Copy Markdown
Contributor

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

@erlend-aasland
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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