russcip fails to compile on Windows MSVC: u32 != i32

[KarelPeeters]

Problem

When trying to compile russcip on Windows MSVC, I get a couple of errors that are all similar to this:

error[E0308]: mismatched types
  --> [...]\.cargo\registry\src\index.crates.io-6f17d22bba15001f\russcip-0.2.6\src\branchrule.rs:34:43
   |
32 |     fn from(val: BranchingResult) -> Self {
   |                                      ---- expected `u32` because of return type
33 |         match val {
34 |             BranchingResult::DidNotRun => ffi::SCIP_Result_SCIP_DIDNOTRUN,
   |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found `i32`
   |
help: you can convert an `i32` to a `u32` and panic if the converted value doesn't fit
   |
34 |             BranchingResult::DidNotRun => ffi::SCIP_Result_SCIP_DIDNOTRUN.try_into().unwrap(),
   |                                                                          ++++++++++++++++++++

Cause

The underlying reason is that the enum SCIP_Result type is represented by unsigned int on Linux, but by int in Windows MSVC. Apparently this is allowed by the C++ spec:

Declares an unscoped enumeration type whose underlying type is not fixed (in this case, the underlying type is an implementation-defined integral type that can represent all enumerator values; this type is not larger than int unless the value of an enumerator cannot fit in an int or unsigned int.

bindgen then just exposes the underlying type as u32 or i32 to Rust, see https://github.com/rust-lang/rust-bindgen/issues/1966.

In turn, russcip does not handle this type difference correctly, and fails to compile.

Solution

I see a couple of potential solutions:

• Tell bindgen to generate an actual Rust enum instead. This might be problematic for the reasons described in https://github.com/rust-lang/rust-bindgen/issues/758, but I don't know how this applies to SCIP specifically.
• Expose this type difference to users russcip, by changing all types to SCIP_Result instead of u32. This is annoying because it just pushes the problem onto users, who will probably all also end up with either broken Linux or Windows builds.
• Add casts to u32 in all the right places in russcip, at least for windows targets. Then everyone would be using u32 all the time, even though it's not what the underlying SCIP library is using.

I think the last solution is the easiest to implement, and also the best one. I could submit a PR for any solution if that's helpful and a decision has been made.

This might also be the only thing stopping Windows CI from working, in which case it can be turned on again! https://github.com/scipopt/russcip/blob/cc34f3d7faece624be2c1907fc3def8d0a1e86ea/.github/workflows/build_and_test.yml#L84

[mmghannam]

Hi @KarelPeeters! thank you for the very well-structured detailed issue. I agree that the last solution sounds like the best fit, and I would be happy to review your PR :)

[KarelPeeters]

Since looking into this more and actually putting together a fix, I think the second solution is actually the better one.

russcip already isolates users from SCIP_Result with its own separate enum types, eg. BranchingResult. The functions that were failing to compile earlier are exactly those that convert between those. Of course it's fine if they have to know about the underlying type, their entire purpose is hiding it from users!

I've submitted this second solution as https://github.com/scipopt/russcip/pull/115.