dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.97k stars 4.66k forks source link

GeneratedComInterfaceAttribute can cause error CS0111: Type 'InterfaceImplementation' already defines a member called 'ABI_Blah' with the same parameter types #101242

Closed smourier closed 4 months ago

smourier commented 5 months ago

Description

When using .NET8's GeneratedComInterfaceAttribute, we cannot define a COM interface with two methods with the same name if they share a "similar" signature (not sure exactly of what type of parameter causes it but with different COM interface parameters it fails), it raises CS0111: Type 'InterfaceImplementation' already defines a member called 'ABI_Blah' with the same parameter types.

Reproduction Steps

Consider this simple .NET 8 project:

Exe net8.0 enable true true true

And these interface definitions:

    [GeneratedComInterface, Guid("e4817011-64d7-4626-a2ed-ac0cf6ad76e7")]
    public partial interface IFace1
    {
        void Blah(IFace2 p);
        void Blah(IFace3 p);
    }

    [GeneratedComInterface, Guid("ccdd226a-30fc-4f87-9b17-492b30f81cab")]
    public partial interface IFace2
    {
    }

    [GeneratedComInterface, Guid("dda31fd2-813a-461f-928e-b129481f11a3")]
    public partial interface IFace3
    {
    }

Expected behavior

It should just compile fine.

Note this type of interface is perfectly valid in COM, here is a Microsoft one https://learn.microsoft.com/en-us/windows/win32/api/dwrite_3/nn-dwrite_3-idwritegdiinterop1 defined like this:

interface DWRITE_DECLARE_INTERFACE("4556BE70-3ABD-4F70-90BE-421780A6F515") IDWriteGdiInterop1 : public IDWriteGdiInterop
{
    ...
    STDMETHOD(GetFontSignature)(
        _In_ IDWriteFont* font,
        _Out_ FONTSIGNATURE* fontSignature
        ) PURE;

    STDMETHOD(GetFontSignature)(
        _In_ IDWriteFontFace* fontFace,
        _Out_ FONTSIGNATURE* fontSignature
        ) PURE;
    ....
};

Actual behavior

Building creates this error

E:\temp\ConsoleApp6.Program.IFace1.cs(93,25,93,33): error CS0111: Type 'InterfaceImplementation' already defines a member called 'ABI_Blah' with the same parameter types

Regression?

I don't think so since GeneratedComInterfaceAttribute is pretty new.

Known Workarounds

We can just rename IFace's second Blah with something else that doesn't conflict as COM names are not technically important but this raises lots of other issues, especially with .NET where names are.

Configuration

.NET 8.0.4 (tested on x64 but I don't think it's relevant here).

Other information

No response

AaronRobinsonMSFT commented 4 months ago

Note this type of interface is perfectly valid in COM

This is not true. Function overloading is not valid in a COM interface. The MIDL compiler specifically doesn't support this - see here.

here is a Microsoft one https://learn.microsoft.com/en-us/windows/win32/api/dwrite_3/nn-dwrite_3-idwritegdiinterop1 defined like this:

This is invalid in IDL, probably why the IDL for these interfaces aren't public.

smourier commented 4 months ago

This is not true. Function overloading is not valid in a COM interface. The MIDL compiler specifically doesn't support this - see here.

MIDL doesn't allow overloading but I don't think MIDL (or even IDL) has ever been strictly mandatory to do COM. And I kinda remember overloading was not prohibited by ODL/IDL (just by MIDL). Their purpose is to define binary contracts, not named one, so even the term "overloading" is irrelevant to COM interfaces, it's more an OOP (ie historically C++) term.

I mean, saying you don't want to do because it's too weird is one thing, saying it's invalid in COM is another.

Quite a number of headers in the SDK were never based on IDL (not only DWrite, dbgeng.h, dbdaoint.h, xaudio2.h etc.).

AaronRobinsonMSFT commented 4 months ago

I mean, saying you don't want to do because it's too weird is one thing, saying it's invalid in COM is another.

It is invalid because it can't be expressed in C. The name of the function needs to be different or else during compilation there will be a redefinition of the same function. Yes, it can be express in C++, but that is an implementation detail that is often exploited because C is used far less often. However the expressibility of the contract must adhere to the lowest common denominator and in this case that is C.

I'm not against adding support for this but we are going to hit several complex issue if/when we enable the export scenario. Meaning define in C# and generate IDL/unmanaged defintiions. The MIDL compiler, if I recall, adds a suffix in some cases to disambiguate. My gripe is trying to reconcile our potential approach with whatever the MIDL compiler does.

smourier commented 4 months ago

It is invalid because it can't be expressed in C. The name of the function needs to be different or else during compilation there will be a redefinition of the same function. Yes, it can be express in C++, but that is an implementation detail that is often exploited because C is used far less often. However the expressibility of the contract must adhere to the lowest common denominator and in this case that is C.

You can say "ComWrappers source generator only supports MIDL constructs", but I find that a bit rough (hum... ok, I'll trade it for TLB generation back... deal?).

COM has always been language-agnostic (it was historically even platform-agnostic), there's no notion of overloading in it since it's "only" a binary standard, talks about interface ids, vtable binary layouts, method orders and signatures, never about names (and so it doesn't even have to come up with mangled name algorithms like with C++).

https://learn.microsoft.com/en-us/windows/win32/com/the-component-object-model

The only language requirement for COM is that code is generated in a language that can create structures of pointers and, either explicitly or implicitly, call functions through pointers

https://learn.microsoft.com/en-us/windows/win32/com/com-technical-overview#objects-and-interfaces

IDL is a tool for the convenience of the interface designer and is not central to COM interoperability

Is it officially specified somewhere that if a COM interface cannot be expressed in C, it's invalid?

AaronRobinsonMSFT commented 4 months ago

@smourier I agree with where you are going with this and your perspective is the same I typically have - COM is an ABI defining technology. However, that just isn't how it is used or thought about in practice. It is a specific implementation of an ABI and comes with a myriad of special cases that, when not followed, create complex support scenarios that are leveraged for clever and niche purposes. The .NET runtime's goal, as I see it, is to enable support for COM as defined by the suite of tooling that exists for COM on the Windows platform. This means tools like the MIDL compiler, regsvr32, et al are the source of truth and reflect what is expected from the COM ecosystem.

Yes, the layout and order of COM vtable slots are technically all that matters and programming languages shouldn't be a concern. Yes, one can manually define their own IUnknown, generally follow the ABI rules and the implementation will likely interoperate with most other COM aware technologies - VBScript, CScript, MSVC C and C++, and of course .NET. Yes, naming is orthogonal to the ABI itself and should be the concern of the programming language consuming/projecting. Yes, the HRESULT return value is primarily for remoting.

We are closer than might appear. My biggest concern here is exposing niche features that are leveraged by teams that merely want a reference counting interop story so they reach for IUnknown and really aren't doing anything with COM. The biggest offenders of this sort of thing are the DirectDraw/Direct2D/Direct3D APIs. Returning structs from a COM function function exposes a myriad of ABI issues with MSVC, outside of remoting, that have forced the interop team to create annoying bespoke solutions - we addressed this particular one in .NET Core by creating the CallConvMemberFunction.

All this reduces to, sure this can be done in COM if one does a ton of manual work and avoids using the most common tools. The DirectDraw/Direct2D/Direct3D APIs have well-maintained managed projections like Win2D and TerraFx. If this becomes a blocking issue for other APIs, lets reconsider it, but for now I'd like to avoid continuing down the path of adding these sorts of features when the workaround is so simple.