Allow foreach-ing over IEnumerator-like types

[fitdev]

Currently there is a very nice feature in C# that allows certain language patterns to be used on types that do not implement the typically required interfaces as long as those types have certain members defined on them like void Dispose() for IDisposable-like type and Enumerator<T> GetEnumerator() for IEnumerable<T>-like type.

This is very useful as sometimes it is not feasible to have types implement these interfaces, and yet there is a need to allow these types to be used in language statements like foreach.

I recently ran across a slight limitation with foreach though because of how it works:

foreach (var p in IEnumerableLikeStruct) DoSomething(p);

gets rewritten by the compiler to:

var IEnumerableLikeStructEnumerator = IEnumerableLikeStruct.GetEnumerator();
while (IEnumerableLikeStructEnumerator.MoveNext()) { DoSomething(IEnumerableLikeStructEnumerator.Current); }

And here lies the problem: there is no way to get to IEnumerableLikeStructEnumerator from the foreach - it is completely hidden from the user by the compiler.

In most cases this is not an issue, however for certain patterns it does become an issue, especially where performance is needed, and hence both the IEnumerable<T>-like type and IEnumerator<T>-like type have to be structs. If these types were class es it would not be a problem, because one could have a class that acts both as IEnumerable and IEnumerator:

class IEnumerableLikeEnumerator {
  public GetEnumerator() => this;
  public T Current { get; private set; }
  public bool MoveNext() { ... }
}

It works for classes because they are reference types, and so public GetEnumerator() => this; returns the same instance. However structs which have to be used for performance are value types and so public GetEnumerator() => this; will always create a new instance on the stack when the lowered foreach code calls IEnumerableLikeStruct.GetEnumerator(). This is always true regardless of whether the struct is readonly struct or a ref struct or both.

So what is the problem with having 2 distinct objects when foreaching? The problem is that we cannot access members (fields, properties in particular) on the "Enumerator" that gets returned by IEnumerableLikeStruct.GetEnumerator().

A concrete example:

I was trying to write a pixel enumerator that will produce pixels going stride at a time vertically and pixel at a time horizontally. And I really wanted to simply write: foreach (PixelPointer* p in MyPixelEnumerator) ..., and that worked fine as long as I did not need to access the enumerator. However I often need to know which X and Y the enumerator is currently returning me a pixel pointer for. So, as part of pixel enumerator implementation I expose these as properties:

public unsafe ref struct PixelEnumerator<TPixel> where TPixel : unmanaged {

  //Part of definition is omitted for brevity

  readonly int Right;
  readonly int Bottom;
  readonly int StartY;
  readonly int OffsetLengthPlusPixelLength;

  public int X { readonly get; private set; }
  public int Y { readonly get; private set; }  
  
  public readonly PixelEnumerator<TPixel> GetEnumerator() => this; //<-- This is wrong as it returns a new instance and so X and Y become inaccessible from the point of view of the caller

  public TPixel* Current { readonly get; private set; }
  
  public bool MoveNext() {
    if (++X < Right) {
      Current++;
      return true;
    } else if (++Y < Bottom) {
      X = StartX;
      Current = (TPixel*)((byte*)Current + OffsetLengthPlusPixelLength);
      return true;
    }
    return false;
  }
  
 }

So, unfortunately the above approach does not work, because the X and Y that get updated are updated on the invisible-to-the-caller instance and not on the MyPixelEnumerator instance within the foreach (var p in MyPixelEnumerator) statement.

Workarounds:

There are two but I do not like either:

1. Modify PixelEnumerator so that instead of returning a copy of itself in GetEnumerator(), it return a new specialized "hacky" nested type that will contain a pointer to the original PixelEnumerator, so that it can be properly modified by the actual enumerator:

public unsafe ref struct PixelEnumerator<TPixel> {

  //rest of PixelEnumerator code

  public readonly Enumerator GetEnumerator() => new((PixelEnumerator<TPixel>*)Unsafe.AsPointer(ref Unsafe.AsRef(in this)));

  public readonly ref struct Enumerator {

    public Enumerator(PixelEnumerator<TPixel>* it) {
      pit = it;
    }

    readonly PixelEnumerator<TPixel>* pit;

    public TPixel* Current => pit->Current;

    public bool MoveNext() => pit->MoveNext();

  }
  
}

The problem with this approach is two-fold:

• It is hacky with all the unsafe stuff.
• Most importantly it was shown to be 2X worse performance wise, since Current => pit->Current; and pit->MoveNext(); really do add an overhead and likely prevent inlining.

2. An alternative is simply to forego the use of foreach and use while instead, slightly modifying the MoveNext:

while (MyPixelEnumerator.MoveNext(out var p)) ...

public unsafe ref struct PixelEnumerator<TPixel> {

  public bool MoveNext(out TPixel* p) { ... }

}

But again, this does not look as nice.

The Suggestion:

And so I think it would be nice to allow foreaching over types that also satisfy the IEnumerator pattern (i.e. have T Current and bool MoveNext()) instead of allowing strictly those that satisfy the IEnumerable pattern.

[timcassell]

Can you make your enumerable a regular struct instead of ref struct? Then you could just store the ref field directly in the ref struct enumerator. Sadly ref fields to ref structs are not supported yet.

[fitdev]

Thanks for the suggestion. I will try that. Though I suspect perf penalty will still be there because of indirection, not to mention the bulkier implementation of this requiring an extra inner enumerator type.

[timcassell]

It's possible the JIT could optimize it. Would need to benchmark it or look at assembly to find out. I would guess the JIT doesn't optimize pointers the same as refs, but I'm no expert on it.

[fitdev]

Just ran some benchmarks. Unfortunately the ref field approach with enumerable being a regular struct has exactly the same performance as the pointer approach - both are 2X worse than the baseline.

[TahirAhmadov]

I've never done anything like this so take it with a grain of salt, but can you wrap the pixel pointer in a struct which contains the pixel pointer and the X/Y that you need?

[TahirAhmadov]

public unsafe struct PixelPointer<TPixel> where TPixel : unmanaged
{
  public int X;
  public int Y;
  public TPixel* Pixel;
}
public unsafe ref struct PixelEnumerator<TPixel> where TPixel : unmanaged {

  //Part of definition is omitted for brevity

  readonly int Right;
  readonly int Bottom;
  readonly int StartY;
  readonly int OffsetLengthPlusPixelLength;
  int X, Y;

  public readonly PixelEnumerator<TPixel> GetEnumerator() => this; //<-- This is wrong as it returns a new instance and so X and Y become inaccessible from the point of view of the caller

  public PixelPointer<TPixel> Current { readonly get; private set; }
  
  public bool MoveNext() {
    if (++X < Right) {
      Current = new PixelPointer<TPixel> { Pixel = Current.Pixel + 1, X = X, Y = Y };
      return true;
    } else if (++Y < Bottom) {
      X = StartX;
      Current = new PixelPointer<TPixel> { Pixel = (TPixel*)((byte*)Current.Pixel + OffsetLengthPlusPixelLength), X = X, Y = Y };
      return true;
    }
    return false;
  }
 }
foreach(var pixelWrapper in MyPixelEnumerator)
{
  Console.WriteLine("X = " + pixelWrapper.X + ", Y = " + pixelWrapper.Y + ", pixel = " + (*pixelWrapper.Pixel));
}

Pardon any syntax errors, I don't work on things like this at all.

[fitdev]

Thank you for the suggestion. I will try it for perf out of curiosity. However this is rather specific to the concrete example. So even if the perf is ok, it is till not a replacement for the feature being asked for 2 reasons:

1. Now the caller of which there will be 100s of course would have to do pixelWrapper.Pixel instead of simply Pixel - small I know, but that's why I liked foreach approach until I discovered this limitation which lead to a bug in my code.
2. In my example MoveNext() modifies X and Y, but there maybe other use cases where even more variables get modified, and it would be simply much easier to read them off of an enumerator directly (plus less memory copying as a result).

[fitdev]

Just perf-tested your approach and it performed very well actually. So I guess perf-wise it is acceptable, though not as nice to my taste aesthetically (there is now a whole new pixel wrapper type that the callers have to deal with). So thank you for the suggestion.

But until they add support I suggest here, I guess I will be using either this wrapper type or while (enumerator.MoveNext(out p) workaround.

[TahirAhmadov]

I think the perf is similar to accessing the properties on the enumerator because fundamentally, you're still passing around the same amount of data - the pointer and the X/Y. The fact that they're all wrapped in a struct does not add any overhead.

Personally, I'm not sure which way I would go if I had this scenario. Part of me says that the pixel enumeration is such a specific use case, that the while loop is more appropriate. However, if you like the foreach, I think my approach is better than accessing X/Y on the enumerator - it follows the C# "flow" more natively, IMO.

[fitdev]

I think I will be using your wrapper struct approach as my primary method. Turns out perf-wise it is basically free, but on top of that I can write rather nice code like:

foreach (var (p, x, y) in bitmap) { ... }

once the wrapper type was improved with public void Deconstruct(out TPixel* p, out int x, out int y).

So thanks again for this tip!

[HaloFour]

I guess the question is whether it's worth a language feature for the foreach pattern to support GetEnumerator methods that return ref types. The compiler does support calling GetEnumerator, but it'll make a copy at the callsite anyway. So you'd need new syntax to describe that you want the enumerator to be by-ref also. But that doesn't avoid the problem of the ref method not being able to return ref this. I think there's a proposal to allow that from ref structs?

[fitdev]

That's a possible approach I guess. But not what I was suggesting. I was suggesting that instead of:

var IEnumerableAndIEnumeratorLikeStructEnumerator = IEnumerableAndIEnumeratorLikeStruct.GetEnumerator();
while (IEnumerableAndIEnumeratorLikeStructEnumerator MoveNext()) { DoSomething(IEnumerableAndIEnumeratorLikeStructEnumerator.Current); }

the compiler would instead lower it down to this:

while (IEnumerableAndIEnumeratorLikeStruct.MoveNext()) { DoSomething(IEnumerableAndIEnumeratorLikeStruct.Current); }

when the user writes foreach code for IEnumerableAndIEnumeratorLikeStruct when IEnumerableAndIEnumeratorLikeStruct itself has MoveNext and Current Enumerable members:

foreach (var p in IEnumerableAndIEnumeratorLikeStruct) DoSomething(p);

You don't have to support refs for that or need any new syntax. Just recognize that if a type has _Enumerator)-like members MoveNext() and Current (as opposed to just Enumerable-like member GetEnumerator()), then it itself should be used in the foreach without calling GetEnumerator() since it is itself an enumerator-like type.

[HaloFour]

IMO, redefining the enumerable pattern to support directly enumerating over enumerators directly would be more involved and niche. Doing that without changing the meaning of existing code would be tricky and could introduce subtle bugs.

[fitdev]

Feature can be made opt-in with an attribute on the type that will say something like [IForeachEnumerator] or whatever. The LD team has already been doing a lot using attribute approach for features like custom collection builders, and awaitables for example.

Of course, I cannot speak much for implementation side as I know very little, but it seems implementation should be pretty easy (there are of coyrse some extra checks that need to be made but basically it boil;s down to this):

1. Does the type we are trying to foreach over have T Current {get;} and bool MoveNext()?
2. If yes, does the type have [IForeachEnumerator] attribute applied signalling that it opts-in?
3. If so, then simply use the type instance as enumerator and forego generating x.GetEnumerator().

Some of the above can be further simplified if the type does not have GetEnumerator() and does not implement IEnumerable. So if such a type defined Current and MoveNext() there should not be any ambiguity as to how it should work, and so step 2 (the opt in) may be skipped.

[HaloFour]

Sure, it can be solved, but that complicates the design. I just don't see the need for a language feature here, the foreach pattern has never supported raw enumerators as it's already trivial to use while and the language wouldn't have ownership of the lifestyle of that enumerator.

[ufcpp]

#3867

[fitdev]

Thanks for the link. Yeah it looks a bit similar to what I was thinking about, though that proposal was a bit more involved as I can tell.