Enumerable.ToArray() fails on the results of some uses of Enumerable.Cast()

Generally any implementation of IEnumerable<T> should be able to produce a T[] with a call to ToArray().

However, in some cases the IEnumerable<T> produced by a call to Cast() will in fact be an array of another type that can be converted on the fly by the implementation of GetEnumerator(). This works well in many cases and is a valuable optimisation (rather than going through CastIterator) but if ToArray() is called on it, the result is an ArrayTypeMismatchException that would not be expected by the user, especially since code might call ToArray() without knowing that Cast() had been used, or pass the result of Cast() to a method, not knowing that ToArray() would be used.

In particular this happens if you Cast() an array of one enum type to another enum type.

A test case would be nice.

    private enum Enum0
    {
        First,
        Second,
        Third
    }

    private enum Enum1
    {
        First,
        Second,
        Third
    }

    [Fact]
    public void ToArray_Cast()
    {
        var source = new[] { Enum0.First, Enum0.Second, Enum0.Third };
        Assert.Equal(new[] { Enum1.First, Enum1.Second, Enum1.Third }, source.Cast<Enum1>().ToArray());
    }

But even the simple case does not work in the presence of generics.

static X Cast<X, Y>(Y y)
{
  return (X)y;
}

You would have to do:

static X Cast<X, Y>(Y y)
{
  return (X)(object)y;
}

Which means you have to cast to object then target.

Fix Enumerable.ToArray throwing in some cases.

I don't follow, leppie. Generally source.Cast<Enum1>() produces a perfectly fine IEnumerable<Enum1> generally someSequence.ToArray() where someSequence is an IEnumerable<Enum1> produces a perfectly find Enum1[]. It's surprising behaviour that source.Cast<Enum1>().ToArray() fails, especially since the code using Cast and the code using ToArray could be quite far apart.

Ah, I get you now. As a rule Enumerable<Cast>() does do pretty much the same thing as return (X)(object)y. (To be precise, it does foreach (object obj in source) yield return (TResult)obj). The problem is with the optimisation of:

IEnumerable<TResult> typedSource = source as IEnumerable<TResult>;
if (typedSource != null) return typedSource;

Which can cause problems if ToArray() then in turn tries to optimise for ICollection<TElement> collection = source as ICollection<TElement>.

But those two optimisations are hit by more code than is affected by this bug, so the key is to fix the bug without removing the optimisations.

@hackcraft : In that case, the issue is legit :D Good catch.

It is pretty nasty that it does boxing though...

This really is a bug in the Array.CopyTo implementation. On one hand the runtime allows you to freely cast enum arrays with the same underlying type and to unbox enums with the same underlying type but different actual type. On the other hand Array.CopyTo claims that you can't copy to an enum array with the same underlying type. That doesn't make sense no matter how you look at it.

@mikedn

I agree with @mikedn, and I'd rather this be addressed at the source than at every possible place this pattern is employed. @jkotas, who should look at this?

This looks like enhancement request for Array.Copy that is called by Array.CopyTo. The following does not work today:

        var source = new Enum0[3];
        var dest = new Enum1[3];
        Array.Copy(source, dest, 3);

I do not see a fundamental problem with making this work. It should be a simple fix: change GetSignatureCorElementType to GetVerifierCorElementType throughout the implementation of Array.Copy. It should not be a breaking change because of it is changing throwing behavior to non-throwing behavior.

However, such change should be reviewed by the FX design group for consistency issues. I believe that we generally disallow implicit enum conversions, e.g. Array.SetValue or reflection APIs won't perform enum conversions. Is it a good idea to allow them for Array.Copy and keep everything else as is?

@terrajobst

Either that request, or that in this code:

object source = new Enum0[3];
bool b = source is IEnumerable<Enum1>;

b would be false instead of true (it's true today), just as it's false today in this code:

var source = new Enum0[3];
bool b = source is IEnumerable<Enum1>;

But that would be a breaking change. It'd be better if the rest of Array could be made consistent with the current type checking behavior.

cc: @terrajobst

b should really be true in this code:

var source = new Enum0[3];
bool b = source is IEnumerable<Enum1>;

This looks like an invalid optimization performed by C# compiler. I have opened dotnet/roslyn#3830

Is it a good idea to allow them for Array.Copy and keep everything else as is?

Hmm, I'm tempted to say that this should be done only for CopyTo and only for that CopyTo that's accessible via a ICollection<T>. The Array's own Copy and CopyTo should not be changed.

Consider the following code:

class Program {
    enum X {
        Foo
    }
    enum Y {
        Foo
    }
    static void Main() {
        X x = (X)(object)Y.Foo; // works, this why the proposed Cast/ToArray fix works

        X[] a = new X[1];
        a.SetValue(Y.Foo, 0); // doesn't work and I don't think that it should work
        a.CopyTo(new Y[1], 0); // same as above
        Array.Copy(a, new Y[1], 1); // same as above

        IList<Y> l = ((object)a) as IList<Y>;
        l[0] = Y.Foo; // works
        Y y = l[0]; // works
        l.CopyTo(new Y[1], 0); // doesn't work
    }
}

I think that last CopyTo in the above example should work. As is now the runtime allows the cast to IList<Y> but the resulting interface is only partially functional.

I suspect this ultimately goes back to pre-generics .NET broken-but-intractable copying of pre-generics Java's broken-but-intractable array covariance.

To consider it another way. If this issue had been found in initial testing of Cast and it wasn't deemed too minor to care about, the conclusion could well have been that either the optimisation in Cast was too risky, or the optimisation in ToArray was too risky, and one or both of them should be removed.This would have stopped the problem hitting in this point that was introduced with .NET 3.5, but left any issues at 3.0 or earlier as is.

But coming in now with that would cause a performance hit on code that benefits from the optimisation, which is likely quite a lot. As such a change like that in this PR has the advantage of being relatively low-impact, and it could be diked out if a deeper change made it unnecessary.

As such a change like that in this PR has the advantage of being relatively low-impact

While that may be true, the fix proposed in dotnet/corefx#2242 only fixes one particular case of this issue:

var source = new Enum0[] { Enum0.First };
var results = source.Cast<Enum1>().ToArray();

It doesn't fix using ToList:

var results = source.Cast<Enum1>().ToList();

nor using List<T>'s ctor directly:

var results = new List<Enum1>(source.Cast<Enum1>());

nor without Cast<TResult>:

var results = new List<Enum1>(source as object as IEnumerable<Enum1>);

nor using some other IList<T>.CopyTo wrapper:

new ReadOnlyCollection<Enum1>(source as object as Enum1[]).CopyTo(new Enum1[1], 0);

nor any number of other occurrences of this pattern in the framework itself, in 3rd-party libraries, or in application code.

It seems the right solution here is to make new or existing CopyTo to work with everything assignment-compatible.
I have entered https://github.com/dotnet/corefx/issues/13816 to track that.

I think we should close this for now until dotnet/corefx#13816 is fixed.

Let's keep it open to not loose track.

Blocked on dotnet/corefx#13816

Add test for Issue 2241 and close it.

Add test for Issue 2241 and close it. (#21979)

Navigation Menu

Enumerable.ToArray() fails on the results of some uses of Enumerable.Cast() #14794

8 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Enumerable.ToArray() fails on the results of some uses of Enumerable.Cast() #14794

Description

Activity

leppie commented on Jul 3, 2015

JonHanna commented on Jul 3, 2015

leppie commented on Jul 3, 2015

JonHanna commented on Jul 3, 2015

JonHanna commented on Jul 3, 2015

leppie commented on Jul 3, 2015

mikedn commented on Jul 4, 2015

stephentoub commented on Jul 4, 2015

jkotas commented on Jul 4, 2015

stephentoub commented on Jul 4, 2015

jkotas commented on Jul 4, 2015

mikedn commented on Jul 5, 2015

JonHanna commented on Jul 5, 2015

stephentoub commented on Jul 5, 2015

8 remaining items

VSadov commented on Nov 18, 2016

VSadov commented on Nov 18, 2016

karelz commented on Nov 18, 2016

karelz commented on Nov 18, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions