Rust arrays on C FFI are super confusing

[gnzlbg]

Arrays are passed to C as a raw pointers, which means that foo does not move it, so this example is confusing at best.

#[repr(C)]
struct I32(i32);

extern "C" {
    fn foo(x: [I32; 2]);
}

fn main() {
    let x = [I32(0), I32(1)];
    unsafe { foo(x) };
    println!("x = {}", x[0].0);
}

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error[E0382]: borrow of moved value: `x`
  --> src/main.rs:11:24
   |
10 |     unsafe { foo(x) };
   |                  - value moved here
11 |     println!("x = {}", x[0].0);
   |                        ^^^^^^ value borrowed here after move
   |
   = note: move occurs because `x` has type `[I32; 2]`, which does not implement the `Copy` trait

error: aborting due to previous error

If one does not try to use the moved value, this will silently compile, but x will be deallocated as soon as the function returns, yet the C code could still try to read (or even write - the code above doesn't make it clear what C can actually do with the pointer...) to it.

It would be better if we would require code to be more explicit about this, e.g., by writing:

extern "C" {
    fn foo(x: *const [I32; 2]);
    // or:
    fn foo(x: *mut [I32; 2]);
}

instead. This makes it clear that foo doesn't own the array, how many elements are expected behind the pointer, and whether the foreign function only reads or also might write to it.

We could avoid breaking changes due to updating C FFI code by allowing people to still call foo(x) but treating it a as a unique or shared borrow depending on the mutability of the FFI declaration, and then applying a coercion to the raw pointer, while simultaneously emitting a warning to users that they should be more explicit and write foo(&x as *const _) instead.

[gnzlbg]

FWIW, my preferred solution to this problem would be to just warn when arrays are not used behind a pointer on foreign function declarations, and maybe ban that in a future edition.

I would prefer if we would just do that, and call it a day.

However, I imagine that a lot of code will want to update to avoid this warning, or be compatible with a future edition. This will require API breaking changes all over the place. We could either say, "such is life", or we could try to allow updating the signatures of the FFI declarations without breaking their callers.

Iff that were something worth doing, then the idea I suggested of automatically borrowing the array according to the mutability of the C FFI foreign function declaration fixes the error message of my example, where it is incorrectly diagnosed that the array was moved. In a sense, this is what we are already doing when we transform moves of arrays into pointers when calling foreign functions, so it isn't really a type system change, but rather a bug fix to the implementation of the type system that we already have.

[ollie27]

Is this not just a duplicate of #36464?

[eddyb]

It's an accident that the ABI matches up, since [T; N] in Rust is equivalent to std::array<T, N> in C++, i.e. struct { T[N]; } in C (although we might not guarantee any of that).

And then there's stuff like this, which I don't think we should have landed - the situation in question is impossible in C (since you always would have a wrapper struct):

rust/src/librustc_target/abi/call/mips64.rs

Lines 91 to 95 in 0b7e28a

	abi::FieldPlacement::Array { .. } => {
	// Arrays are passed indirectly
	arg.make_indirect();
	return;
	}

[elichai]

I think raw arrays should be marked as improper_ctypes. arrays should be passed by ref/pointer.

EDIT: Related: #24578.

Will try to make a PR.