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

Improve support for macros #86

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ndessart
Copy link
Contributor

This change improves support for float literal and int literals in macros and most importantly adds support for function-like macros !

This is probably a breaking change.

Things seems to work pretty well (at least according to my expectations) with clang 11 but with clang 6 test_macro.py seems to enter an infinite recursion.

@ndessart ndessart changed the title [DEV] improve support for macros Improve support for macros Mar 15, 2021
trolldbois
trolldbois previously approved these changes Mar 15, 2021
@trolldbois trolldbois self-assigned this Mar 15, 2021
@trolldbois
Copy link
Owner

I'm getting some infinite recursion on test_callback too

Copy link
Owner

@trolldbois trolldbois left a comment

Choose a reason for hiding this comment

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

Awesome pull proposal.
But I don't think I like what it produces. Or I don't understand why/if it's in line with this project.

  1. This pull add a lot of Macro-related supporting functions into the generated code, to dynamically evaluate the content of Macros. So it adds Macro parser code to the generated code to evaluate the Macro when the generated code is ran.
  2. I'm of the opinion that macro functions should be translated to python functions directly when possible or just commented out (in the generated code) for the unsupported cases

I don't understand the use cases for the feature.
Do you have a few example of when this would be useful ?
#84 has a request for this I guess, but I don't understand the usecase.

Are you proposing to handle C functions next ? Emulation of C code in python ?

@trolldbois trolldbois dismissed their stale review March 15, 2021 22:36

removal my previous review

@ndessart
Copy link
Contributor Author

Dynamic evaluation of function-like macros has a lot of implications and I don't have enough hindsight to determine :
a) what could reasonably be supported by ctypeslib
b) what could be the actual use cases beyond bitwise operations and function parameters binding/reordering.

Initially, I just wanted to support macros that expand to constant literals (int and float). Currently something as simple as

#include <stdint.h>
#define FOO UINT64_MAX

fails because the macro UINT64_MAX defined by glibc stdint.h involves intermediate function-like macros.

Are you proposing to handle C functions next ? Emulation of C code in python ?

No, my objective was to follow the principle of least surprise so trying my best to support macros that look "simple enough".
I don't plan to open Pandora's box nor do I plan to implement a C interpreter in Python

My only real world use case (so far) for dynamic evaluation of function-like macros is to debug ctypeslib macro expansion (while debugging ctypeslib itself or the Python binding it generates). Other than that I don't think my bindings would need anything more than a working build-time constant folding macro expansion.

While implementing function-like macro support, I've switched multiple times back and forth between partial macro expansion and full constant folding until I realized that partial macro expansion (/dynamic macros) is easier to debug.

I should probably add a command line option to activate partial macro expansion, what do you think ?

That being said, I have to admit that I was also a bit curious to see what could reasonably be achieved with dynamic function-like macros. So far my answer would be: not very much beyond simple bit mask / bitwise operations

            #define FOO(foo) (foo & 0x0FFFF)

and function parameter binding/reordering :

            #define FOO(...) ("foo", __VA_ARGS__, "bar")
            #define BAR(a, b, c) FOO(c, b, a)

(See test/test_macro_advanced.py)

@trolldbois
Copy link
Owner

Note: My head is hurting at thinking of ramifications.

So there is what i'm comfortable seeing/my guidelines for ctypeslib for now. Still thinking of what that means.

  1. Default behavior generated code should be simple, and easily readable. Like a simple C to py translation of that file. One C input, one py module.

  2. Generated code should contain python variables, variables values and records type definition that are compatible with C

  3. (thanks to your previous pull request) Generated code should contain C function bindings that are easily usable

  4. Generated code should try to support easily translated values for object macro, just because they are easy enough and practical.

  5. Generated code should not try to translate/emulate all code, or try to hard to be a preprocessor

  6. Generated code should not contain "too much" supporting code

  7. Generated code should only contains definition required to the C input (and not all the include's definitions) (by default behavior)

Point 1 potentially opens the door to a more complex framework, a bit like CFFI, where ctypeslib has to be present to execute the generated code ?
the stdint.h headers could get specialized treatment if one absolutely wants to use the MACRO from that headers in some python code
Something like

# normal stuff ...
generator = codegen.Generator(input_io)
generator.generate(parser, items)

# but we absolutely want a macro value
from ctypeslib.utils import macro_advanced_support
macro_namespace = macro_advanced_support('stdint.h')
# or macro_namespace = macro_advanced_support('my_file.c')
print(macro_namespace.UINT64_MAX)  # 18446744073709551615

So yeah, in short, I think I'm coming to like your code here, but let's take it in another direction.

  1. Make it super optional for default behavior with a CLI arg
  2. Make it pass all the tests? I will activate the pull request github actions

@ndessart ndessart force-pushed the dev-macro-branch branch 2 times, most recently from b32915c to b562533 Compare March 24, 2021 19:04
@ndessart
Copy link
Contributor Author

ndessart commented Mar 24, 2021

  1. Make it super optional for default behavior with a CLI arg
  2. Make it pass all the tests? I will activate the pull request github actions

I've updated this pull request, I've reworked it and things should be better now...

Bonus:

  • I think I've fixed some unrelated bugs and performance issues

Change-Id: Iea2826e9ae3e99222c107ee65a5198e67eec6306
Change-Id: I46050074c481a0e5c442b18d0d39a7d936080074
Change-Id: I5027bc6a22b916c056cdc04be4a8731fc59b132c
Change-Id: I6a83b3c2acc09f3a5de066291fe0b3783e153b7a
Change-Id: I0b1144be826d574885e7d4c9a917878f7646bfaf
Change-Id: I228b32952d262d7b59a7ab59a31c58a092a34a0d
Change-Id: Ie57ea25ae6f62d0349f8e01fe1e5df156f8524d3
@ndessart
Copy link
Contributor Author

ndessart commented Mar 29, 2021

I've just rebased my pull request and fixed a libclang version parsing issue.

  1. Make it super optional for default behavior with a CLI arg

I've added a "--advanced-macro" cli flag that activates the generation of function-like macro. However the default behavior of the -m flag has changed with this PR because now ctypeslib will try its best to perform a constant folding of every function-like macro appearing in the definition of a to-be-generated (non-funcion-like) macro. In this case, the generated output doesn't require codegen/preprocess.py while with the advanced macro activated, this file is inlined in the generated output.

  1. Make it pass all the tests? I will activate the pull request github actions

I know the tests results look bad right now but locally with the latest changes, I pass every test with clang 11 / python 3.9.1 in my Linux virtual environment. I don't know why the github actions get cancelled and/or fail because test-callbacks.so is not found... I'll try to investigate.

I'm open to any advice or pointers you may have on this PR or on how to fix the github actions.

I'd also be happy to answer any question you may have on this PR but first I think the "[DEV] improve performances by using some caching" commit deserves some explanations.

When I was debugging my code, I've encountered performance issues and/or what appeared to be "infinite recursions" so I've profiled ctypeslib and tried to cache/memoize the functions that where called the most and/or appeared to be libclang bottleneck after a few trial and error I've ended with the following cached functions (see: ctypeslib/codege/cache.py:_cache_functions) :

    "ctypeslib.codegen.cindex.Cursor.get_tokens",
    "ctypeslib.codegen.cindex.SourceLocation.__contains__",
    "ctypeslib.codegen.cindex.Token.cursor",

Other functions of ctypeslib or libclang don't seemed to be worth caching (the codegen.cache.cached* decorators don't enable caching unless the function name is in the ctypeslib/codege/cache.py:_cache_functions list).

While I was implementing the cache feature, I've replaced every "print" call in codegenerator so I've decided to kill two birds with one stone and to replace the old-style % string formatting by (more performant) f-strings.

Finally, in this cache/performance commit, I've also reworked the ClangParser.all and ClangParser.all_set attributes because it caused a caching bug.

Change-Id: I08fc301da66b3551d17c8ca708fde48fbc11890c
Change-Id: Icd099241a380318c05bb9b94b3818816cdc9c50b
@trolldbois
Copy link
Owner

thinking about it ...

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.

2 participants