Add def.h to bindgen + solved deprecation warning#36
Merged
mmghannam merged 6 commits intoscipopt:mainfrom Dec 16, 2025
Merged
Conversation
… callback The new callback is needed because bindgen cannot infer type-casted macros. For example the macro '#define SCIP_INVALID (double)1e99' is too complicated. This is because of the type-cast to double. If we simply remove the (double), then bindgen CAN generate a binding for it. It also defaults to an `f64` in Rust, which is what the C-representation would look like too. The callback should only be used for types of which we know that Rust can infer the datatype. Therefore, I made sure that it only goes off on; 1. macros that the user deliberately targets (added via `target`-method) 2. only looks at casts to clang-keywords. So no weird custom mumbo-jumbo casts
Member
|
This is great! thanks a lot @HunkBenny. Just one thing as a sanity check, please add a test that just accesses something from def.h. Just to test that it works on all setup methods. |
Contributor
Author
Do you think something as simple as this would work? |
mmghannam
reviewed
Dec 12, 2025
Member
Yes that was exactly what I had in mind. thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Mo!
Sorry in advance for also including a deprecation-warning fix in this PR, figured I would tackle that one as well.
The reason why I need this is because of
SCIP_INVALID. (See scipopt/russcip#269)This constant is defined as a C-macro over here; https://scipopt.org/doc/html/def_8h_source.php#l00178. It would be nice if we had access to this macro in Rust as well. By letting
bindgengenerate these constants, we do not need to manually define everything ourselves. Therefore, I added 'def.h' tobindgen.There was one problem though,
bindgenhas a hard time when dealing with type-casts in macros. Therefore it skipped generating bindings forSCIP_INVALID. The solution for this was defining a callback by implementing this trait. This callback looks for a pattern like: '( KEYWORD )', where I believe the KEYWORD to be a clang-keyword. I did not dare to dive further in the clang-rabbithole, but it seemed to work for standard C-types (double) and not for other types likeSCIP_Real. Which is precisely what I think we should want.As an extra safety I made sure that the macros we want to target should be added to the
DeriveCastedConstantstruct. I think thatbindgenmust have a good reason why it ignores type-casted macros, so I think it is best to not let this callback blindly noscope every single type-casted macro that crosses its path!I tested this in my local version of
russcipand was able to accessffi::SCIP_INVALID. 🎉