Avoid hardcoded assumed pointer types

ivan-aksamentov commented 2 weeks ago

Depends on https://github.com/blas-lapack-rs/lapack-sys/pull/15

This replaces hardcoded platform-specific char* types with the opaque types from core::ffi. This is necessary when cross-compiling.

I believe when interacting with C and Fortran, both lax and lapack-sys should not assume integer and pointer types. Current setup is biased towards x86_64 platform, for the most part due to a bug in lapack-sys (see https://github.com/blas-lapack-rs/lapack-sys/pull/15), and it could cause broken builds and undefined behavior on other platforms.

For example, cross compilation build could fail with:

error[E0308]: mismatched types
     --> /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/lax-0.16.0/src/cholesky.rs:30:26
      |
30    |                     $trf(uplo.as_ptr(), &n, AsPtr::as_mut_ptr(a), &n, &mut info);
      |                     ---- ^^^^^^^^^^^^^ expected `*const u8`, found `*const i8`
      |                     |
      |                     arguments to this function are incorrect
...
41    | impl_cholesky_!(c64, lapack_sys::zpotrf_);
      | ----------------------------------------- in this macro invocation
      |
      = note: expected raw pointer `*const u8`
                 found raw pointer `*const i8`
note: function defined here
     --> /workdir/.build/docker-aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/release/build/lapack-sys-5fad11066d38a1a6/out/bindings.rs:12886:12

Full build log for aarch64-unknown-linux-gnu: build.log.txt

In this PR I change i8 to core::ffi::c_char - this, for example, resolves to i8 on x86 and to u8 on ARM, but is also universal for any platform rust supports, removing bias towards x86.

ivan-aksamentov commented 2 weeks ago

Closing as duplicate to https://github.com/rust-ndarray/ndarray-linalg/pull/354

ivan-aksamentov commented 2 weeks ago

Reopening, because my version is better - it uses core::ffi::c_char not std.

Either way the root cause is https://github.com/blas-lapack-rs/lapack-sys/pull/15 and it needs to be merged for the thing to not be UB.

ivan-aksamentov commented 2 weeks ago

https://github.com/rust-ndarray/ndarray-linalg/pull/354 now also uses core::. Closing

rust-ndarray / ndarray-linalg

Avoid hardcoded assumed pointer types #384