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

[C# 7.3] Better (but not bestest) betterness fix #499

Closed
BillWagner opened this issue Mar 16, 2022 · 1 comment · Fixed by #263
Closed

[C# 7.3] Better (but not bestest) betterness fix #499

BillWagner opened this issue Mar 16, 2022 · 1 comment · Fixed by #263
Milestone

Comments

@BillWagner
Copy link
Member

I found the following while working on #484

Consider this code:

public class BrokenOverloadResolution
{
    static void Test(Action a)
    {
    }

    static void Test(Func<object> f)
    {
    }

    public void MethodGroupExactMatching()
    {
        // This fails to compile with
        // 
        // error CS0121: The call is ambiguous between the following methods or properties: 'BrokenOverloadResolution.Test(Action)' and 'BrokenOverloadResolution.Test(Func<object>)'
        //
        // Fixed in 7.3
        // Probably in Method group conversion:
        // "For a method group conversion, candidate methods whose return type doesn't match up with the delegate's return type are removed from the set."
        // https://github.com/dotnet/roslyn/issues/8942#issuecomment-299536091
        //Test(Listen);

        //
        // non-void version works
        //
        // Test2 (DontListen);
    }

    void Listen()
    {
    }

    object DontListen()
    {
        return null;
    }
}

This compiles without error starting in V7.3, based on this issue: dotnet/roslyn#8942 (comment)

BillWagner added a commit to BillWagner/csharpstandard that referenced this issue Mar 16, 2022
This gets close to what's implemented in C# 6.0.

I think this fails as a description in the case of dotnet#499

For 7.3, it would be better to describe that change in the section on Method Group conversion.
@jskeet
Copy link
Contributor

jskeet commented Mar 16, 2022

Shorter version I'm using to check for breaking changes:

using System;

public class BrokenOverloadResolution
{
    static void Test(Action a)
    {
        Console.WriteLine("Action");
    }

    static void Test(Func<object> f)
    {
        Console.WriteLine("Func");
    }

    static void Main()
    {
        Test(DontListen);
    }

    static object DontListen()
    {
        return null;
    }
}

With C# compiler version 4.1.0-5.22109.6, this prints "Func"

BillWagner added a commit that referenced this issue Mar 24, 2022
* Incorporate Better Betterness

This commit adds the notes from https://github.com/dotnet/roslyn/blob/main/docs/specs/CSharp%206/Better%20Betterness.md

* Incorporate changes from #283

Issue #283 contains notes for how the implementation differed from the proposed better betterness text.

* compat for delegate conversion

This commit brings in the method group conversion fix that caused dotnet/roslyn#6750

It needs wordsmithing for a number of reasons:
- "corresponding argument" isn't defined.
- the section references itself.
- I think there may have been requirements from the prior commit regarding delegates and expressions that are still needed.

* initial feedback.

* section reference

* a bit of editing

* I think this gets close

This gets close to what's implemented in C# 6.0.

I think this fails as a description in the case of #499

For 7.3, it would be better to describe that change in the section on Method Group conversion.

* review from the meeting.

* fix awkward grammar

* limit bullet to method group conversion.
@gafter gafter added this to the C# 7.x milestone May 13, 2022
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 a pull request may close this issue.

3 participants