dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.02k stars 4.03k forks source link

Tuple element nullability state doesn't flow after null check #39831

Open alrz opened 4 years ago

alrz commented 4 years ago

Version Used: master (https://github.com/dotnet/roslyn/commit/89b4f60648fadeebdd53088fe8391bf9197bba7b)

Steps to Reproduce:

using System;
using System.Collections.Generic;
#nullable enable
public static class Program {
    public static void M(IEnumerable<(object?, object? Nullable)> items) {
        var list = new List<(object?, object NonNullable)>();
        foreach(var item in items) {
            if (item.Nullable is null)
                return;
            list.Add(item); // warns (none expected)
        }
    }
}

Expected Behavior: no warning at list.Add(item)

Actual Behavior: warns about mismatched nullability

alrz commented 4 years ago

Actually, the state does flow, but it won't affect the type so list.Add((item.Item1, item.Nullable)) wouldn't warn. Since tuple elements have a corresponding type arg, the compiler could deduce a new type every time an element is null checked.

jaredpar commented 4 years ago

@jcouv this seems to be by design to me. The compiler is correctly inferring the state change in the Nullable field of the tuple here however it's keeping the overall type of the tuple the same. That matches my intuition here and our general behavior around tuples

        (object?, object? Nullable) local = default;
        local.Nullable = new object();
        list.Add(local); // warns 
        local.Nullable.ToString(); // no warn

Do you agree here?

jcouv commented 4 years ago

Yes, I agree, this follows the current design. There were two behaviors we discussed:

We currently use the first approach. But some parts of the language use the second approach (method type inference, conversions). I think Mads was interested in reviewing this in LDM again though.

jaredpar commented 4 years ago

Removing Area-Compilers as this is a language design issue at this point.