-
Notifications
You must be signed in to change notification settings - Fork 115
Handle CoCreateable classes in ComSourceGenerators mode #1502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Have you considered just making the class a static class? [Guid("00021401-0000-0000-C000-000000000046")]
internal partial static class ShellLink
{
public static T CreateInstance<T>() where T : class
{
PInvoke.CoCreateInstance<T>(typeof(ShellLink).GUID, null, CLSCTX.CLSCTX_SERVER, out T ret).ThrowOnFailure();
return ret;
}
}Also, does it need to generic? One other thing to consider is putting these statics on the actual interfaces when you're constrained to your namespace Windows.Win32.UI.Shell;
public unsafe partial interface IShellLinkW
{
public static Guid ClassGuid => new("00021401-0000-0000-C000-000000000046");
public static IShellLinkW CreateInstance()
{
PInvoke.CoCreateInstance(ClassGuid, null, CLSCTX.CLSCTX_SERVER, out IShellLinkW ret).ThrowOnFailure();
return ret;
}
}Having the helpers hanging off the interface might improve discovery. You could also include the static class as well to get people looking for it both ways? |
I'm hoping to find a syntax that could work for both ComImport mode and not. ComImport has the constraint that class must be empty, so that does have me wondering if there should be a special static factory class somewhere else, where it could be static.
Unfortunately the declaration of coclass in win32metadata doesn't tell you anything other than the class name and GUID. What interfaces are associated with it is not described. So there's no way to know IShellLinkW would be the right interface. Also with this approach you can ask for any interface on the thing, like: |
Bummer, might be a nice addition in the future. |
Also, being a class with constructor that I can mark Obsolete will help folks find the new pattern when migrating from ComImport to GeneratedComInterface mode, so I think I'll stick with this pattern. With .NET 10 I can do "extension everything" to make this a static extension method even on the ComImport type so folks can use same syntax before/after. |
This is a fine approach, but there is a slightly more AOT friendly manner. [Guid("00021401-0000-0000-C000-000000000046")]
internal partial static class ShellLink
{
private static ReadOnly<byte> CLSID_ShellLink = /* 00021401-0000-0000-C000-000000000046 as a byte array */;
...
public static T CreateInstance<T>() where T : class
{
PInvoke.CoCreateInstance<T>(new Guid(CLSID_ShellLink), null, CLSCTX.CLSCTX_SERVER, out T ret).ThrowOnFailure();
return ret;
}
}This is more pay-for-play. If a user does |
If no one calls CreateInstance, won't the trimmer see it's uncalled and also remove the reference to typeof(ShellLink).GUID and thus realize it doesn't need to keep metadata? Or is the determination of what metadata to keep made before trimming? |
|
But I'm ok with adding a static. Is there a reason the static should be ReadOnlySpan and then construct the GUID in the function versus the static is: ? |
Updated [Microsoft.Windows.CsWin32](https://github.com/microsoft/CsWin32) from 0.3.228 to 0.3.238. <details> <summary>Release notes</summary> _Sourced from [Microsoft.Windows.CsWin32's releases](https://github.com/microsoft/CsWin32/releases)._ ## 0.3.238 ## Changes: * #1520: Don't make void* params Span<byte> in friendly methods * #1517: CsWin32Generator should allow newer language versions This list of changes was [auto generated](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=12697392&view=logs). ## 0.3.236 NOTE: This changes the signature of methods with optional parameters. This change is also documented at https://microsoft.github.io/CsWin32/docs/getting-started.html: ### Optional out/ref parameters Some parameters in win32 are `[optional, out]` or `[optional, in, out]`. C# does not have an idiomatic way to represent this concept, so for any method that has such parameters, CsWin32 will generate two versions: one with all `ref` or `out` parameters included, and one with all such parameters omitted. For example: ```c# // Omitting the optional parameter: IsTextUnicode(buffer); // Passing ref for optional parameter: IS_TEXT_UNICODE_RESULT result = default; IsTextUnicode(buffer, ref result); ``` ### Working with Span-typed and MemorySize-d parameters In the Win32 APIs there are many functions where one parameter is a buffer (`void*` or `byte*`) and another parameter is the size of that buffer. When generating for a target framework that supports Spans, there will be overloads of these functions that take a `Span<byte>` which represents both of these parameters, since a Span refers to a chunk of memory and a length. For example, an API like [IsTextUnicode](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-istextunicode) has a `void*` parameter whose length is described by the iSize parameter in the native signature. The CsWin32 projection of this method will be: ```c# BOOL IsTextUnicode(ReadOnlySpan<byte> lpv, ref IS_TEXT_UNICODE_RESULT lpiResult) ``` Instead of passing the buffer and length separately, in this projection you pass just one parameter. Span is a flexible type with many things that can be converted to it safely. You will also see Span parameters for things that may look like a struct but are variable sized. For example, [InitializeAcl](https://learn.microsoft.com/en-us/windows/win32/api/securitybaseapi/nf-securitybaseapi-initializeacl) looks like it returns an ACL struct but the parameter is annotated with a `[MemorySize]` attribute in the metadata, indicating it is variable-sized based on another parameter. Thus, the cswin32 projection of this method will project this parameter as a `Span<byte>` since the size of the parameter is variable: ```c# // The cswin32 signature: static BOOL InitializeAcl(Span<byte> pAcl, ACE_REVISION dwAclRevision) { ... } ``` And you would call this by creating a buffer to receive the ACL. Then, after the call you can reinterpret the buffer as an ACL: ```c# // Make a buffer Span<byte> buffer = new byte[CalculateAclSize(...)]; InitializeAcl(buffer, ACE_REVISION.ACL_REVISION); // The beginning of the buffer is an ACL, so cast it to a ref: ref ACL acl = ref MemoryMarshal.AsRef<ACL>(buffer); // Or treat it as a Span: Span<ACL> aclSpan = MemoryMarshal.Cast<byte, ACL>(buffer); ``` CsWin32 will also generate a struct-typed parameter for convenience but this overload will pass `sizeof(T)` for the length parameter to the underlying Win32 API, so this only makes sense in some overloads such as [SHGetFileInfo](https://learn.microsoft.com/windows/win32/api/shellapi/nf-shellapi-shgetfileinfow) where the parameter has an annotation indicating it's variable-sized, but the size is only ever `sizeof(SHFILEINFOW)`: ```c# // Span<byte> overload: static nuint SHGetFileInfo(string pszPath, FILE_FLAGS_AND_ATTRIBUTES dwFileAttributes, Span<byte> psfi, SHGFI_FLAGS uFlags) // ref SHGETFILEINFOW overload: static nuint SHGetFileInfo(string pszPath, FILE_FLAGS_AND_ATTRIBUTES dwFileAttributes, ref SHFILEINFOW psfi, SHGFI_FLAGS uFlags) ... (truncated) ## 0.3.235 ## What's Changed * Handle CoCreateable classes in ComSourceGenerators mode by @jevansaks in microsoft/CsWin32#1502 * Simplify decimal conversions by @jevansaks in microsoft/CsWin32#1512 * Prevent SafeHandle from being re-generated in downstream assembly by @jevansaks in microsoft/CsWin32#1514 * Fix ArithmeticOverflow in HANDLE types and other helpers when CheckForOverflowUnderflow is enabled by @jevansaks in microsoft/CsWin32#1513 **Full Changelog**: microsoft/CsWin32@v0.3.228...v0.3.235 Commits viewable in [compare view](microsoft/CsWin32@v0.3.228...v0.3.238). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
COM source generators do not handle co-classes, you have to manually call CoCreateInstance. Per my proposal in #1500, I am making CsWin32 generate a class that looks like this:
So the consumer would write code like this instead:
One downside is this means it's hard to write consuming code that uses co classes and flips between the two modes. The problem is, in ComImport mode, you can't have any methods on the class so we'd have to hide the factory method somewhere else which makes it less discoverable for the COM generators. In a future change we can use the "extension everything" .NET 10 feature to make this available in both modes.
I also generated a
CreateInstance<T>for allowMarshaling=false mode because ... why not? The signature of the method returns HRESULT because it has to return a pointer out param. But this fixes #1422.Fixes #1422, #1500.