Bring back the cocreateable structs

[AArnott]

1204 removed cocreateable structs from the metadata. This breaks scenarios like this one:

On .NET you can actually create these structs and underneath you get the COM object, such that you can cast to a COM interface and call into it. But when the struct is removed (on the basis that the GUID constant is still there) CsWin32 cannot tell that a cocreateable struct should be created for that guid constant, and folks using CsWin32 to cocreate these objects are now broken.

This technique is shown off in this blog post: https://mike-ward.net/2008/09/02/a-lean-method-for-invoking-com-in-c/

Please bring back the cocreateable structs!

[kennykerr]

Fine! 😜 Just make sure there aren't any dupes and l'll go back to harmonizing them in Rust.

[AArnott]

So... to avoid dupes are you suggesting we drop the constants, since we must keep the structs, @kennykerr? Where did the constants come from in the first place? I can't imagine the original C header files defining the same symbol as both a constant and a struct type.

[kennykerr]

Yep, it seems to do that erroneously. See CODECAPI_ALLSETTINGS for an example.

[riverar]

I'm not convinced we should be keeping these structs around in metadata. It's the job of the projection author to generate idiomatic code for their world.

[kennykerr]

You can always recreate the structs in whatever way makes the CLR happy on your end. 😊

[AArnott]

It's the job of the projection author to generate idiomatic code for their world. You can always recreate the structs in whatever way makes the CLR happy on your end. 😊

The metadata in v30 doesn't provide the info CsWin32 would need to create the structs. All we have left are guid constants, and since not all guid constants represent cocreateable objects, CsWin32 cannot function without the structs. I agree idiomatic specifics should be in the projections, but the data has to be in the metadata in the first place. If other projections don't like the structs, can't you ignore them? In fact CsWin32 doesn't take them as-is either: we change them to classes, because that's what .NET requires.

I'm not convinced we should be keeping these structs around in metadata

Why not? The structs are in the header files. Shouldn't the metadata retain what's in the headers?

[kennykerr]

since not all guid constants represent cocreateable objects

Not all structs with a GUID attribute are creatable either. In fact, many many are not. Take CODECAPI_ALLSETTINGS for example. 😁

[riverar]

Is that always true?

// Windows.Win32.winmd, Version=24.0.1.28499
[Guid(136193u, 0, 0, 192, 0, 0, 0, 0, 0, 0, 70)]
public struct ShellLink
{
}

[AArnott]

@riverar I deleted my last comment (that you were evidently replying to) because I was mistaken. The COMImport attribute was something CsWin32 adds.

[riverar]

@aarnott Ah ok!

[AArnott]

I guess we should look for a way for the metadata to convey which guids/structs are cocreatable. So long as there's a way for CsWin32 to create these cocreatable classes, I'll be content. It would certainly be goodness to avoid generating a C# class for CODECAPI_ALLSETTINGS since it isn't cocreatable.

There does appear to be evidence in the header files to identify that ShellLink is cocreateable. While on the other hand, CODECAPI_ALLSETTINGS is only a constant in the headers.

So it seems to me that if the metadata conflates these two, it needn't. So perhaps we can have our constants for CODECAPI_ALLSETTINGS (without a struct) and have our struct for ShellLink after all.

[riverar]

Can you provide a snippet of what that link is pointing to, for external peasants like me?

[kennykerr]

Unless the Win32 metadata can definitively and accurately distinguish this, I'd prefer to just leave it as is now - a consistent set of GUID constants - as there's really no way to look at the Windows headers and know whether a given GUID constant or struct is intended for CoCreateInstance or a million other uses. A given language can then decide how they want to handle that ambiguity.

[sotteson1]

I think I jumped the gun on this one @AArnott. I was worried the structs were needed as things that could be CoCreated but @kennykerr said he didn’t need them. I’m going to bring them back and ensure we don’t have constants and structs with the same name and GUID.

[riverar]

@sotteson1 I'm okay with rolling back the change temporarily but we should continue the conversation here on how we clean these up properly.

[AArnott]

@riverar sure:

One header file contains this:

// CoClasses:
class __declspec(uuid("00021401-0000-0000-c000-000000000046")) ShellLink;

That makes it pretty obvious that ShellLink is meant as a coclass.

It also appears elsewhere like this:

#ifdef __cplusplus
typedef class ShellLink ShellLink;
#else
typedef struct ShellLink ShellLink;
#endif /* __cplusplus */

In contrast the "only a constant" case appears only simply like this:

// {6a577e92-83e1-4113-adc2-4fcec32f83a1 }
OUR_GUID_ENTRY(CODECAPI_ALLSETTINGS,
0x6a577e92, 0x83e1, 0x4113, 0xad, 0xc2, 0x4f, 0xce, 0xc3, 0x2f, 0x83, 0xa1)

[sotteson1]

If there are no duplicates, what’s left to clean up?

[AArnott]

@sotteson1 for one thing, CODECAPI_ALLSETTINGS appears as a struct and a constant in the metadata. If you can discern that it should only be a constant (because it is not cocreateable) and keep ShellLink as a struct (because it is cocreateable), and of course do this in a general way, then I think we're good.

While CODECAPI_ALLSETTINGS was defined both ways but ShellLink was only defined as a struct, it seems that something in the metadata production can tell a difference. So if your fix is to suppress the struct only where the constant also exists, that sounds like it may strike the right result.

[kennykerr]

// CoClasses: class __declspec(uuid("00021401-0000-0000-c000-000000000046")) ShellLink;

Unless Steve plans to parse the "CoClasses" comment that seems irrelevant.

There are also plenty of GUID constants that aren't structs and yet are meant to be used with CoCreateInstance. So either way, you're left with ambiguity.

[AArnott]

The // CoClasses may indeed be irrelevant for the parser, but the class keyword isn't, right? That gives away that it should be a class. Given the CODECAPI_ALLSETTINGS constant does not appear with a class keyword anywhere seems to validate that this could be the condition we use.

[riverar]

I think a better approach would be for the C# side to gather up all CLSID GUIDs and generate the structures it needs.

That said, CLSID_ShellLink is currently missing. We probably need to change the IDL workflow to do the right thing when it encounters:

library ShellCoreObjects
{
...
    // CLSID_ShellLink
    [ uuid(00021401-0000-0000-C000-000000000046) ] coclass ShellLink { interface IShellLinkW; }

[kennykerr]

but the class keyword isn't, right?

Not really. C++ class and struct are the same thing. The only difference has to do with default visibility.

[AArnott]

Not really. C++ class and struct are the same thing.

Why does that matter? Either keyword makes ShellLink different from CODECAPI_ALLSETTINGS because CODECAPI_ALLSETTINGS doesn't appear as a struct or class in the headers.

[AArnott]

for the C# side to gather up all CLSID GUIDs and generate the structures it needs

Forgive my COM ignorance, but are all CLSIDs meant to represent cocreateable COM objects? If so, that might work. But how would CsWin32 "gather up" all of them? I don't see anything in the metadata to identify such CLSIDs. If they all are guaranteed to uniquely follow a naming convention, that would kinda work. But as CsWin32 works with other metadata too (not just win32metadata), I'd rather we know definitively rather than relying the APIs to just happen to follow some naming convention.

[sotteson1]

Here's what I propose: bring back the structs, as most of them do represent something CoCreatable. For ones that aren't that we want to be constants, we can provide a way to turn them into constants via the settings. Rust can continue to collapse them all into constants, and CsWin32 can treat them as CoCreatable objects. Also, the tests will fail if there and any guid-only structs and guid constants that share the same name. What do you all think?

[AArnott]

I think if we can do that quickly, that's good as it will unblock CsWin32 to upgrade from v24 metadata that we're on now. I'd rather avoid the manual selection of non-cocreatable structs though, even if it is kept in the metadata repo. What do you think of the techniques I suggest above to discern between the two cases via the header files while producing the metadata?

[sotteson1]

ClangSharp won't tell me if it's a struct or class. They both get scraped as a struct. But I think the vast majority of these are class, not struct, and they're meant to be CoCreatable.

[riverar]

Here's what I propose: bring back the structs [...]

Agreed, for the short term.

For ones that aren't that we want to be constants, we can provide a way to turn them into constants via the settings. [...]

I don't agree with this approach as it imposes additional (and arguably unnecessary) upkeep on metadata side.

We are currently discussing here how to identify coclasses and their associated IDs, so authors can grab the authoritative list and do something with it (like generate structures). I think we're narrowing in on a plan, where we alter metadata tooling to get this info (e.g. recognize CLSID_ constant name, parse IDL, read TLBs, etc) and perhaps emit a [CLSID] attribute of sorts.

Similar discussion / need: https://github.com/microsoft/win32metadata/issues/271

[AArnott]

ClangSharp won't tell me if it's a struct or class. They both get scraped as a struct

We don't need to distinguish between struct and class though. We just need to distinguish whether struct/class appears or not. ShellLink is a struct/class and CODECAPI_ALLSETTINGS is not. Doesn't that give us the info we need?

[sotteson1]

This code creates a struct for CODECAPI_ALLSETTINGS:

ksmedia.h: DEFINE_GUIDSTRUCT("6A577E92-83E1-4113-ADC2-4FCEC32F83A1", CODECAPI_ALLSETTINGS );

define CODECAPI_ALLSETTINGS DEFINE_GUIDNAMED(CODECAPI_ALLSETTINGS )

I have no idea why they did that. But the same guid is created in uuids.h, so I can just exclude the one from ksmedia.h which gets rid of the struct.

[AArnott]

Fixed by #1214