Docs: The unobvious implicit cast in foreach statements

[eduard-malakhov]

Today, I've run against a quiet unobvious behavior of the C# compiler that I believe deserves to be documented. I've submitted an issue to the roslyn team (dotnet/roslyn#25080), but it looks like this behavior is considered correct (although I can't even find proper words to express how I disagree).

There's a detailed description in the issue, in short: when the C# compiler has to process a statement like
C# foreach (T item in items) { }
where items is of type IEnumerable<TImpl> (or equivalent enumerable type as supported by foreach) and T is an interface that is not implemented by TImpl, it (compiler) adds a cast from object to T and allows the statement to compile. Thus, at some point at run time there might be an InvalidCastException upon an attempt to cast an item from items that does not implement T.

It think this needs to be documented somewhere and I haven't found any related docs. @mairaw what do you think about this?

edit by www: Add document details section so this displays in the issues control on the foreach reference page.

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

• ID: d710feeb-8f03-b211-a070-155938929d6b
• Version Independent ID: c980ea8e-ffb4-fbb1-1761-292858616cdd
• Content: C# foreach statement
• Content Source: docs/csharp/language-reference/keywords/foreach-in.md
• Product: dotnet-csharp
• GitHub Login: @BillWagner
• Microsoft Alias: wiwagn

[svick]

In which article would you expect to find this information?

[pkulikov]

This section on foreach tries to show how that statement is compiled.
A quote from there:

The above steps, if successful, unambiguously produce a collection type C, enumerator type E and element type T. A foreach statement of the form

foreach (V v in x) embedded_statement

is then expanded to:

{
    E e = ((C)(x)).GetEnumerator();
    try {
        while (e.MoveNext()) {
            V v = (V)(T)e.Current;
            embedded_statement
        }
    }
    finally {
        ... // Dispose e
    }
}

Can it be a good place to mention InvalidCastException?

[svick]

@pkulikov That's the specification. If you can get though all of the preceding text, then I think it can be assumed that you don't need a reminder that a cast can throw.

[eduard-malakhov]

@svick I think it would be nice to be able to find it somewhere. That's the key, Google does a good job of indexing the docs as well as the rest of the web, we just need to pick a good place. I am not familiar with the entire structure of the docs, so I'd like to hear suggestions from the team.

One place I could think of (definitely questionable) is the foreach, in keyword reference. Maybe not the best place, but this behavior is specific to way the compiler handles foreach statements. At least we could add some more "advanced" examples that demonstrate this sort of usage alongside with the trivial examples we have there already.

Some article about compiler logic itself could work too, but I'm not sure if there are any articles about compiler internals.

[pkulikov]

What about this page in language reference?

[eduard-malakhov]

@pkulikov I would agree with @svick here. Specification is great to understand how things work, but I'd prefer it more adapted to an average developer. Many people I think won't read the specification even if they are able to find the right article.

[pkulikov]

@eduard-malakhov @svick you are right, I agree with suggestion of @eduard-malakhov to update keyword reference page with one or two more examples. So far it looks the best place.

[eduard-malakhov]

@pkulikov ok, thank you for participating. I will then create some examples, submit a PR to update keyword reference, and we'll see where it goes.

[mairaw]

The proposal looks good to me. But I'd prefer to have @BillWagner input on this one.

[BillWagner]

This is a good proposal.

I'll add some details to your example that should be part of the explanation. Let's start with your original example:

namespace ConsoleApp1
{
    class SomeType { }

    class DifferentType { }

    interface SomeRandomInterface { }

    class Program
    {
        static void Main(string[] args)
        {
            SomeType[] arrayOfSomeType = new SomeType[]
            {
                new SomeType(),
            };

            IEnumerable<SomeType> enumerableOfSomeType = arrayOfSomeType;

            //  This compiles, but throws InvalidCastException at run time (and puzzels me)
            foreach (SomeRandomInterface item in enumerableOfSomeType) { }

            //  This won't compile (thankfully)
            //foreach (DifferentType item in enumerableOfSomeType) { }
        }
    }
}

One of the commenters on your roslyn issue pointed out that it does not compile if SomeType is sealed. It only compiles when SomeType could have derived classes.

Aha!

In that case, the actual objects in the collection may not be SomeType but could be ATypeDerivedFromSomeType. That derived type could implement SomeRandomInterface.

As others have mentioned, this behavior dates from a time before generics, where SomeType would likely be object. This idiom would have been more common then.

Does that help explain why this is this?

pinging @madsTorgersen for any corrections.

[sjb-sjb]

Would be nice to have an example where the feature is useful, in addition perhaps to the example where it was not very useful. Like this:

ObservableCollection<Order> Orders = … 

Orders_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) 
{ 
    if (e.NewItems != null) {
        foreach (Order order in e.NewItems) { 
            ...
        } 
    }
}

[BillWagner]

See also AB#1702034

Given the number of comments on this issue, I think fixing this issue will address the internal work item.