Skip to content
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

Harden method overload support #3477

Merged
merged 7 commits into from
Feb 10, 2025
Merged

Harden method overload support #3477

merged 7 commits into from
Feb 10, 2025

Conversation

kennykerr
Copy link
Collaborator

@kennykerr kennykerr commented Feb 7, 2025

Rust does not support method overloads so windows-bindgen handles overloads assumed by many Windows APIs in a way that those APIs can be both called, and if necessary implemented, in Rust. Today I came across some metadata for a new project that broke in some surprising ways. Unfortunately, developers may not think to test their APIs with languages that lack overload support and tools like MIDLRT don't help to catch such mistakes.

Consider a simple IDL example:

runtimeclass A 
{
    A();
    Int32 Method();
    Int32 Method(Int32 a);
}

This class has an overloaded method named "Method". MIDLRT will generate the Overload attribute to provide alternative method names for languages that lack overload support. You might not realize this but while C++ supports overloads, COM does not. So this impacts even C++ code using these APIs, although C++/WinRT will smooth things over for you. The metadata will look something like this (I've elided some unimportant details):

public sealed class A : IA
{
    public extern A();

    [Overload("Method")]
    public extern int Method();

    [Overload("Method2")]
    public extern int Method([In] int a);
}

The overloads are actually defined by the exclusive IA interface synthesized by MIDLRT, but it's convenient to think of them as class methods, logically speaking. Languages like Rust that don't support overloads can use the method names Method and Method2 to call this API.

So far so good. Consider a second IDL example:

runtimeclass B 
{
    B();
    [method_name("MethodOne")] Int32 Method();
    [method_name("MethodTwo")] Int32 Method(Int32 a);
}

This class defines the overload names explicitly so MIDLRT doesn't have to. The metadata will look something like this:

public sealed class B : IB 
{
    public extern B();

    [Overload("MethodOne")]
    public extern int Method();

    [Overload("MethodTwo")]
    public extern int Method([In] int a);
}

Languages like Rust that don't support overloads can use the method names MethodOne and MethodTwo to use this API. This is always preferred because the API author can decide what the overloads should be called and give them more descriptive and meaningful names.

But it's not just about making things easier for Rust developers. Consider the following IDL example:

[exclusiveto(D)]
interface ID
{
    Int32 Method();
    Int32 Method(Int32 a);
}

[exclusiveto(D)]
interface ID2
{
    Int32 Method(Int32 a, Int32 b);
    Int32 Method(Int32 a, Int32 b, Int32 c);
}

runtimeclass D: [default] ID, ID2
{
    D();
}

This class defines the overloads of Method across two different exclusive interfaces. The trouble is that MIDLRT will now, regrettably, add the Overload attributes for each interface without considering the fact that these interfaces have to overload with each other in the class. The metadata will look something like this:

public sealed class D : ID, ID2
{
    public extern D();

    [Overload("Method")]
    public extern int Method();

    [Overload("Method2")]
    public extern int Method([In] int a);

    [Overload("Method")]
    public extern int Method([In] int a, [In] int b);

    [Overload("Method2")]
    public extern int Method([In] int a, [In] int b, [In] int c);
}

See the problem? There are two overloads called "Method" and two overloads called "Method2".

In summary, don't rely on MIDLRT to deal with overloads. At best it will make it inconvenient for developers. At worst it will make your API unusable in some languages. Always specify the overloads explicitly. Something like this:

[exclusiveto(E)]
interface IE
{
    [method_name("MethodOne")] Int32 Method();
    [method_name("MethodTwo")] Int32 Method(Int32 a);
}

[exclusiveto(E)]
interface IE2
{
    [method_name("MethodThree")] Int32 Method(Int32 a, Int32 b);
    [method_name("MethodFour")] Int32 Method(Int32 a, Int32 b, Int32 c);
}

runtimeclass E: [default] IE, IE2
{
    E();
}

This update to windows-bindgen just further hardens the code generator to deal with this pathological case by detecting these machine-generated overload names and contextualizing them properly for APIs.

@riverar
Copy link
Collaborator

riverar commented Feb 7, 2025

Thanks for the super quick turnaround!

@kennykerr kennykerr merged commit b8e3197 into master Feb 10, 2025
78 checks passed
@kennykerr kennykerr deleted the overloads branch February 10, 2025 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants