locked
VB.NET yield in async CTP has a potential bug when stepping backwards in a for loop?

    Question

  • I'm trying to work with the new release of the Visual Studio Async CTP release, and I'm running across what I believe to be a bug.  Take the following code:

     

    Private Iterator Function MyCounter(firstVal As Integer, lastVal As Integer) As IEnumerable(Of Integer)
    	' This fails to yield results when stepping backwards
    	Dim stepVal = If(firstVal < lastVal, 1, -1)
    	For i As Integer = firstVal To lastVal Step stepVal
    		Yield i
    	Next
    
    	' This works fine if I just hardcode the values, so what's wrong...
    	'For i As Integer = 10 To 1 Step -1
    	'	Yield i
    	'Next
    End Function
    

     

    Then, I test the code like so:

    ' Count up
    For Each value As Integer In MyCounter(1, 10)
     Console.WriteLine(value.ToString())
    Next
    
    ' Blast off!!! Fails...
    For Each value As Integer In MyCounter(10, 1)
     Console.WriteLine(value.ToString())
    Next
    
    

     

    It looks like my iterator method works fine while counting upwards, but fails while counting backwards.  Is this a bug in the VB async framework?  Or is a bug in my test or a misunderstanding on my part of how this should be working?


    --- EDIT ---

    After decompiling the resulting assembly using reflector, it's obvious that this is in fact a bug in the way the VB compiler is handling the Yield keyword in the async CTP.  The bug isn't just with stepping backwards, but is in fact a bug with any step value at all other than 1.  See below:

    <CompilerGenerated> _
    Friend Overrides Function MoveNext() As Boolean Implements IEnumerator.MoveNext
      Dim $doFinallyBodies As Boolean = True
      Try 
        If (Me.$State = 1) Then
          goto Label_0097
        End If
        If (Me.$State = -1) Then
          Return False
        End If
        If Me.$Disposing Then
          Return False
        End If
        Me.stepVal = IIf((Me.lastVal < Me.firstVal), -1, 1)
        $doFinallyBodies = CBool(Me.stepVal)
        Me.VB$t_i4$L0 = Me.lastVal
        Me.i = Me.firstVal
        Do While ((($doFinallyBodies >> &H1F) Xor Me.i) <= (($doFinallyBodies >> &H1F) Xor Me.VB$t_i4$L0))
          Me.$Current = Me.i
          Me.$State = 1
          $doFinallyBodies = False
          Return True
        Label_0097:
          If Me.$Disposing Then
            Return False
          End If
          Me.$State = 0
          Me.i = (Me.i + $doFinallyBodies)
        Loop
      Catch exception1 As Exception
        ProjectData.SetProjectError(exception1)
        Dim $ex As Exception = exception1
        Me.$State = -1
        Throw
        ProjectData.ClearProjectError
      End Try
      Me.$State = -1
      Return False
    End Function
    
    The offending line is:

    $doFinallyBodies = CBool(Me.stepVal)
    

    This essentially converts any non-zero step value provided into the integer 1.  A normal for/step loop in VB decompiles to this:

    ' AS WRITTEN
    Console.WriteLine("Step loop...")
    Dim firstVal = 10
    Dim lastVal = 1
    Dim stepVal = If(lastVal < firstVal, -2, 2)
    For i As Integer = firstVal To lastVal Step stepVal
      Console.WriteLine(i.ToString())
    Next
    
    ' DECOMPILED version
     Console.WriteLine("Step loop...")
     Dim firstVal As Integer = 10
     Dim lastVal As Integer = 1
     Dim stepVal As Integer = IIf((lastVal < firstVal), -2, 2)
     Dim VB$t_i4$L1 As Integer = stepVal
     Dim VB$t_i4$L0 As Integer = lastVal
     Dim i As Integer = firstVal
     Do While (((VB$t_i4$L1 >> &H1F) Xor i) <= ((VB$t_i4$L1 >> &H1F) Xor VB$t_i4$L0))
        Console.WriteLine(i.ToString)
        i = (i + VB$t_i4$L1)
    Loop
    

    Notice that "VB$t_i4$L1" is a legitimate integer instead of a CBool()ed value.

     

    Is there a more appropriate place to report these findings, because if this product ships like this there are going to be a lot of really mad VB.NET developers.

    • Edited by MattMc3 Tuesday, April 26, 2011 11:14 AM more info
    Monday, April 25, 2011 2:08 PM

Answers

  • Matt,

    Thanks for your clear bug-report and detailed analysis. These forums are a fine place to report these findings since we read them regularly. Alternatively you can email me directly (lwischik@microsoft.com). Or you can file Connect bugs.

    Good news: this is a CTP-only bug which has already been fixed in our proper codebase.

    Bad news: The only workaround I can think for the CTP is, if you have a For loop with a non-constant Step, then rewrite it into a While loop.

     

    Analysis: I described some of the bugs we'd fixed here: http://blogs.msdn.com/b/lucian/archive/2011/04/16/async-ctp-refresh-compiler-bug-fixes.aspx

    In particular, the section "Fixed bug: a VB compiler bug with late-bound For loops in async/iterators" mentions that a lot of our bugs were due to failing to lift temporary variables into the generated state machine class. In this case the CTP is failing to lift the "step temporary variable" into a field in the state machine class: the compiler still thinks that the step temporary is stored in Local Variable Slot Number 1. But, as Reflector shows, it's not: that spot is actually occupied by doFinallyBodies.

    Our proper codebase does seem to lift all temporary variables correctly. I'd failed to backport this particular temp-variable-lifting to the CTP.

    I'll add a note to this on my blog page about known CTP bugs.

     

    Thanks again for the bug report. These kinds of codegen bugs, as you say, are really serious.

    --
    Lucian

    Tuesday, April 26, 2011 9:56 PM
    Owner

All replies

  • Matt,

    Thanks for your clear bug-report and detailed analysis. These forums are a fine place to report these findings since we read them regularly. Alternatively you can email me directly (lwischik@microsoft.com). Or you can file Connect bugs.

    Good news: this is a CTP-only bug which has already been fixed in our proper codebase.

    Bad news: The only workaround I can think for the CTP is, if you have a For loop with a non-constant Step, then rewrite it into a While loop.

     

    Analysis: I described some of the bugs we'd fixed here: http://blogs.msdn.com/b/lucian/archive/2011/04/16/async-ctp-refresh-compiler-bug-fixes.aspx

    In particular, the section "Fixed bug: a VB compiler bug with late-bound For loops in async/iterators" mentions that a lot of our bugs were due to failing to lift temporary variables into the generated state machine class. In this case the CTP is failing to lift the "step temporary variable" into a field in the state machine class: the compiler still thinks that the step temporary is stored in Local Variable Slot Number 1. But, as Reflector shows, it's not: that spot is actually occupied by doFinallyBodies.

    Our proper codebase does seem to lift all temporary variables correctly. I'd failed to backport this particular temp-variable-lifting to the CTP.

    I'll add a note to this on my blog page about known CTP bugs.

     

    Thanks again for the bug report. These kinds of codegen bugs, as you say, are really serious.

    --
    Lucian

    Tuesday, April 26, 2011 9:56 PM
    Owner
  • Fantastic Lucian, thank you.  It's certainly understandable that a CTP would have some rough edges here and there, but I'm glad to hear this will be addressed in the final RTM.  I'm sure many VB devs are like me and chomping at the bit to finally have the Yield keyword.  Great work!
    Tuesday, April 26, 2011 10:26 PM
  • Check out this:

    Dim xml = <ul>
               <%= Iterator Function()
                         For i = 1 To 10
                           Yield <li><%= i %> is a number</li>
                         Next
                        End Function()
                %>
                </ul>

    XML+IteratorLambda !!! And it's useful!

    Tuesday, April 26, 2011 10:32 PM
    Owner