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

Open issue: list-pattern questions for 1/3/2022 #5599

Closed
jcouv opened this issue Jan 3, 2022 · 2 comments
Closed

Open issue: list-pattern questions for 1/3/2022 #5599

jcouv opened this issue Jan 3, 2022 · 2 comments

Comments

@jcouv
Copy link
Member

jcouv commented Jan 3, 2022

Previous discussion: https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-10-20.md#slices-that-return-null

If a Slice method is annotated, we will treat it as potentially returning null for the purposes of subsumption and exhaustiveness.

Example of scenario with annotations (not so bad, since only affects a warning):

#nullable enable

public class C 
{
    public int Length => 0;
    public int this[int i] => 0;
    public C? Slice(int i, int j) => null;
    public void M() 
    {
        _ = this switch
        {
          null => 0,
          [..not null] => 1,
          // warning CS8655: The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '[.. null]' is not covered.
        };
    }
}

But the weirdness is not limited to nullable-annotated Slice methods. It happens whenever Slice returns a reference type:

System.Span<int> s = default;
switch (s) 
{
    case [..[1],2,3]:
    case [1,2,3]: // error (The switch case is unreachable. It has already been handled by a previous case or it is impossible to match.)
        break;
}
 
int[] a = default;
switch (a) 
{
    case [..[1],2,3]:
    case [1,2,3]: // no error
        break;
}

@alrz proposed to not have a null-test during the slice operation. Yes, this could result in unexpected behavior (null-ref exception, which patterns typically avoid), but that's the case for all other assumptions we've been baking into list-patterns.

After email thread, we have 3 options:

  1. status quo (emit null-test and keep above difference in diagnostics)
  2. remove the null-test (and thus error in both scenarios above)
  3. perform reachability analysis without null-test (same diagnostics as option 2) but include null-test in codegen (same runtime behavior as option 1)
  4. perform reachability analysis and codegen with a "Slice and throw if null" operation

FYI @AlekseyTs @333fred @CyrusNajmabadi

Relates to list-patterns (proposal)

@alrz
Copy link
Member

alrz commented Jan 3, 2022

I'd add a fourth option "removing assumptions on the slice value entirely" because this is a very unlikely scenario in real code and a solution that could also keep the nullability analysis intact while emitting the null check (not mentioned in the OP) doesn't seem to worth it to go through. That would result in no error for both cases.

@jcouv
Copy link
Member Author

jcouv commented Jan 3, 2022

LDM 1/3/2022: landed on option 2 (remove the null-check)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants