"Mismatched types" generated for COM methods that take or return array of COM interface pointers as "pointer-to-first plus separate array-length"

[EndilWayfare]

Summary

TL;DR, generated bindings that used to compile now fail to compile with "mismatched types". Mismatch is between the type expected by the interface wrapper and what the map over the vtable invocation result actually returns.

Using the current master branch instead of the latest crates.io release results in the same behavior.

I can manually comment out the offending items from the generated bindings (and make notes of which they are, why it's necessary, etc.), so this is not currently a high-priority blocking item on my end. The priority comes from long-term, forward-looking correctness, as well as potentially revealing a blind-spot in the windows-rs COM support.

This is kind of a tricky issue, because there are so many unknowns on my end. I'm not certain what the "real problem" is, because I'm not sure what the "correct behavior" actually looks like, even in a non-Rust, non-windows-rs context. So, I'm just going to start at the beginning.

SolidWorks COM API wrapper

I am working from the type libraries redistributed with a SolidWorks installation. The API is so old that not only do I not have .winmd to work from, I have to "decompile" the .tlb files into .idl files that the WinmdGenerator SDK even knows what to do with, kind of. This has (seemed to have) worked so far, after a lot of finagling.

I mention that here just to underscore that, because of the multi-step translation process, I actually don't know if this is even a faithful translation of the original SolidWorks interface definition, even without considering whether the original SolidWorks devs wrote that definition correctly.

Problem Bindings

All the problematic methods are

• deprecated (except one)
• only to be used in an in-process context
• ones that I've never actually used

Some of the successor methods are using VARIANT arrays instead (SolidWorks is a heavy user of IDispatch "OLE Automation"), but some of them are still pointer-and-length based (just with one less pointer indirection, for some reason, which seems to work correctly).

Return Result<*mut Option<IFoo>> vs Result<IFoo>

impl IBody {
    // ...
    pub unsafe fn IGetIntersectionEdges<P0>(
        &self,
        toolbodyin: P0,
    ) -> windows_core::Result<*mut Option<IEdge>>
    where
        P0: windows_core::Param<IBody>,
    {
        unsafe {
            let mut result__ = core::mem::zeroed();
            (windows_core::Interface::vtable(self).IGetIntersectionEdges)(
                windows_core::Interface::as_raw(self),
                toolbodyin.param().abi(),
                &mut result__,
            )
            .map(|| result__)
        }
    }
// ...
}
#[repr(C)]
pub struct IBody_Vtbl {
    pub base__: windows::Win32::System::Com::IDispatch_Vtbl,
    // ...
    pub IGetIntersectionEdges: unsafe extern "system" fn(
        *mut core::ffi::c_void,
        *mut core::ffi::c_void,
        *mut *mut *mut core::ffi::c_void,
    ) -> windows_core::HRESULT,
    // ...
}

error[E0308]: mismatched types
     --> crates\solidworks-sys\src\gen\sldworks.rs:32014:13
      |
32008 |       ) -> windows_core::Result<*mut Option<IEdge>>
      |            ---------------------------------------- expected `Result<*mut Option<IEdge>, windows_core::Error>` because of return type
...
32014 | /             (windows_core::Interface::vtable(self).IGetIntersectionEdges)(
32015 | |                 windows_core::Interface::as_raw(self),
32016 | |                 toolbodyin.param().abi(),
32017 | |                 &mut result__,
32018 | |             )
32019 | |             .map(|| result__)
      | |_____________________________^ expected `Result<*mut Option<IEdge>, Error>`, found `Result<*mut *mut c_void, Error>`
      |
      = note: expected enum `Result<*mut Option<IEdge>, _>`
                 found enum `Result<*mut *mut c_void, _>`

IBody.IGetIntersectionEdges is deprecated in favor of IBody2.IGetIntersectionEdges (which is itself deprecated in favor of IBody2.GetIntersectionEdges)

impl IBody2 {
    // ...
    pub unsafe fn GetIntersectionEdges<P0>(
        &self,
        toolbodyin: P0,
    ) -> windows_core::Result<windows::Win32::System::Variant::VARIANT>
    where
        P0: windows_core::Param<windows::Win32::System::Com::IDispatch>,
    {
        unsafe {
            let mut result__ = core::mem::zeroed();
            (windows_core::Interface::vtable(self).GetIntersectionEdges)(
                windows_core::Interface::as_raw(self),
                toolbodyin.param().abi(),
                &mut result__,
            )
            .map(|| core::mem::transmute(result__))
        }
    }
    pub unsafe fn IGetIntersectionEdges<P0>(&self, toolbodyin: P0) -> windows_core::Result<IEdge>
    where
        P0: windows_core::Param<IBody2>,
    {
        unsafe {
            let mut result__ = core::mem::zeroed();
            (windows_core::Interface::vtable(self).IGetIntersectionEdges)(
                windows_core::Interface::as_raw(self),
                toolbodyin.param().abi(),
                &mut result__,
            )
            .and_then(|| windows_core::Type::from_abi(result__))
        }
    }
    // ...
}
#[repr(C)]
pub struct IBody2_Vtbl {
    pub base__: windows::Win32::System::Com::IDispatch_Vtbl,
    // ...
    pub GetIntersectionEdges: unsafe extern "system" fn(
        *mut core::ffi::c_void,
        *mut core::ffi::c_void,
        *mut windows::Win32::System::Variant::VARIANT,
    ) -> windows_core::HRESULT,
    pub IGetIntersectionEdges: unsafe extern "system" fn(
        *mut core::ffi::c_void,
        *mut core::ffi::c_void,
        *mut *mut core::ffi::c_void,
    ) -> windows_core::HRESULT,
    // ...
}

The IBody2 interface compiles successfully in its entirety.

The only difference in the generated wrappers between v0.58 and v0.59 is the unsafe block enclosing the method body.

    pub unsafe fn IGetIntersectionEdges<P0>(
        &self,
        toolbodyin: P0,
    ) -> windows_core::Result<*mut Option<IEdge>>
    where
        P0: windows_core::Param<IBody>,
    {
        let mut result__ = core::mem::zeroed();
        (windows_core::Interface::vtable(self).IGetIntersectionEdges)(
            windows_core::Interface::as_raw(self),
            toolbodyin.param().abi(),
            &mut result__,
        )
        .map(|| result__)
    }

Parameter of *const *const Option<IFoo> vs *const Option<IFoo>

Our lovely friend, IBody, is at it again

impl IBody {
    // ...
    pub unsafe fn IRemoveFacesFromSheet(
        &self,
        numoffaces: i32,
        facestoremove: *const *const Option<IFace>,
    ) -> windows_core::Result<()> {
        unsafe {
            (windows_core::Interface::vtable(self).IRemoveFacesFromSheet)(
                windows_core::Interface::as_raw(self),
                numoffaces,
                facestoremove,
            )
            .ok()
        }
    }
// ...
}
#[repr(C)]
pub struct IBody_Vtbl {
    pub base__: windows::Win32::System::Com::IDispatch_Vtbl,
    // ...
    pub IRemoveFacesFromSheet: unsafe extern "system" fn(
        *mut core::ffi::c_void,
        i32,
        *const *const *mut core::ffi::c_void,
    ) -> windows_core::HRESULT,
    // ...
}

error[E0308]: mismatched types
     --> crates\solidworks-sys\src\gen\sldworks.rs:32059:17
      |
32056 |             (windows_core::Interface::vtable(self).IRemoveFacesFromSheet)(
      |             ------------------------------------------------------------- arguments to this function are incorrect
...
32059 |                 facestoremove,
      |                 ^^^^^^^^^^^^^ expected `*const *const *mut c_void`, found `*const *const Option<IFace>`
      |
      = note: expected raw pointer `*const *const *mut c_void`
                 found raw pointer `*const *const Option<IFace>`

As before, IBody.IRemoveFacesFromSheet is deprecated in favor of IBody2.IRemoveFacesFromSheet. However, IBody2.RemoveFacesFromSheet exists, but is not specified as superseding IBody2.IRemoveFacesFromSheet.

impl IBody2 {
    // ...
    pub unsafe fn RemoveFacesFromSheet(
        &self,
        numoffaces: i32,
        facestoremove: &windows::Win32::System::Variant::VARIANT,
    ) -> windows_core::Result<()> {
        unsafe {
            (windows_core::Interface::vtable(self).RemoveFacesFromSheet)(
                windows_core::Interface::as_raw(self),
                numoffaces,
                core::mem::transmute_copy(facestoremove),
            )
            .ok()
        }
    }
    pub unsafe fn IRemoveFacesFromSheet(
        &self,
        numoffaces: i32,
        facestoremove: *const Option<IFace2>,
    ) -> windows_core::Result<()> {
        unsafe {
            (windows_core::Interface::vtable(self).IRemoveFacesFromSheet)(
                windows_core::Interface::as_raw(self),
                numoffaces,
                core::mem::transmute(facestoremove),
            )
            .ok()
        }
    }
    // ...
}
#[repr(C)]
pub struct IBody2_Vtbl {
    pub base__: windows::Win32::System::Com::IDispatch_Vtbl,
    // ...
    pub RemoveFacesFromSheet: unsafe extern "system" fn(
        *mut core::ffi::c_void,
        i32,
        windows::Win32::System::Variant::VARIANT,
    ) -> windows_core::HRESULT,
    pub IRemoveFacesFromSheet: unsafe extern "system" fn(
        *mut core::ffi::c_void,
        i32,
        *const *mut core::ffi::c_void,
    // ...
}

Again, the only difference in the generated wrappers between v0.58 and v0.59 is the enclosing unsafe block.

The other problematic methods are just variations on this common theme.

Questions

Is this a simple matter of "getting confused by double indirection"?

As far as I can tell, windows-bindgen appears to be going "well, it's a pointer type, and pointer types are copy-able, so do the copy-able thing, which is the identity map". I think. Maybe. It's very complicated, and I'm very new to that codebase.

How is this "supposed" to work?

If I replace .map(|| result__) with .map(|| core::mem::transmute(result__)), it compiles. Given that transmute is black freakin magic "just_ pretend that the bits mean something different", I personally have no idea if this means it's "correct". It makes sense in my head, where *mut c_void turns into *mut IFoo ("cast void to the type you're actually pointing to"), which turns into Option<IFoo> ("nullptr is the niche for Option discriminant"), without any binary/layout changes along the way.

And then, if that's valid, shouldn't it be a simple matter of (if Some) using Interface::as_raw/Interface::into_raw, <pointer>::offset, and Interface::from_raw to grab the following elements of the array? Those interface pointers have already been "given" to me by returning from the method, and it's my responsibility to release them (by wrapping them in a type that will do that on drop), right? Something like,

let len = body.IGetIntersectionEdgeCount(&other)? as isize * 2; // [sic]. Documentation: "The total number of edges in this array is twice the value returned from IBody2::IGetIntersectionEdgeCount)."
let first = body
    .IGetIntersectionEdges(&other)
    .and_then(|ptr| ptr.read().ok_or_else(windows_core::Error::empty))?;

let ptr = first.into_raw();
let edges: Vec<IEdge> = (0..len)
    .map(|offset| IEdge::from_raw(ptr.offset(offset)))
    .collect();

Is that right? I've been programming in the lower-level Microsoft realm for just long enough to know that "if you've done it 'the wrong way', it still might 'work', and if you've done it 'the right way', it still might not work."

Should I be seeing some explicit Ref/OutRef?

Because even though I haven't fully grok'd how they work, in a post-#3386 world, not seeing them is slightly concerning to me. :)

Disclaimer

I know that "supporting 3rd-party metadata" is not a top priority for this project. The solidworks-sys crate I'm working on is not public (yet) and quite chonky (~650k lines of bindings for the main sldworks namespace alone). My apologies if this is out of scope or if providing the relevant code-snippets inline is less helpful than a live repo.

The windows-rs project is seriously magical. Thank you guys for the hard work you've put into it. I feel like I've learned a ton so far, and I'd love to see it go as far as it can go. Maybe help out if/where I can. Cheers!

Crate manifest

Crate code

[kennykerr]

Can you share the .winmd file that you're using to generate the bindings? I can have a quick look at that to see if there's anything amiss.

[EndilWayfare]

SolidWorks.txt

it's not really a .txt, but github doesn't like the .winmd extension

[kennykerr]

Here's the metadata representation for the first two build errors I encountered:

HRESULT IGetIntersectionEdges([In] IBody ToolBodyIn, [Out][RetVal] IEdge** EdgeListOut);
HRESULT IRemoveFacesFromSheet([In] int NumOfFaces, [In] IFace** FacesToRemove);

Both appear to be different array formulations that aren't handled too well because they're not consistent with existing array parameters in the Win32 metadata. You could compare this with existing APIs to see what attributes or transformations might produce working bindings. Neither indicate that they are arrays and so there's some confusion as to why these seemingly non-array parameters include the purported level of indirection.

I'm working on a next generation of parsing and code generation that should be a lot more resilient to arbitrary metadata like this but it will be a little while until that's ready to test.

[EndilWayfare]

Makes sense; that's likely why they were deprecated, if I had to guess.

Other than that, am I roughly correct about how to consume "arrays" of COM pointers like this, and what my responsibilities as the COM "caller" are?

[kennykerr]

It's not clear from this metadata. Keep in mind that the metadata implies an interface pointer when IBody is encountered. So for example IGetIntersectionEdges has a signature that is roughly equivalent to the following C++ signature:

HRESULT IGetIntersectionEdges(IBody*, IEdge***);

I have no idea whether that's correct and then what the semantics are for ownership as that is not conventional COM parameter passing.

[riverar]

Everything looks generally correct here in terms of metadata. This API returns an array of pointers, per the docs. And the length is retrieved out of band.

It might help to emit metadata with [PreserveSig] on everything to avoid any transformations that might be blowing up here.

I do want to clarify that the word "deprecated" is getting used here, but the documentation indicates these APIs are obsolete which typically means you shouldn't be using them at all and they will likely not work/be missing. So maybe excluding them from your crate is the better solution here? 😅 (Or maybe not, the suggested replacement API doesn't look much better.)

Edit: Also of interesting note is that the SolidWorks docs suggest using IDispatch methods when dealing with anything containing arrays.

[riverar]

(Sorry, am fighting my keyboard today and triggered the wrong button.)

[EndilWayfare]

Yes, I agree that going with the IDispatch methods is the best way to go.

The "deprecated" vs "obsolete" question is a tricky one. The API isn't versioned, but SolidWorks-the-application is. Yearly releases can add new methods/interfaces, but the old ones still exist and should be supported (YMMV going further into the past, but this is the "aspirational" goal) so that old clients continue to work and new clients can target old releases of SolidWorks. Because of licensing/subscription, you(r users) may be "stuck" on an old year/service-pack.

"Deprecated" generally implies "move to the new thing, the old will be removed at some point", but the "at some point" in this case is fuzzy and kind of delegated to the consuming user.

It's a difficult design question of how to surface this information in the more-idiomatic solidworks crate (if reasonable at all), but the best choice for a -sys-style crate seems to be "provide the entire interface as is documented to exist". So that "correctness" consists of "did we do what they told us to do" and leaves "but did it work" as a "that's on them" problem.

[EndilWayfare]

But in this case, "provide the entire interface" needs to be qualified with "that can be made to compile".

The "--filter" switch in windows-bindgen only works at type-level, so I can't use it to exclude these members. I already have a bit of postprocessing that gets done on the generated bindings, so I can add this filtering there.

Binding Postprocessing

windows-bindgen turns enum declarations into a newtype wrapper and module-level pub consts. I've read elsewhere here that this is the "simplest obvious correct choice", because of the C-based semantics of MIDL enums (non-exhaustiveness, and such), and I agree.

What I've done, though, is take the module-level constants and turn them into associated constants of their type, such that

pub struct swObjectEquality(pub i32);
pub const swObjectNotSame: swObjectEquality = swObjectEquality(0i32);
pub const swObjectSame: swObjectEquality = swObjectEquality(1i32);
pub const swObjectUnsupported: swObjectEquality = swObjectEquality(2i32);

becomes

pub struct swObjectEquality(pub i32);
impl swObjectEquality {
    pub const swObjectNotSame: Self = swObjectEquality(0i32);
    pub const swObjectSame: Self = swObjectEquality(1i32);
    pub const swObjectUnsupported: Self = swObjectEquality(2i32);
}
pub const swObjectNotSame: swObjectEquality = <swObjectEquality>::swObjectNotSame;
pub const swObjectSame: swObjectEquality = <swObjectEquality>::swObjectSame;
pub const swObjectUnsupported: swObjectEquality = <swObjectEquality>::swObjectUnsupported;

This way, you can do the C/C++-style "brought in by a header somewhere" existing behavior, but also a more C#-style "namespaced to type" access.

[kennykerr]

What's the plan here? Metadata fix or bindgen hardening?