Switch to single precompiled header in macro fallback

[jbaublitz]

Fix for issue reported here.

@SeleDreams Could you also confirm that this resolves your problem?

[SeleDreams]

Fix for issue reported here.

@SeleDreams Could you also confirm that this resolves your problem?

I tested, seems like it still doesn't generate the IRQ defines when I have other headers included

[jbaublitz]

@SeleDreams Can you provide me with a minimal reproducer to reproduce your problem with the headers you're using? I'm no longer able to reproduce the problem with this PR in the setup that originally helped me confirm that there was a bug.

[SeleDreams]

I'll try to do a reproduction case as soon as possible. Right now with my great luck I have an X server issue on the linux machine I work on so I can't do it right now

Le lun. 29 avr. 2024 à 20:02, John Baublitz @.***> a écrit :

@SeleDreams https://github.com/SeleDreams Can you provide me with a minimal reproducer to reproduce your problem with the headers you're using? I'm no longer able to reproduce the problem with this PR in the setup that originally helped me confirm that there was a bug.

— Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust-bindgen/pull/2823#issuecomment-2083340511, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD4UF4PB56GDOHDE2TOYJHDY72DNHAVCNFSM6AAAAABG6PCDBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBTGM2DANJRGE . You are receiving this because you were mentioned.Message ID: @.***>

[jbaublitz]

This looks reasonable but I guess we should sort out why it's still not working for @SeleDreams? Or do you want this merged now?

I would like to figure out what's going on with @SeleDreams before we merge this as they reported the problem originally so I'd like to make sure it works in their use case too. If it's a setup problem, I'll help debug and if there are two bugs, I'll fix the second one. :)

A test for this would also be amazing, but if it's too hard to set up lmk.

How would you generally suggest I go about testing multiple headers? Is there infrastructure for that?

[jbaublitz]

I'll try to do a reproduction case as soon as possible. Right now with my great luck I have an X server issue on the linux machine I work on so I can't do it right now Le lun. 29 avr. 2024 à 20:02, John Baublitz @.> a écrit : … @SeleDreams https://github.com/SeleDreams Can you provide me with a minimal reproducer to reproduce your problem with the headers you're using? I'm no longer able to reproduce the problem with this PR in the setup that originally helped me confirm that there was a bug. — Reply to this email directly, view it on GitHub <#2823 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD4UF4PB56GDOHDE2TOYJHDY72DNHAVCNFSM6AAAAABG6PCDBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBTGM2DANJRGE . You are receiving this because you were mentioned.Message ID: @.>

Okay sounds good! Just let me know when you can provide the reproducer, and I'll take a look. Basically I just want to rule out a setup problem because since bindgen doesn't have a lot of error reporting infrastructure yet (I think it's planned), setup problems will look exactly the same as a bug: bindings for the constants are just not generated. If it's a bug, I'll fix it. If not, I'll try to point you to exactly what's going on.

[SeleDreams]

@jbaublitz So, I was able to make a reproduction project, however, for some weird reason, it seems to delete the wrapper.h as well everytime I build the project which in itself could be another bug https://github.com/SeleDreams/reproduction_test

[jbaublitz]

@SeleDreams Thanks so much for the report and taking the time to craft a reproducer! It appears there were two bugs. One was that Clang only supports one precompiled header at a time which I resolved, but the other is that -I flags were not getting passed from .clang_arg() calls to the header precompilation. The reason I didn't bump into this is that I was testing on headers in system folders, but for headers that are in nonstandard places (like in your case) these were not being picked up.

The resolution I've come to is forwarding all -I and -isystem flags. @emilio Let me know if you think other flags should be forwarded too.

[jbaublitz]

I've added a test that tests both multiple headers and also the -I parameters being forwarded.

[jbaublitz]

Is there any good reason not to forward all flags? Think of a macro that desugars to something like sizeof(int), if you don't pass the right flags (like --target and so) you might end up with the wrong constant value, right? Same with sizeof(enum foo) if you don't pass -fshort-enums or what not... Probably countless examples too.

The reason I didn't forward all flags is that generally there's a header provided at the end of the clang args in my tests which causes the precompilation to fail. If you'd prefer, I can just remove the last argument instead of exclusively passing the include directives.

[emilio]

Yes. Excluding what we don't need seems like a better approach imho

[jbaublitz]

@emilio I've taken the approach of filtering out any argument that matches the elements in input_headers or -include based on how bindgen generates c_args for the main translation unit. Let me know if you have any issues with this approach.

@SeleDreams Once the code review is done, do you mind testing your actual setup and confirming the problem is resolved?

[jbaublitz]

@SeleDreams I'm assuming this resolved your issue? If not, please let me know.

[SeleDreams]

that seems to have worked