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.

Answer 1

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 ([e-mail]). 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

Answer 2

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!

Answer 3

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!