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.06k stars 4.04k forks source link

Exhaustiveness check quacked on nested deconstruction #55719

Open WhiteBlackGoose opened 3 years ago

WhiteBlackGoose commented 3 years ago

Version Used:

Mine is 6.0.100-preview.6.21355.2, not sure which one is on sharplab, but it's also reproducable there.

Steps to Reproduce:

Create a record of two props with a property being this record. Switch over null and over (var ..., (var ..., tail):

#nullable enable

LinkedList<(T, T)>? GroupBy2<T>(LinkedList<T>? list)
    => list switch    // No warning
    {
        null => null,
        (var h1, (var h2, var tail)) => new((h1, h2), GroupBy2(tail))
        // no warning but try to pass new LinkedList<int>(5, null) and get a SwitchExpressionException
    };

public sealed record LinkedList<T>(T Head, LinkedList<T>? Tail);

Expected Behavior:

Warning, because (_, null) case not covered

Actual Behavior:

No warning.

Youssef1313 commented 3 years ago

Few observations:

The warning is produced if one the following changes are made:

alrz commented 3 years ago

Possible duplicate of https://github.com/dotnet/roslyn/issues/52714

333fred commented 3 years ago

Definitely a duplicate.

Youssef1313 commented 3 years ago

@333fred The behavior I'm observing (stated in https://github.com/dotnet/roslyn/issues/55719#issuecomment-901691189) doesn't seem to apply on the other issue. Is the reason known? If not, I think it's worth keeping this one open and see if a fix for #52714 also fixes this one or not.

333fred commented 3 years ago

Ah, you're right Youssef, this is not a duplicate.

RikkiGibson commented 3 years ago

We do expect a warning here because in this scenario the nullable context is enabled. I wouldn't particularly expect a fix for this to change the behavior in #52714.