Skip to content
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

Fix function macros with empty params list #189

Merged
merged 2 commits into from
Dec 17, 2023
Merged

Conversation

mara004
Copy link
Contributor

@mara004 mara004 commented Dec 5, 2023

This fixes a bug in compiling function macros without params that was present in ctypesgen since at least 2008 according to git blame.

Previously, e.g. the following macro from pymacro.h

#  define Py_UNREACHABLE() \
    Py_FatalError("Unreachable C code path reached")

would translate to

Py_UNREACHABLE = Py_FatalError("Unreachable C code path reached")

which (without try/except guards) promptly raises an exception when importing the module, rendering the macro unusable.
i.e. macros without params were not translated to a function, but the action was called once on import time, and the symbol name merely got assigned the return value of that call.

With this patch, it now correctly translates to

def Py_UNREACHABLE():
    return Py_FatalError('Unreachable C code path reached')

This was discovered during development of the pypdfium2-team fork, thanks to a new option to disable macro guards.
I thought I'd also submit it here because it's a very simple, standalone fix.

This fixes a bug in compiling function macros without params that was present in ctypesgen since at least 2010 according to git blame.

Prior to this fix, e.g. Py_UNREACHABLE from pymacro.h would translate to
Py_UNREACHABLE = Py_FatalError("Unreachable C code path reached")
which would promptly raise an exception on import time
@nilason
Copy link
Collaborator

nilason commented Dec 17, 2023

There is a flake8 failure, please correct it.

@mara004
Copy link
Contributor Author

mara004 commented Dec 17, 2023

There is a flake8 failure, please correct it.

Due to line length I think. I don't usually hard wrap lines and rely on dynamic viewer wrapping instead.
Feel free to update the PR using GH CLI.

I'm mostly working with my own, revised codebase now, and am not interested in flake compliance. Feel free to pick what you like, while I try to submit simple patches or give pointers here as well. But I don't intend to spend time on the usual PR process any longer.

@nilason nilason merged commit 3b56564 into ctypesgen:master Dec 17, 2023
8 checks passed
@mara004
Copy link
Contributor Author

mara004 commented Dec 17, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants