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

Bad sequence point in switch with pattern matching in async method #28288

Closed
jcouv opened this issue Jul 5, 2018 · 7 comments
Closed

Bad sequence point in switch with pattern matching in async method #28288

jcouv opened this issue Jul 5, 2018 · 7 comments

Comments

@jcouv
Copy link
Member

jcouv commented Jul 5, 2018

Set a breakpoint in GenerateDeconstructMethodCodeFixProvider.RegisterCodeFixesAsync and start debugging any of the tests for this fixer (such as TestDeconstructionDeclaration_Simple).
The stepping shows the return statement as executed, when it is not actually executed.

switch-seq-point

Unfortunately, I was not able to narrow down a small repro yet (the ones I tried behaved as expected).

This is using VisualStudio.15.Preview/15.8.0-pre.2.0+27729.1 on PR #28286

@jcouv
Copy link
Member Author

jcouv commented Jul 5, 2018

This reminds me of a similar issue (not related to switch but possibly to pattern matching): #17076

Tagging @gafter for awareness

@jcouv jcouv changed the title Bad sequence point in switch statement Bad sequence point in switch with pattern matching Jul 5, 2018
@gafter
Copy link
Member

gafter commented Jul 5, 2018

I was able to repro it using the following code:

using System.Threading.Tasks;

public class Program
{
    public static async Task Main()
    {
        switch (M())
        {
            case C c:
                break;
            default:
                return;
        }

        if (M() != null)
        {
        }
    }

    private static object M()
    {
        return new C();
    }
}

class C
{
}

@jcouv
Copy link
Member Author

jcouv commented Jul 5, 2018

Awesome. Thanks a bunch!

@jcouv jcouv self-assigned this Jul 5, 2018
@jcouv jcouv added this to the 15.8 milestone Jul 5, 2018
@jcouv jcouv changed the title Bad sequence point in switch with pattern matching Bad sequence point in switch with pattern matching in async method Jul 6, 2018
@jcouv
Copy link
Member Author

jcouv commented Jul 6, 2018

The sequence points seem correct.
Tagging @tmat for some advice.

        [Fact, WorkItem(28288, "https://github.com/dotnet/roslyn/issues/28288")]
        void Test()
        {
            var source = @"

public class Program
{
    public static async System.Threading.Tasks.Task Main()
    {
        switch (new Program())
        {
            case Program p:
                M(1);
                break;
            default:
                M(2);
                return;
        }

        await System.Threading.Tasks.Task.CompletedTask;
    }
    private static void M(int i) { }
}";
            var verifier = CompileAndVerify(source);
            verifier.VerifyIL("Program.<Main>d__0.System.Runtime.CompilerServices.IAsyncStateMachine.MoveNext()", @"{
  // Code size      169 (0xa9)
  .maxstack  3
  .locals init (int V_0,
                Program V_1,
                System.Runtime.CompilerServices.TaskAwaiter V_2,
                System.Exception V_3)
  // sequence point: }, sequence point: <hidden>
  IL_0000:  ldarg.0
  IL_0001:  ldfld      ""int Program.<Main>d__0.<>1__state""
  IL_0006:  stloc.0
  .try
  {
    // sequence point: <hidden>
    IL_0007:  ldloc.0
    IL_0008:  brfalse.s  IL_0059
    // sequence point: switch (new Program())
    IL_000a:  newobj     ""Program..ctor()""
    IL_000f:  stloc.1
    IL_0010:  ldloc.1
    IL_0011:  brfalse.s  IL_001d
    IL_0013:  ldloc.1
    IL_0014:  stloc.1
    // sequence point: M(1);
    IL_0015:  ldc.i4.1
    IL_0016:  call       ""void Program.M(int)""
    // sequence point: break;
    IL_001b:  br.s       IL_0025
    // sequence point: M(2);
    IL_001d:  ldc.i4.2
    IL_001e:  call       ""void Program.M(int)""
    // sequence point: return;
    IL_0023:  leave.s    IL_0095
    // sequence point: await System.Threading.Tasks.Task.CompletedTask;
    IL_0025:  call       ""System.Threading.Tasks.Task System.Threading.Tasks.Task.CompletedTask.get""
    IL_002a:  callvirt   ""System.Runtime.CompilerServices.TaskAwaiter System.Threading.Tasks.Task.GetAwaiter()""
    IL_002f:  stloc.2
    // sequence point: <hidden>
    IL_0030:  ldloca.s   V_2
    IL_0032:  call       ""bool System.Runtime.CompilerServices.TaskAwaiter.IsCompleted.get""
    IL_0037:  brtrue.s   IL_0075
    IL_0039:  ldarg.0
    IL_003a:  ldc.i4.0
    IL_003b:  dup
    IL_003c:  stloc.0
    IL_003d:  stfld      ""int Program.<Main>d__0.<>1__state""
    // async: yield
    IL_0042:  ldarg.0
    IL_0043:  ldloc.2
    IL_0044:  stfld      ""System.Runtime.CompilerServices.TaskAwaiter Program.<Main>d__0.<>u__1""
    IL_0049:  ldarg.0
    IL_004a:  ldflda     ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<Main>d__0.<>t__builder""
    IL_004f:  ldloca.s   V_2
    IL_0051:  ldarg.0
    IL_0052:  call       ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.AwaitUnsafeOnCompleted<System.Runtime.CompilerServices.TaskAwaiter, Program.<Main>d__0>(ref System.Runtime.CompilerServices.TaskAwaiter, ref Program.<Main>d__0)""
    IL_0057:  leave.s    IL_00a8
    // async: resume
    IL_0059:  ldarg.0
    IL_005a:  ldfld      ""System.Runtime.CompilerServices.TaskAwaiter Program.<Main>d__0.<>u__1""
    IL_005f:  stloc.2
    IL_0060:  ldarg.0
    IL_0061:  ldflda     ""System.Runtime.CompilerServices.TaskAwaiter Program.<Main>d__0.<>u__1""
    IL_0066:  initobj    ""System.Runtime.CompilerServices.TaskAwaiter""
    IL_006c:  ldarg.0
    IL_006d:  ldc.i4.m1
    IL_006e:  dup
    IL_006f:  stloc.0
    IL_0070:  stfld      ""int Program.<Main>d__0.<>1__state""
    IL_0075:  ldloca.s   V_2
    IL_0077:  call       ""void System.Runtime.CompilerServices.TaskAwaiter.GetResult()""
    IL_007c:  leave.s    IL_0095
  }
  catch System.Exception
  {
    // sequence point: <hidden>
    IL_007e:  stloc.3
    IL_007f:  ldarg.0
    IL_0080:  ldc.i4.s   -2
    IL_0082:  stfld      ""int Program.<Main>d__0.<>1__state""
    IL_0087:  ldarg.0
    IL_0088:  ldflda     ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<Main>d__0.<>t__builder""
    IL_008d:  ldloc.3
    IL_008e:  call       ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)""
    IL_0093:  leave.s    IL_00a8
  }
  // sequence point: }
  IL_0095:  ldarg.0
  IL_0096:  ldc.i4.s   -2
  IL_0098:  stfld      ""int Program.<Main>d__0.<>1__state""
  // sequence point: <hidden>
  IL_009d:  ldarg.0
  IL_009e:  ldflda     ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<Main>d__0.<>t__builder""
  IL_00a3:  call       ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetResult()""
  IL_00a8:  ret
}", sequencePoints: "", source: source);
        }

seq-point

Note to self: seq-points branch

@jcouv jcouv modified the milestones: 15.8, 16.0 Jul 9, 2018
@jaredpar jaredpar added the Bug label Aug 30, 2018
@jcouv jcouv modified the milestones: 16.0, 16.0.P2 Nov 12, 2018
@jcouv
Copy link
Member Author

jcouv commented Dec 20, 2018

Another variant of this was spotted: #31665 (switch statement inside if/else)

@gafter gafter added this to IDE/EE/Debug issues in Compiler: Pattern-Matching Dec 20, 2018
@jcouv jcouv modified the milestones: 16.0.P2, 16.0.P3, 16.0 Dec 31, 2018
@jcouv jcouv modified the milestones: 16.0, 16.1, 16.2 Apr 18, 2019
@jcouv jcouv modified the milestones: 16.2, 16.3 Jun 19, 2019
@jcouv jcouv removed this from the 16.3 milestone Jul 1, 2019
@jcouv jcouv added this to the Compiler.Next milestone Jul 1, 2019
@jcouv jcouv modified the milestones: Compiler.Next, 16.4 Sep 26, 2019
@jcouv jcouv added this to Active/Investigating in Compiler: Julien's umbrellas Sep 26, 2019
@jcouv
Copy link
Member Author

jcouv commented Sep 26, 2019

I'm able to repro this even without await suspensions.
Tagging @chuckries @tmat for advice. Thanks

        [Fact, WorkItem(28288, "https://github.com/dotnet/roslyn/issues/28288")]
        public void BadSequencePoint_TODO2()
        {
            var source = @"
using System.Threading.Tasks;

public class C
{
    public static async Task Main()
    {
        object o = new C();
        switch (o)
        {
            case C c:
                System.Console.Write(1);
                break;
            default:
                return;
        }

        if (M() != null)
        {
        }
    }

    private static object M()
    {
        return new C();
    }
}";
            // The debugger highlights the `break` then the `return` (instead of the `if`)
            var v = CompileAndVerify(source);
            v.VerifyIL("C.<Main>d__0.System.Runtime.CompilerServices.IAsyncStateMachine.MoveNext()", @"
{
  // Code size       73 (0x49)
  .maxstack  2
  .locals init (System.Exception V_0)
  .try
  {
    // sequence point: object o = new C();
    IL_0000:  newobj     ""C..ctor()""
    // sequence point: <hidden>
    IL_0005:  isinst     ""C""
    IL_000a:  brfalse.s  IL_0014
    // sequence point: System.Console.Write(1);
    IL_000c:  ldc.i4.1
    IL_000d:  call       ""void System.Console.Write(int)""
    // sequence point: break;
    IL_0012:  br.s       IL_0016
    // sequence point: return;
    IL_0014:  leave.s    IL_0035
    // sequence point: if (M() != null)
    IL_0016:  call       ""object C.M()""
    IL_001b:  pop
    // sequence point: <hidden>
    IL_001c:  leave.s    IL_0035
  }
  catch System.Exception
  {
    // sequence point: <hidden>
    IL_001e:  stloc.0
    IL_001f:  ldarg.0
    IL_0020:  ldc.i4.s   -2
    IL_0022:  stfld      ""int C.<Main>d__0.<>1__state""
    IL_0027:  ldarg.0
    IL_0028:  ldflda     ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder C.<Main>d__0.<>t__builder""
    IL_002d:  ldloc.0
    IL_002e:  call       ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)""
    IL_0033:  leave.s    IL_0048
  }
  // sequence point: }
  IL_0035:  ldarg.0
  IL_0036:  ldc.i4.s   -2
  IL_0038:  stfld      ""int C.<Main>d__0.<>1__state""
  // sequence point: <hidden>
  IL_003d:  ldarg.0
  IL_003e:  ldflda     ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder C.<Main>d__0.<>t__builder""
  IL_0043:  call       ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetResult()""
  IL_0048:  ret
}
", sequencePoints: "C+<Main>d__0.MoveNext", source: source);
        }

@jcouv jcouv modified the milestones: 16.4, 16.5 Oct 29, 2019
@jcouv jcouv moved this from Active/Investigating to Misc in Compiler: Julien's umbrellas Dec 6, 2019
@jcouv jcouv modified the milestones: 16.5, 16.6 Jan 7, 2020
@gafter gafter moved this from IDE/EE/Debug issues to Backlog in Compiler: Pattern-Matching Mar 26, 2020
@gafter gafter moved this from Backlog to Pended (Expression Breakpoints) in Compiler: Pattern-Matching Mar 26, 2020
@gafter gafter moved this from Pended (Expression Breakpoints) to Active in Compiler: Pattern-Matching Mar 26, 2020
@gafter gafter assigned gafter and unassigned jcouv Mar 26, 2020
gafter pushed a commit to gafter/roslyn that referenced this issue Mar 27, 2020
@gafter gafter moved this from Active to In Review in Compiler: Pattern-Matching Mar 27, 2020
gafter pushed a commit that referenced this issue Mar 31, 2020
@gafter
Copy link
Member

gafter commented Mar 31, 2020

Fixed in #42824

@gafter gafter closed this as completed Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants