Slice pattern with type pattern generates redundant type test (or fails to perform the test intended)

[idg10]

Version Used: C# 11.0, .NET SDK 7.0.302.

Steps to Reproduce:

Compile this code:

object[] list = new string[]{ "alpha", "papa", "omega" };
if (list is ["alpha", .. string[] theRest, "omega"])
{
    Console.WriteLine($"Match! {theRest.Length}");
}

Inspect the generated IL. The part that evaluates the slice pattern includes this code:

call   !!0[] [System.Runtime]System.Runtime.CompilerServices.RuntimeHelpers::GetSubArray<object>(!!0[],
            valuetype [System.Runtime]System.Range)
isinst  string[]

I believe that isinst instruction is aiming to deal with the fact that although the static type of list is object[], the type compatibility rules for arrays mean it might actually be a string[] (and in this example, it is). The thinking then appears to be that although that string[] theRest doesn't match the static type of list, it might be a runtime match.

So this calls GetSubArray, and then asks whether the array that came back is really a string[].

Unfortunately, this serves no purpose. GetSubArray<object> always returns an object[] even when, as in this case, the input was actually a string[]. (To put it another way, the compiler seems to assume that GetSubArray will preserve the array element type. But that's wrong. In fact GetSubArray returns an array where the element type is determined by the type argument.)

So that isinst string[] will always fail.

So at best, that test is redundant. At worst, this is a bug: if the intention here is that the pattern should succeed when the runtime type of list is string[], this code fails to achieve that. That pattern only matches the array in this example if we change list's declared type to string[]. (Since in general, slice patterns reject impossible matches at compile time, it seems more likely that this is indeed a bug, and that this pattern was meant to match in this example.)

Expected Behavior: well this depends on what the compiler is trying to achieve. If the actual behaviour of this code (the pattern does not match) is correct, then the compiler should not emit that isinst string[] test, because it will always fail. (And perhaps the compiler should be reporting an error, because it does that in most cases where a slice pattern is an impossible match.) But if that pattern is in fact meant to match in this scenario, then the compiler is emitting code that fails to achieve this.

Actual Behavior: the compiler emits a type check that always fails.

[jcouv]

I agree. The GetSubArray call returns a string[] so we should be able to omit the string[] type test on the result. Seems like we do it in other patterns (tried object o = null; _ = o is object o2;).
FYI @alrz

[alrz]

The GetSubArray call returns a string[]

Turns out that's no longer the case: dotnet/runtime#66011 the pattern here relies on the previous implementation.

Compiler doesn't have any special knowledge on GetSubArray other than it is assumed that it won't return null. Other than that, I think the behavior is expected; string[] is matched against an object[] input so there's a type check. It could however produce an error instead based on this knowledge as OP suggests.

[jcouv]

Oh, I'd missed that the local is declared as object[]. So we're calling GetSubArray<object> which returns object[]... My bad for triaging late at night.

The behavior is the same as for existing patterns:

object[] o = new string[] { };
_ = o is string[] o2; // no error, emits `isinst string[]`

[CyrusNajmabadi]

@jcouv yes. but in this case, because we create the object[] (due to the .. in teh pattern) we know things will never succeed. It's subtly different from our views on things when users declare other variables :)

[DoctorKrolic]

@jcouv Since list patterns are gonna be discussed on an LDM, I think this issue worth attention there as well. Namely the question here are:

1. Should this case emit an error or at least a warning? Technically, variable of type object[] can store string[], but since array slicing uses a GetSubArray method with a well-known behavior, we known for sure the pattern can never match. Considering that other impossible patterns (that rely on language rules without assumptions about specific methods behavior though) emit an error wouldn't it be better to be consistent and block this case from compiling as well?
2. If it is decided that this case should still be compilable (for instance, emit a warning but still allow to build such code), should compiler evaluate slicing pattern first, so in runtime the check can fail fast and avoid spending time on evaluating other patterns in the list?

[jcouv]

Discussed in LDM 2023-11-27.
@jkotas It looks like this is an unintended side-effect/regression of dotnet/runtime#66011 (which stated "it is very unlikely for any code out there to depend on this co-variant arrays corner case.").
LDM's preference would be to keep the previous apparent "virtual" behavior of slicing an array, if possible.
Tagging @stephentoub @MadsTorgersen

[jkotas]

LDM's preference would be to keep the previous apparent "virtual" behavior of slicing an array, if possible.

PR that reverts to the previous behavior dotnet/runtime#95296

There is a question that needs to be answered. Should the actual type of array slice always match the type of the source array? (It is not the case even for the previous behavior.)

[jkotas]

Fixed by dotnet/runtime#95296

[333fred]

Discussed in https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-11-27.md#subarray-slicing-breaking-change

[jcouv]

Closing as fixed. Thanks @jkotas