Normal "while" statement change by comp/dec to "using Enumerator" without inheriting from IDisposable (entity framework)

[Kein]

public void AllDecendantsList(Transform transform)
{
    List<Transform> returnValue = new List<Transform>();;
    int CurrentTransformIndex = returnValue.Count;
    foreach (Transform t in transform)
    {
        if (condition)
        {
            //stuff
            this.myDict.Add(t.gameObject, value);
        }
        returnValue.Add(t);
    }
    while (CurrentTransformIndex < returnValue.Count)
    {
        Transform currentTransform= returnValue[CurrentTransformIndex++];
        if (currentTransform.childCount > 0)
        {
            int childCounter = 0;
            foreach (Transform t2 in currentTransform)
            {
                if (condition)
                {
                    //stuff
                    this.myDict.Add(t2.gameObject, value);
                }
                returnValue.Add(t2);
                childCounter++;
            }

        }
    }
}

turned into

public void AllDecendantsList(Transform transform)
    {
        List<Transform> returnValue = new List<Transform>();
        int CurrentTransformIndex = returnValue.Count;
        using (IEnumerator enumerator = transform.GetEnumerator())
        {
            while (enumerator.MoveNext())
            {
                object obj = enumerator.Current;
                Transform t = (Transform)obj;
                if (condition1)
                {
                    //stuff
                    this.myDict.Add(t.gameObject, value);
                }
                returnValue.Add(t);
            }
            goto IL_1C0;
        }
        IL_D2:
        Transform currentTransform = returnValue[CurrentTransformIndex++];
        if (currentTransform.childCount > 0)
        {
            int childCounter = 0;
            foreach (object obj2 in currentTransform)
            {
                Transform t2 = (Transform)obj2;
                if (condition1)
                {
                    //stuff
                    this.myDict.Add(t2.gameObject, value);
                }
                returnValue.Add(t2);
                childCounter++;
            }
        }
        IL_1C0:
        if (CurrentTransformIndex >= returnValue.Count)
        {
            return;
        }
        goto IL_D2;
    }

It works afterwards, however, any attempt to recompile "auto-optimized" (oh god) method it will result in:

To reproduce, use same assembly, any method/class https://github.com/0xd4d/dnSpy/issues/469#issuecomment-317031307

[airbreather]

Can confirm (edit: though, it does not seem to have anything to do with the details in the title, so I'd like to request that the title be changed). Here's a nice, slimmed-down full repro:

class A
{
    void X(System.Collections.IEnumerable e)
    {
        foreach (var s in e)
        {
            return;
        }
    }
}

Punch that into a class library project, compile in release mode (compiled with VS2017 15.3.0 Preview 5.0). ILSpy 2.4.0.1963 decompiles the result to:

using System;
using System.Collections;

internal class A
{
    private void X(IEnumerable e)
    {
        using (IEnumerator enumerator = e.GetEnumerator())
        {
            if (enumerator.MoveNext())
            {
                object arg_0F_0 = enumerator.Current;
            }
        }
    }
}

Which is indeed incorrect, because System.Collections.IEnumerator indeed does not extend System.IDisposable (only System.Collections.Generic.IEnumerator<T> does)... odd that if you just take out the return; line, it gets recognized as a regular ol' foreach loop.

The full CIL of the method from ILDASM, in case this is actually a case where a newer compiler has started emitting a different pattern and you can't immediately get your hands on such a compiler:

.method private hidebysig instance void  X(class [mscorlib]System.Collections.IEnumerable e) cil managed
{
  // Code size       46 (0x2e)
  .maxstack  1
  .locals init ([0] class [mscorlib]System.Collections.IEnumerator V_0,
           [1] class [mscorlib]System.IDisposable V_1)
  IL_0000:  ldarg.1
  IL_0001:  callvirt   instance class [mscorlib]System.Collections.IEnumerator [mscorlib]System.Collections.IEnumerable::GetEnumerator()
  IL_0006:  stloc.0
  .try
  {
    IL_0007:  br.s       IL_0012
    IL_0009:  ldloc.0
    IL_000a:  callvirt   instance object [mscorlib]System.Collections.IEnumerator::get_Current()
    IL_000f:  pop
    IL_0010:  leave.s    IL_002d
    IL_0012:  ldloc.0
    IL_0013:  callvirt   instance bool [mscorlib]System.Collections.IEnumerator::MoveNext()
    IL_0018:  brtrue.s   IL_0009
    IL_001a:  leave.s    IL_002d
  }  // end .try
  finally
  {
    IL_001c:  ldloc.0
    IL_001d:  isinst     [mscorlib]System.IDisposable
    IL_0022:  stloc.1
    IL_0023:  ldloc.1
    IL_0024:  brfalse.s  IL_002c
    IL_0026:  ldloc.1
    IL_0027:  callvirt   instance void [mscorlib]System.IDisposable::Dispose()
    IL_002c:  endfinally
  }  // end handler
  IL_002d:  ret
} // end of method A::X

[siegfriedpammer]

This was fixed in the new decompiler engine (which just landed on the master branch). Please note that we do not restore the foreach loop, but keep the try-finally-Dispose() structure and a nested if statement.

[Kein]

Please note that we do not restore the foreach loop, but keep the try-finally-Dispose() structure and a nested if statement.

Can you elaborate on this? You mean you do not restore already altered code or you mean you still process foreach in nested IFs as IDisposale enumerator while loop? What exactly was fixed then in the 2nd case I wonder?

[dgrunwald]

The new decompiler produces this code:

    private void X(IEnumerable e)
    {
        IEnumerator enumerator = e.GetEnumerator();
        try
        {
            if (enumerator.MoveNext())
            {
                object _ = enumerator.Current;
            }
        }
        finally
        {
            IDisposable disposable = enumerator as IDisposable;
            if (disposable != null)
            {
                disposable.Dispose();
            }
        }
    }

This is correct (compiles and has the same semantics), even if it's not as short as the original.

ILSpy currently can only detect foreach if there's a loop in the IL code. I don't think it's worth the effort to add a special case for this code construct.

[Kein]

Oh good lord