Improved Win32 array transformation

[kennykerr]

Improves win32 array parsing and fixes a few minor correctness issues with shared array lengths.

[samlh]

I would love to be able to use these method signature to be able to avoid the possibility of runtime failures, but I'm not sure if this is good idea because this makes it impossible to call the APIs with dynamic length slices - you could only call the APIs with compile-time fixed length arrays.

If the compile-time length restriction is a problem, I think the API wrapper may be forced to add run-time checks. I'm not sure how the runtime checks would emit failures - perhaps panic, as failing them would be a programmer error?

Perhaps 2 different signatures could be provided to allow for trading off compile-time checks for runtime flexibility?

[kennykerr]

@samlh I wondered about that, but you can still use a slice if you so desire, you just have to use a run time check to confirm that it has the right length. For example, the GetTempFileNameA function requires an array with a length of exactly 260 characters. Here's how you can use it with anything that can produce a slice, including dynamic containers like Vec.

fn wrapper(buffer: &mut [u8]) {
    unsafe {
        GetTempFileNameA(".", "test", 0x7b, buffer.try_into().expect("slice length not 260"));
    }
}

[samlh]

Right, that works if you know at compile time what length you need to cast to.

However, for a function like ID3D12CommandQueue::UpdateTileMappings, you might want to call it with 2 regions on one frame, and 3 regions on another frame, so the caller doesn't have a single compile-time length to cast to.

To put it another way const PARAM1: usize isn't necessarily a constant in practice :).

[kennykerr]

Got it - that's a good example.

[samlh]

One approach to this problem is to make a new type to represent the invariant, with multiple constructors (untested):

struct SameLenSlices2<T1,T2>(t1: &[T1], t2: &[T2]);

impl<T,U> TryInto<SameLenSlices2<T,U> for (&[T],&[U]) { ... }

impl<const LEN: usize,T,U> Into<SameLenSlices2<T,U> for (&[T;LEN],&[U;LEN]) { ... }

// Repeat for SameLenSlices3, etc

fn UpdateTileMappings(regions: SameLenSlices2<D3D12_TILED_RESOURCE_COORDINATE, D3D12_TILE_REGION_SIZE>, ...)

Either way, callers would need to do (coords,sizes).into() or (coords,sizes).try_into().unwrap() depending on whether the values were arrays or slices.

I'm throwing this out as an idea, but I do think it is likely overkill...

[chyyran]

For what its worth, just tested this out and it is working great for the specific use case in #1567 but what @samlh brought up seems like a pretty big dealbreaker.

I wonder if it would be appropriate to just not length check the slices and allowing the length parameter to be passed as part of the call signature while still going through with the Option<&(mut) [T]> transform for the rest of the params, unsafely ignoring the actual lengths of the input params altogether.

Not familiar enough with bindgen to know if such a transform will require much more extra code at runtime. If the caller is not careful there is obvious unsoundness here but as its unsafe its up to the caller to maintain invariants regardless.

As a related aside, perhaps that would allow similar transforms for functions that take [in, out] length parameters as well like CSGetShader or RSGetScissorRects which are currently typed

pub unsafe fn RSGetScissorRects(&self, pnumrects: *mut u32, prects: *mut RECT);

and

pub unsafe fn CSGetShader(&self, ppcomputeshader: *mut Option<ID3D11ComputeShader>, ppclassinstances: *mut Option<ID3D11ClassInstance>, pnumclassinstances: *mut u32)

respectively, dereferencing the array length parameter as necessary as the disambiguator of the input slice lengths. It would still be dangerous (arguably no more than it is dangerous to use in C++), but the ergonomics would be much better than the as_mut_ptr needed to use these APIs right now.

[kennykerr]

Thanks for the feedback! I'm just taking a few days off, but I'll likely scale this back to avoid such ambiguities for now. Separately I'm hoping to reduce the number of methods that must be unsafe so I'd like to avoid introducing code that makes unverifiable assumptions.

[kennykerr]

Ended up scaling back the changes to only include the parsing improvements and very limited code gen changes to improve correctness. It avoids all parameters that share a length and have any optional parameters.