Runtime: Enumerable.Empty<T> in .NET Core 3.0 cannot be cast to IList<T>

[natemcmaster]

Yes, casting is bad...so we probably shouldn't do this, but is it reasonable to expect this to keep working?

using System;
using System.Collections.Generic;
using System.Linq;

namespace cast_to_list
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine($"There are {GetBugs().Count} bugs in casting Enumerable.Empty<T> to IList<T>");
        }

        private static IList<int> GetBugs()
        {
+           return (IList<int>)Enumerable.Empty<int>();
        }
    }
}

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netcoreapp2.1;netcoreapp3.0</TargetFrameworks>
  </PropertyGroup>

</Project>

The cast works in .NET Core 2.1, but not 3.0. In 3.0, it fails with:

Unhandled Exception: System.InvalidCastException: Unable to cast object of type 'System.Linq.EmptyPartition`1[System.Int32]' to type 'System.Collections.Generic.IList`1[System.Int32]'.

I'm using .NET Core 2.1.5 and 3.0.0-preview1-27003-04.

cc @rainersigwald

[stephentoub]

This changed as part of my PR https://github.com/dotnet/corefx/pull/31025.

is it reasonable to expect this to keep working?

IMHO, no, especially in a major release. I think it's perfectly reasonable for an API that returns IEnumerable<T> to change what concrete implementation of it it hands back. That's a key reason to return an interface in the first place.

[stephentoub]

ps Of course, if it really ends up being a sore point for lots of folks, that changes what's reasonable ;)

[natemcmaster]

I agree, but I figured I would at least ask since "reasonable" is an imprecise metric.

Microsoft/MSBuild has code in VS 15.8 that does this. I don't know how hard it will be to update it. I'll open a new issue on MSBuild to fix it up. cc @rainersigwald @AndyGerlicher

[stephentoub]

pps I expect if we wanted to address this, we'd do so by adding the interface to https://github.com/stephentoub/corefx/blob/48429ccae7b566d5b7cc2a1862885123daadd806/src/System.Linq/src/System/Linq/Partition.SpeedOpt.cs#L20 and implementing it appropriately. The downside of that is primarily larger binaries / working set.

[stephentoub]

Of course, if we're concerned about people casting to IList<T>, we'd probably also be concerned about them casting to T[], in which case we'd probably want to revert that whole line and go back to returning Array.Empty<T>(). The downside to that is we lose downstream optimizations based on IPartition<T>.

[aalmada]

is it reasonable to expect this to keep working?

IMHO, no, especially in a major release. I think it's perfectly reasonable for an API that returns IEnumerable<T> to change what concrete implementation of it it hands back. That's a key reason to return an interface in the first place.

@stephentoub True but ICollection<T> and IList<T> perform a lot better in many scenarios. For this reason, LINQ and many other third-party libraries, are full of conditions like if (source is ICollection<T>). Returning different implicit contracts under different conditions will be a source of confusion. I stepped into this issue while unit testing dotnet/corefx#37388 and the tests were failing unexpectedly.

Unfortunately, casts are expensive and these conditions add unnecessary overhead at runtime.

The late addition of IReadOnlyCollection<out T> and IReadOnlyList<out T> interfaces to the framework made this issue even worse. Adding these to LINQ would mean more runtime conditions and, because these are covariant, these would be WAY MORE EXPENSIVE...

IMHO, enumeration methods should return the highest admissible enumeration interface so that the best enumeration strategy is inferred at compile time.

Yes, that's what interfaces are for but, IEnumerable<T> is the System.Object of enumeration...

[replaysMike]

It would have been more useful for this change to exist had you made EmptyPartition<> at least an accessible type. This affects various serialization libraries that exist out there (Newtonsoft, AnySerializer) as it went from being a non-generic array to a generic enumerable of objects. Fixable for multitarget libraries but has some odd consequences.