Reconsider where we use class instead of struct about pinvoke HOT 11 CLOSED

Maybe we could define another codegen attribute to decorate struct parameters with that causes another overload to be generated with IntPtr there so that someone can easily pass in IntPtr.Zero to omit the optional parameter:

public static extern bool SetupDiEnumDeviceInterfaces(
    SafeDeviceInfoSetHandle deviceInfoSet,
    [Optional] ref SP_DEVINFO_DATA deviceInfoData,
    ref Guid interfaceClassGuid,
    int memberIndex,
    ref SP_DEVICE_INTERFACE_DATA deviceInterfaceData);

Also produces:

public static extern bool SetupDiEnumDeviceInterfaces(
    SafeDeviceInfoSetHandle deviceInfoSet,
    IntPtr deviceInfoData,
    ref Guid interfaceClassGuid,
    int memberIndex,
    ref SP_DEVICE_INTERFACE_DATA deviceInterfaceData);

from pinvoke.

Yes using native pointers to structs seems to be more in-line with the latest changes and if it also removes the negative side effects. why not?.
So +1 for structs.

from pinvoke.

A couple of options that occur to me:

1. The hand-authored P/Invoke signature uses struct* syntax. The generator produces the ref struct overload. But that assumes the pointer is to just one struct when it might be the start of an array.
2. The hand-authored P/Invoke signature uses ref struct syntax. The generator produces IntPtr and/or struct* overloads. No negative side-effects that I can think of, but means our code generator is now producing overloads in the opposite direction as well (from non-pointer to pointer).

With the latter option, we may not even need an [Optional] attribute. In native code, it's always a pointer, so documentation is required to know if it's optional. But if we add such an attribute, the user doesn't need to rely on documentation for it.

from pinvoke.

I rather be consistent and avoid the 2nd option. So for the 1st one what about to create an attribute OfferArrayOverloadAttribute at the method level specifying if we need the array overload on that specific method something like:

[OfferArrayOverload()]
public static extern bool SetupDiEnumDeviceInterfaces(
    SafeDeviceInfoSetHandle deviceInfoSet,
    [Optional] SP_DEVINFO_DATA* deviceInfoData,
    ref Guid interfaceClassGuid,
    int memberIndex,
    ref SP_DEVICE_INTERFACE_DATA deviceInterfaceData);

Then we can use that per case basis rather than globally in the class and the direction of code generation is always the same (from pointer to non-pointer).

from pinvoke.

Now that we're talking about arrays, it occurs to me we can solve this problem without classes or special generators: just expose the struct as an array:

public static extern bool SetupDiEnumDeviceInterfaces(
    SafeDeviceInfoSetHandle deviceInfoSet,
    SP_DEVINFO_DATA[] deviceInfoData,
    ref Guid interfaceClassGuid,
    int memberIndex,
    ref SP_DEVICE_INTERFACE_DATA deviceInterfaceData);

That array may have one element, or many. But it's passed by reference, it can be null, and doesn't require any pointers.
It does require an allocation where a pointer to a struct wouldn't, however. But if our generator created an overload where struct[] were converted to struct*, that would allow avoiding the allocation where necessary. That would violate your preference that we not generate native pointers where there were none. On the other hand though, we're already generating methods with native pointers (but the original method had native pointers in those cases, whereas this wouldn't).

from pinvoke.

OfferArrayOverloadAttribute at the method level

We'd need it at the parameter level. Since different parameters of the same method would need different treatment.

Another concern I have with this is that with additional attributes, authoring code in these libraries becomes increasingly unnatural. I'm afraid the guidance would lead to "use this, don't use that, and make sure in this case you add extra attributes, except in this corner case...".

from pinvoke.

I'm liking the first option better as well. Simply because it's natural for a contributor to read the docs that say "takes a pointer to struct" and write out struct* because it's so close to the doc and native header code.
As for whether the code gen produces ref struct or struct[], yes a parameter attribute would be able to indicate. I wonder what the default should be and how we could help the author discover when the default is inappropriate. We might have an attribute that specifies either way, and emit a compiler warnings when the attribute is missing to help prevent incorrect overloads that never get noticed because the author (and PR reviewers) never see them.

from pinvoke.

We'd need it at the parameter level

Agree.

I'm afraid the guidance would lead to "use this, don't use that, and make sure in this case you add extra attributes, except in this corner case...".

You will have that either way. If we go through the 2nd path contributors will have to know the insights of writing the code to auto generate overloads using the code generation (ie: use an struct array if you want an struct pointer but use a native pointer for built-In types, etc).

from pinvoke.

I think the default should be struct[], it is more general for the same reasons you gave here and some hint when it is not appropriated would be ideal.

Actually I am not quite convinced the same reasons will apply here. So I am back to "don`t know which one is more general use case"

If we can't find a clear winner. I would lean toward ref struct and then use the attribute to generate the array on those places where we know the parameter is in fact an array.

from pinvoke.

OK, so sum up on the discussion and requirements:

1. Hand-authored overload uses struct*
2. Code gen may need to generate IntPtr, and either ref struct or struct[]

In all cases if a parameter accepts an "optional" struct (per docs), the IntPtr (or struct[] when applicable) can satisfy the need for VB callers.

Code gen should determine whether to generate ref struct or struct[] based on a parameter level attribute. The details of that attribute TBD.

from pinvoke.

We'll take care of the rest with #91

from pinvoke.