rust-bindgen
rust-bindgen copied to clipboard
Support complex macros.
This supersedes https://github.com/rust-lang/rust-bindgen/pull/2268 which aimed to support casts in macros.
The cmacro
crate I developed over the last months allows parsing casts in variable-like macros and even more complex macros, such as function-like macros and nested macros.
Macro parsing has been split from Var
into a separate MacroDef
type, since macros need to be parsed last. Variables can be evaluated at parse time, macros on the other hand can only be fully evaluated after all types, variables, functions and other macros have been parsed. This allows getting the correct final value and is needed for parsing nested macros.
:umbrella: The latest upstream changes (presumably 5875949e8e598fa9b0e3a018b3547815e2654fc8) made this pull request unmergeable. Please resolve the merge conflicts.
Can someone please take a look at this PR? It's been over three months without a review.
:wave: Hi! Sorry for not giving you an answer in all this time. Somehow I missed this and wasn't aware of it.
I will properly review this PR as I think it might solve many issues regarding the type that is assigned to these define macros when they get translated as constants. Bindgen has depended for a long time on rust-cexpr
but having someone breathe new life to this part of bindgen is good IMHO.
That being said, we still should figure out the logistics of maintenance due to bindgen using your crate and I don't have the whole picture in my head so I'll defer that part to @emilio
Also random question. Is token concatenation using ##
supported?
Yes, concatenation (ident ## ident
) as well as stringification (#ident
) are supported. I think the only problem is Rust's concat_idents!
does not (yet) support concatenating numbers (i.e. ident ## 1
).
Yes, concatenation (
ident ## ident
) as well as stringification (#ident
) are supported. I think the only problem is Rust'sconcat_idents!
does not (yet) support concatenating numbers (i.e.ident ## 1
).
Hmm how would directives like this be translated then?
#define UL(X) X ## UL
#define ONE UL(1)
I think this might actually expand ONE
to 1 as c_ulong
since there is special treatment for concatenating with a literal suffix like UL
.
Is should be working now, there was a bug that treated 1 ## U ## LL
different than 1 ## LL # U
which prevented some variants from being parsed.
Nevermind. The UL
macro is generated correctly as
macro_rules! __cmacro__UL {
($X:ident) => {
concat_idents!($X, UL)
};
}
ONE
currently generates
pub const ONE: _ = concat_idents!(X , UL);
so some special handling in code generation needs to be done as well in addition to doing it in the parser.
ONE
should now correctly generate
pub const ONE: _ = 1;
Hi @reitermarkus. I haven't had enough time to review the most recent changes but I suspect we need to figure out a way to avoid breaking the --rust-target
flag with this feature as cmacro
is able to emit arbitrary Rust code. I suggest two options depending on your priorities:
- We could put all these changes behind the
"experimental"
feature. In that way we can merge this faster and incrementally fix any regressions caused by the code generation ofcmacro
. - Be sure that
cmacro
will emit valid code for the 1.30 version of the compiler (which is the oldest version we haven't deprecated).
In either case we need to be able to test that different versions of rustc can compile the code emitted by bindgen
which means fixing https://github.com/rust-lang/rust-bindgen/issues/2277.
PD: I really want to merge this PR as it would fix a lot of weird issues we have with preprocessing directives.
I guess we can also add some support for --rust-target
in cmacro
so we can either not generate code for older versions (e.g. for function-like macros) or alter code generation.
I have now put the code generation for function-like macros behind the experimental
feature.
Variable-like macros should already always generate the correct code.
@pvdrz, please have another look here. Probably best to review https://github.com/rust-lang/rust-bindgen/pull/2543 first though.
:umbrella: The latest upstream changes (presumably 2034a0fda98cb8a5180d4f14b8c0d7949c6cb146) made this pull request unmergeable. Please resolve the merge conflicts.
I'm wondering what the current status is here, is this PR blocked waiting on a review?
This PR is blocked (or superseded) by https://github.com/rust-lang/rust-bindgen/pull/2561.
@pvdrz Are you sure that's the right issue? On first glance it doesn't look the same.
@alex, it's blocked by https://github.com/rust-lang/rust-bindgen/pull/2561 at the moment, i.e. part of this PR depends on the sorting being fixed.
Note that this PR also includes the same changes.
Ah, got it, I see how it's blocked. It was the superseded that confused me.
So further testing/reviewing of https://github.com/rust-lang/rust-bindgen/pull/2561 would be appreciated to get this moving again.
@reitermarkus, this is really exciting because it would resolve a downstream bug I ran into with bssl-sys
. The review comments above looked positive, any idea when it might be merged?
@overhacked, thanks for testing.
any idea when it might be merged?
No idea when this will be merged, I already split some of the changes into separate PRs to make them easier to review, but all of them seem to also have stalled.