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

Need to re-analyze method groups #32697

Closed
jcouv opened this issue Jan 23, 2019 · 0 comments · Fixed by #35655
Closed

Need to re-analyze method groups #32697

jcouv opened this issue Jan 23, 2019 · 0 comments · Fixed by #35655

Comments

@jcouv
Copy link
Member

jcouv commented Jan 23, 2019

In Binder.CreateConversion, the bound method group node gets fixed to have a type (with oblivious annotations). During nullable analysis, we need to undo and re-do that with proper nullable annotation.

Note there is a parallel with lambda conversions. In NullableWalker.GetUnboundLambda() and NullableWalker.ApplyConversion we do this process for lambdas. I suspect we need something similar for method groups.

We should test all scenarios involving NullableWalker.RemoveConversion(). I suspect this logic of undoing effects of Binder.CreateConversion should be factored there. For example, new[] { new Func<string?>(...), () => "" };.

        [Fact]
        public void SuppressNullableWarning_LambdaInOverloadResolution()
        {
            var source =
@"class C
{
    static void Main(string? x)
    {
        var s = M(() => { return x; });
        s /*T:string?*/ .ToString(); // 1

        var s2 = M(() => { return x; }!); // suppressed
        s2 /*T:string?*/ .ToString(); // 2

        var s3 = M(M2);
        s3 /*T:string*/ .ToString(); // 3

        var s4 = M(M2!); // suppressed
        s4 /*T:string*/ .ToString(); // 4
    }
    static T M<T>(System.Func<T> x) => throw null;
    static string? M2() => throw null;
}";
            var comp = CreateCompilation(source, options: WithNonNullTypesTrue());
            comp.VerifyTypes();

            // TODO2
            // Missing warnings on s3 and s4
            comp.VerifyDiagnostics(
                // (6,9): warning CS8602: Possible dereference of a null reference.
                //         s /*T:string?*/ .ToString(); // 1
                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(6, 9),
                // (9,9): warning CS8602: Possible dereference of a null reference.
                //         s2 /*T:string?*/ .ToString(); // 2
                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s2").WithLocation(9, 9)
                );
            CompileAndVerify(comp);
        }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants