Excellent write-up. That makes total sense to me. +1

👍
Although you have one sentence that seems misleading:

It is possible to convert T[] to U[] in most cases where all elements in the array can convert to U.

The conversion's success has nothing to do with the types of elements in the array, and everything to do with how the array itself was created. A T[] can only convert to U[] if in fact T[] already is a U[] that was previously upcast.

@AArnott

I spent probably a good five minutes on that sentence and couldn't find anything I was truly happy with (that didn't expand into a mini version of the C# spec). :(

In general I like the idea and I support the functionality but I'm a little interested to see what others think about the names. We don't have any other methods in the framework called CastUp or CastDown. Perhaps we could call CastUp just Cast and CastDown CastDynamic or something along those lines.

I don't think there are names we could recycle. Cast is wrong in my mind because Linq already provides a method with this name but it's not restricted to being an up-cast -- it will simply perform a direct cast.

Personally, I actually like the proposed names.

CastUp and CastDown makes sense to me. I'm in favor if the proposed names.

I made an update to the proposal. @stephentoub pointed out that the way I had CastUp defined wouldn't work because it required additional constraints on the class type parameter T from within a method. In order to work the types involved need to be inverted

• Class type parameter needs to be the parent type.
• Method type parameter needs to be the derived type.

The method was changed to reflect this and rename to CastFrom as it seems to be a more appropriate name.

For the second method I personally prefer Cast over CastDown. The reason being that the method can cast up, down and even side ways (between interfaces). The more appropriate name is just Cast.

That makes sense. I agree that in that Cast is the right name. Can you update the proposal then?

@terrajobst updated

As a developer trying to use these APIs how am I to figure out that CastFrom actually doesn't allocation without reading documentation? I have a feeling a lot of people will just simply call Cast unless they know what the other one provides. In a case where people call Cast directly with a type can be up cast will it detect this and do the non-allocating version? If so that seems to argue that we just have the Cast method and if not is there a reason?

Another question you are proposing we remove a public API what impact will that have? Have we shipped a stable version with that API?

@weshaggard the term cast is generally associated with a type conversion, not an allocation.

Neither of these APIs is allocating, it is just doing a type conversion under the hood. The difference between the methods is CastFrom doesn't require a dynamic type check but Cast does.

@nguerrera or @AArnott would have to comment on whether or not we've shipped with that API public before.

@jaredpar sorry I miss understood the allocation part after re-reading your proposal I think I confused it with the comparison to the other Create methods which allocate.

In your code example what happens if people do this:

ImmutableArray<Dog> dogArray = ImmutableArray.Create<Dog>();
ImmutableArray<Animal> animalArray = dogArray.Cast<Animal>();

That seems to me like the most natural thing for someone to do. If that isn't substantially worse than calling the static CastFrom then why not just have the one Cast method? The fewer concepts people need to figure out the better.

@weshaggard the code will end up producing the same result with an additional type check. The reason for adding the extra concept is that this type check can be significant in tight loops.

Roslyn for example regularly executes this exact pattern in the parser:

• Create an ImmutableArray<ChildNode>
• Convert to an ImmutableArray<ParentNode>

Profiles showed that the type check in this conversion was taking up a significant portion of parse time. Moving to the non-type checking version was a clear win (and one that we simply cannot regress on).

The awkward consumption of the existing ImmutableArray.Create API is what prompted this request. Roslyn ended up writing their own version which had the basic features called out in this request.

• Roslyn Static Cast

We felt it was enough of a win that it should just be in the core API.

Thanks for the explanation I'm good with the additions. I would like to get clarification from @nguerrera and @AArnott before we remove the one Create method however.

In the previous API Review meeting, I think it was mentioned that we had not shipped a stable version of ImmutableArray yet, and were free to make these changes. Note that we just renamed ReverseContents to Reverse with dotnet/corefx#399, so we are already making changes along these lines.

ImmutableArray itself has never shipped in a stable package, so we're good
there. And I agree with Jared's other comments.

On Mon, Jan 12, 2015, 10:32 PM Matt Ellis notifications@github.com wrote:

In the previous API Review meeting, I think it was mentioned that we had
not shipped a stable version of ImmutableArray yet, and were free to make
these changes. Note that we just renamed ReverseContents to Reverse with
dotnet/corefx#399 dotnet/corefx#399, so we are already
making changes along these lines.

—
Reply to this email directly or view it on GitHub
https://github.com/dotnet/corefx/issues/400#issuecomment-69701353.

Yes, we can still make breaking changes to ImmutableArray, but we had said that we'd keep them to a minimum if at all possible. I do see a few uses of Create<T, TDerived>, so we'd need to coordinate carefully with our partners.

One comment, the type parameter for Cast should be probably just Cast, to be consistent with Linq, and to indicate that it can go both up and down.

Won't Cast hide the lynq extension method? Their behaviors, return types
and cases where they fail are different, so eclipsing one with the other
may be a bad idea.

On Tue, Jan 13, 2015, 11:17 AM David Kean notifications@github.com wrote:

One comment, the type parameter for Cast should be probably just Cast, to
be consistent with Linq, and to indicate that it can go both up and down.

—
Reply to this email directly or view it on GitHub
https://github.com/dotnet/corefx/issues/400#issuecomment-69801834.

Return type differences are fine - it's common to provide strongly typed versions of enumerable extensions. How will their behaviors differ?

@davkean the LINQ Cast method is casting elements and the ImmutableArray<T> version is casting arrays. Concrete example of where this would differ:

Animal a = new Dog(); 
var array = ImmutableArray.Create(a);  // Creates a Animal[] 
Enumerable.Cast<Dog>(array);  // Successfully returns IEnumerable<Dog> 
array.Cast<Dog>();  // Fail: can't convert Animal[] -> Dog[] 

C# binding rules will cause ImmutableArray<T>.Cast to be preferred over Enumerable.Cast.

@jaredpar @davkean @AArnott Given that, I'm inclined to think that we should avoid hiding Enumerable.Cast with something slightly different.

Given that, I'm inclined to think that we should avoid hiding Enumerable.Cast with something slightly different.

CastTo?

CastTo doesn't convey the significant difference IMO.
What about CastArray?

@AArnott CastArray is what I've been thinking about as well.

What's the behavior when the cast doesn't succeed? Can you call this out in the spec?

@davkean from the spec

Cast will be non-allocating but will have a dynamic type check. If that type check fails an exception will be generated (in the same manner as a normal C# array conversion).

Is there something else you wanted to say in addition to that?

Which exception, InvalidCastException?

@davkean it throws whatever the array conversion would throw.

We've reviewed this proposal and we're fine with this approach. We've some concerns around the naming but this can be reviewed on the PR.

Fixed via dbfb49b