List patterns: binding and lowering

[alrz]

Test plan: #51289
Spec: https://github.com/dotnet/csharplang/blob/main/proposals/list-patterns.md

[jcouv]

FYI, I opened a bug for missing void check on Slice method for Ranges (#53754)

---

In reply to: 850578035

[jcouv]

Done with review pass (iteration 41)

[alrz]

With this change, there's an observable difference between .. and .._. In either case Slice is not emitted but we require a non-negative length for the latter. This is the same as doing {_} or (_, _) with ITuple, any discard added affects the required length but the indexer call is not emitted for either of subpatterns.

Thoughts? #Resolved

[jcouv]

I think that's fine

[alrz]

@333fred may have a different opinion. We could add null or Discard above to have both behave the same but at the moment we never treat discards as if they don't exist. Leaving this up to you. Thanks.

[333fred]

I also think that's fine.

[jcouv]

What about a slice with no subpattern in the middle or tail of a list pattern? Do we need to skip emitting a test for those too?
Example: { 1, .., 2 }, { 1, .. } #Closed

[alrz]

That does not require a Slice and therefore nothing to emit.
As for the length, it's already handled below as usual.

[jcouv]

Done with review pass (iteration 42).
FYI Monday is a holiday so there may be delay for second review.

[jcouv]

LGTM Thanks (iteration 42)

[jcouv]

I think there's still a problem with use-site errors. See tests below. Can be PROTOTYPE'd.

        [Fact]
        public void ListPattern_UseSiteErrorOnIndexerAndSlice()
        {
            var missing_cs = @"
public class Missing
{
}
";
            var missingRef = CreateCompilation(missing_cs, assemblyName: "missing")
                .EmitToImageReference();

            var lib2_cs = @"
public class C
{
    public int Length => 0;
    public Missing this[int i] => throw null;
    public Missing Slice(int i, int j) => throw null;
}
";
            var lib2Ref = CreateCompilation(lib2_cs, references: new[] { missingRef })
                .EmitToImageReference();

            var source = @"
class D
{
    void M(C c)
    {
        _ = c is { var item };
        _ = c is { ..var rest };
        var index = c[^1];
        var range = c[1..^1];
    }
}
";
            var compilation = CreateCompilation(new[] { source, TestSources.Index, TestSources.Range },
                references: new[] { lib2Ref }, parseOptions: TestOptions.RegularWithListPatterns);
            // PROTOTYPE missing diagnostics
            compilation.VerifyEmitDiagnostics(
                // (8,21): error CS0012: The type 'Missing' is defined in an assembly that is not referenced. You must add a reference to assembly 'missing, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
                //         var index = c[^1];
                Diagnostic(ErrorCode.ERR_NoTypeDef, "c[^1]").WithArguments("Missing", "missing, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null").WithLocation(8, 21),
                // (9,21): error CS0012: The type 'Missing' is defined in an assembly that is not referenced. You must add a reference to assembly 'missing, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
                //         var range = c[1..^1];
                Diagnostic(ErrorCode.ERR_NoTypeDef, "c[1..^1]").WithArguments("Missing", "missing, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null").WithLocation(9, 21)
                );


            var tree = compilation.SyntaxTrees.First();
            var model = compilation.GetSemanticModel(tree, ignoreAccessibility: false);
            var declarations = tree.GetRoot().DescendantNodes().OfType<VarPatternSyntax>().ToArray();

            verify(declarations[0], "item", "Missing?");
            verify(declarations[1], "rest", "Missing?");

            void verify(VarPatternSyntax declaration, string name, string expectedType)
            {
                var local = (ILocalSymbol)model.GetDeclaredSymbol(declaration.Designation)!;
                Assert.Equal(name, local.Name);
                Assert.Equal(expectedType, local.Type.ToTestDisplayString(includeNonNullable: true));
            }
        }
        
                [Fact]
        public void ListPattern_UseSiteErrorOnIndexAndRangeIndexers_WithFallback()
        {
            var missing_cs = @"
public class Missing
{
}
";
            var missingRef = CreateCompilation(missing_cs, assemblyName: "missing")
                .EmitToImageReference();

            var lib2_cs = @"
using System;
public class C
{
    public int Length => 0;
    public Missing this[Index i] => throw null;
    public Missing this[Range r] => throw null;
    public int this[int i] => throw null;
    public int Slice(int i, int j) => throw null;
}
";
            var lib2Ref = CreateCompilation(new[] { lib2_cs, TestSources.Index, TestSources.Range }, references: new[] { missingRef })
                .EmitToImageReference();

            var source = @"
class D
{
    void M(C c)
    {
        _ = c is { var item };
        _ = c is { ..var rest };
        var index = c[^1];
        var range = c[1..^1];
    }
}
";
            var compilation = CreateCompilation(source, references: new[] { lib2Ref }, parseOptions: TestOptions.RegularWithListPatterns);
            // PROTOTYPE binding for list and slice patterns should fail too
            compilation.VerifyEmitDiagnostics(
                // (8,21): error CS0012: The type 'Missing' is defined in an assembly that is not referenced. You must add a reference to assembly 'missing, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
                //         var index = c[^1];
                Diagnostic(ErrorCode.ERR_NoTypeDef, "c[^1]").WithArguments("Missing", "missing, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null").WithLocation(8, 21),
                // (9,21): error CS0012: The type 'Missing' is defined in an assembly that is not referenced. You must add a reference to assembly 'missing, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
                //         var range = c[1..^1];
                Diagnostic(ErrorCode.ERR_NoTypeDef, "c[1..^1]").WithArguments("Missing", "missing, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null").WithLocation(9, 21)
                );
        }

[alrz]

Looks like the BindIndexerOrIndexedPropertyAccess I'm using also fallback to TryBindIndexOrRange, but is BoundAccess fails and we throw away any diagnostics.

For indexer access it's working almost accidentally - we report the error while TryBindIndexOrRange succeeds. As a result, we've bound the fallback (just like list patterns) but reporting errors from the previous lookup (which is thrown away for list patterns)

Maybe we should depend on BindIndexerOrIndexedPropertyAccess for the fallback lookup and extract the symbol from BoundIndexOrRangePatternIndexerAccess?

I'll leave this as a prototype comment so you can decide what is the correct fix here. Thanks.

[alrz]

Will be addressed by #53784

A quickfix would be to capture diagnostics when we have a BoundIndexOrRangeIndexer, but we might as well just use it to avoid re-binding.

[alrz]

@333fred for review. thanks.

note: we may want to update the feature branch before proceeding with the follow-up.

[333fred]

useSiteInfo

Consider adding an assert about whether diagnostics and dependencies are being accumulated, or we'll eventually have an issue when a refactor is performed.

[alrz]

We've been passing Discard here for ranges too, I followed suit. I can add a prototype comment to revise.

[333fred]

Why are we not visiting the nested patterns? #Resolved

[jcouv]

They indeed need to be visited. This will be addressed in a later PR (here's the relevant commit I'm working on): 933dfd8
With the current PR, multiple nullability scenarios will hit assertions and/or produce incorrect nullability warnings.

[333fred]

Done review pass (commit 42)

[333fred]

LGTM (commit 42), assuming the relevant prototype comments are added for followup.

[alrz]

I forgot to push. comments are added now.

[jcouv]

Merged/squashed. Thanks @alrz!
Please rebase subsequence PRs and ping when ready for a look.