Allow wrapper function generation for functional macros

[Abestanis]

This allows to register functional macros for which a wrapper function will be generated. Currently it is not possible to call functional macros at all. This makes it possible.

Macros don't have a defined argument or return types, which is why they have to be specified explicitly. Some macros can be called with different argument types. This patch only supports creating a wrapper function for one specific set of argument and return types. Wrappers are only generated for configured macros, all others are ignored.

Example

With this change we can expose the following C macros:

#define MACRO_WITH_NO_ARGUMENTS printf("Test")
#define MACRO_WITH_NO_ARGUMENTS_AND_RETURN_VALUE() 0
#define MACRO_WITH_ONE_ARGUMENT_AND_RETURN_VALUE(value) ((float) (value))
#define MACRO_WITH_TWO_ARGUMENTS_AND_RETURN_VALUE(condition, value) ((condition) ? ((float) (value)) : 0)

To generate wrappers for these macros, we have to eiter configure them via command line arguments:

--macro-function "MACRO_WITH_NO_ARGUMENTS"
--macro-function "u8 MACRO_WITH_NO_ARGUMENTS_AND_RETURN_VALUE"
--macro-function "f32 MACRO_WITH_ONE_ARGUMENT_AND_RETURN_VALUE(u32)"
--macro-function "f32 MACRO_WITH_TWO_ARGUMENTS_AND_RETURN_VALUE(bool, u32)"

… or via the Builder:

Builder::default()
     .macro_function("MACRO_WITH_NO_ARGUMENTS", FunctionType::new::<(), ()>())
     .macro_function("MACRO_WITH_NO_ARGUMENTS_AND_RETURN_VALUE", FunctionType::new::<u8, ()>())
     .macro_function("MACRO_WITH_ONE_ARGUMENT_AND_RETURN_VALUE", FunctionType::new::<f32, u32>())
     .macro_function("MACRO_WITH_TWO_ARGUMENTS_AND_RETURN_VALUE", FunctionType::new::<f32, (bool, u32)>())

On the Rust side we can then call the macros as normal functions:

unsafe {
    ffi::MACRO_WITH_NO_ARGUMENTS();
    let value = ffi::MACRO_WITH_NO_ARGUMENTS_AND_RETURN_VALUE();
    let value = ffi::MACRO_WITH_ONE_ARGUMENT_AND_RETURN_VALUE(42);
    let value = ffi::MACRO_WITH_TWO_ARGUMENTS_AND_RETURN_VALUE(true, 42);
}

[Abestanis]

r? @emilio

[bors-servo]

:umbrella: The latest upstream changes (presumably f44a66578dfcc74da290b9707e0cbb290b5ceeb4) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

:umbrella: The latest upstream changes (presumably 251dec94c8c7c6413bdaec0c40ff2acecc42e660) made this pull request unmergeable. Please resolve the merge conflicts.

[pvdrz]

I like this feature but I think it needs more bikeshedding.

One issue I see is that having to write the name of the macro itself and the type makes it dangerously close to just creating a wrapper by hand.

If I had a macro in input.h:

#define INC(value) (value + 1)

I could very well create a wrapper.h with these contents:

#include "input.h"

int inc(int val) {
    return INC(val)
}

And pass it to bindgen. You could argue that just having to do --macro-function="int INC(int)" is shorter but it doesn't feel that much shorter to be honest and the amount of complexity adding to bindgen to do it doesn't feel like a good trade-off to me.

[Abestanis]

Writing the wrapper manually in a header and passing it to bindgen does create a binding, but the function still needs to be included in the final compilation unit, so we also need a source file that includes the header and add it to the build. Otherwise the function referenced by bindgen is not actually present in the library.

I'm also not a fan of having to specify every macro function type, but I'm not aware of any solution on how to avoid doing that and at least this way I can avoid writing C code, which is why this feature is worth it in my opinion.

[bors-servo]

:umbrella: The latest upstream changes (presumably 1d2b579d1c3180e93089ffb1379ee11fac0cccc0) made this pull request unmergeable. Please resolve the merge conflicts.

[pvdrz]

Writing the wrapper manually in a header and passing it to bindgen does create a binding, but the function still needs to be included in the final compilation unit, so we also need a source file that includes the header and add it to the build. Otherwise the function referenced by bindgen is not actually present in the library.

That's a good point, so basically by piggybacking on the --wrap-static-fns video we get the source C file so the user only has to compile it instead of writing it by hand. That's pretty neat!

I'm also not a fan of having to specify every macro function type, but I'm not aware of any solution on how to avoid doing that and at least this way I can avoid writing C code, which is why this feature is worth it in my opinion.

Yeah I don't have a better solution for this either. I'll give you a proper review in a moment

[reitermarkus]

@Abestanis, does https://github.com/rust-lang/rust-bindgen/pull/2369 not solve your use case?

[Abestanis]

@reitermarkus #2369 is very interesting and incredibly cool. Thank you for putting so much effort into it. I can indeed use it for my specific usecase of the one specific macro I am trying to make accessible, but I think there is an issue with the approach of trying to replicate the C macro in Rust:

C and macros have a low of quirks and Rust is different to C and macros in many ways. There will always be some macro which can't yet be represented by #2369 and needs some new feature. Specifically when it comes to types and operations between types, C is a lot more lenient (because of implicit type conversions) while Rust basically disallows most operations between types. This means that many macros loose their ability to be called with different types.

I think the following (terrible, but) valid macro is going to be difficult to represent with #2369, even though I'm sure there are workarounds for this:

#define WEIRD_IS_ONE(x) (x == (uint16_t) 1 || x == (char) '1')

The wrapping approach supports all macro types out of the box, as long as the arguments and return types can be sent across ffi.

In my specific use case I am trying to make the following macro available:

typedef long BaseType_t;
#define pdFALSE ( ( BaseType_t ) 0 )
#define portEND_SWITCHING_ISR( xSwitchRequired ) do { if( xSwitchRequired != pdFALSE ) portYIELD(); } while( 0 )
#define portYIELD_FROM_ISR( x ) portEND_SWITCHING_ISR( x )

With the wrapping approach, I can define a wrapper like this

bindgen::Builder::default()
    .macro_function("portYIELD_FROM_ISR", FunctionType::new::<(), bool>())
    .generate();

and call it like this:

pub fn yield_from_isr(should_do_isr_context_switch: bool) {
    unsafe { ffi::portYIELD_FROM_ISR(should_do_isr_context_switch) }
}

Even though xSwitchRequired and pdFALSE have different types, C allows the comparison to work as expected.

With #2369, the following binding macro is generated:

pub use __cmacro__portEND_SWITCHING_ISR as portEND_SWITCHING_ISR;
#[doc(hidden)]
#[macro_export]
macro_rules! __cmacro__portYIELD_FROM_ISR {
    ($ x : expr) => {
        if $x != 0u8 as BaseType_t {
            vPortYield();
        }
    };
}

I can not call this macro with a bool argument type any more, because a <bool> != <cty::c_long> is not allowed in Rust. Instead I have to work around this issue by explicitly casting the only allowed argument type:

pub fn yield_from_isr(should_do_isr_context_switch: bool) {
    unsafe { ffi::portYIELD_FROM_ISR!(BaseType_t::from(should_do_isr_context_switch)) }
}

So I still think the wrapping approach has some value and could exist together with #2369, but we have to make sure that they don't interfere with each other.

[Abestanis]

@pvdrz Sorry for the long period of silence, I have addressed your feedback.

we might need a different name for this feature as ParseCallbacks::func_macro sounds almost like the same, maybe a more explicit --wrap-func-macro is better?

I'm open for better names, although the ParseCallbacks::func_macro is referring to the exact same concept of a functional macro, so I don't think the two are in conflict. Both allow handling functional macros in some way.

The other thing I'd ask you is avoid editing the style of the CHANGELOG.md so you don't get more merge conflicts.

Yeah, sorry about that, that was not a good idea. I removed all the formatting changes there except the trailing whitespaces, because my editor really wants to remove them when saving. I hope it's ok to leave these changes in there, but If needed I can try to remove them from the commit.

[reitermarkus]

In my specific use case I am trying to make the following macro available:

typedef long BaseType_t;
#define pdFALSE ( ( BaseType_t ) 0 )
#define portEND_SWITCHING_ISR( xSwitchRequired ) do { if( xSwitchRequired != pdFALSE ) portYIELD(); } while( 0 )
#define portYIELD_FROM_ISR( x ) portEND_SWITCHING_ISR( x )

FreeRTOS is exactly what started my PR, actually. You may want to take a look at my fork of freertos-rust here, which is using the cmacro branch of my bindgen fork to generate (almost) all FreeRTOS bindings: https://github.com/reitermarkus/freertos-rust

But yes, the portEND_SWITCHING_ISR macro cannot be generated easily since it in turn calls the portYIELD macro, which in turn wants to write to a register.

The portYIELD macro is actually working fine, I even have a test for it here:

https://github.com/reitermarkus/cmacro-rs/blob/00849e0f5fda2c2de22bff73049faf53ebff54c9/tests/fixtures/assign_pointer.h

[reitermarkus]

@Abestanis, I just found a bug that prevented the portEND_SWITCHING_ISR from being parsed correctly and hence from being generated, it should be possible now with my PR.

[Abestanis]

@reitermarkus I would like to give your MR another try, but it seems like it is no longer compiling.

[bors-servo]

:umbrella: The latest upstream changes (presumably 285eb56cf5a02ee778880b8e1b7bb2e8118a5a88) made this pull request unmergeable. Please resolve the merge conflicts.

[reitermarkus]

@Abestanis, I have now rebased https://github.com/rust-lang/rust-bindgen/pull/2369.

[Abestanis]

@reitermarkus Thank you! ❤️