Runtime: Type: Needs api to distinguish Multidim arrays

[MichalStrehovsky]

Forked off from dotnet/runtime#20376 which is adding IsSZArray to Type. After that proposal got approved, you can check whether a System.Type is a multidim array by writing type.IsArray && !type.IsSZArray.

This check is rather awkward. We used to have this kind of API surface in the CoreRT type system, but it lead to problems with readability of the code that used the API. We ended up adding an IsMDArray property to the type system - refer to dotnet/corert#1925 to see how it improves readability (it lead to substantial improvements in readability in places that were dealing with arrays and the IsArray part was implicit, but we needed to do something special for MD arrays - IsMDArray captures the intent better than !IsSZArray)

Proposal:

Add a new property to System.Type:

      public virtual bool IsMultiDimensionalArray => IsArray && !IsSZArray;

[terrajobst]

The API would make sense, but the check you proposed doesn't seem right. My understanding is that using Array.Create you can create single-dimension, zero based arrays, but those aren't SZ arrays. Your check would return true. It seems the check should be something like this:

type.IsArray && !type.IsSZArray && type.GetArrayRank() > 1

If the original check is indeed the one you intend, then the name should be different.

[ghost]

I'd want it to return true - they may be "single-dimension" and have "zero bounds" but they aren't the same type and this api is specifically useful for the cases where we have to treat them differently (e.g. string serialization, type name generation.)

Coming up with an accurate but usable name should be interesting in light of the fact that .NET designed two incompatible ways to create single-dim zero-bounded arrays :-)

[MichalStrehovsky]

It seems the check should be something like this:
type.IsArray && !type.IsSZArray && type.GetArrayRank() > 1

@terrajobst If GetArrayRank() > 1 is true, you don't even need the !type.IsSZArray check. This is explicitly something that needs to returns true if this is a multidimensional array type, even if it's rank is 1. Multidimensional array of rank 1 is a different type from vector in the .NET type system but reflection API surface doesn't provide good ways to distinguish those.

[MichalStrehovsky]

IsMDArray might be a better name after all. It complements IsSZArray and makes people who are not "in the know" not make assumptions about what that name means.

I can see how IsMultidimensionalArray would suggest the rank is greater than 1, but that's not how the ECMA-335 spec uses the term, or how the .NET type system operates.

IsMDArray puts more cognitive load on people and maybe they'll read the XmlDoc comments for this before using it.

[terrajobst]

I'd want it to return true - they may be "single-dimension" and have "zero bounds" but they aren't the same type and this api is specifically useful for the cases where we have to treat them differently (e.g. string serialization, type name generation.)

But isn't that the difference between SZArray and all other arrays? We already expose IsSZArray and GetArrayRank(). This seems to be all you need, no?

I've no problems with exposing a new API but I think if we call it IsMultidimensionalArray (or IsMDArray) it shouldn't return true unless the array has more than one dimension.

[ghost]

We already expose IsSZArray and GetArrayRank() . This seems to be all you need, no?

True, this is a convenience api, not a functionality hole closer. However, the same argument could be made for IsConstructedGenericType - it's "just" a composition of two other apis, but it has an expressiveness advantage (as well as a small perf advantage if the provider implements it correctly.)

It's simply more direct to say "this type belongs in set A" vs. "this type belongs in set AandB but not in B".

it shouldn't return true unless the array has more than one dimension.

If it were to behave that way, I don't think it's even worth adding an api - it eliminates the reason for proposing it.

[ghost]

BTW, I'm modifying the proposal so that the body of the method is "IsArray && !IsSzArray" rather than "NotImplementedException."

Providers such as CoreRT and CoreCLR are still encouraged to override with a more performant implementation but this reduces the burden on third party providers and makes the relationships between the apis clearer.

[ghost]

One more thought: the bounds are not part of the Type, they're part of array instances.

So one way to think of it is that the IsArray subset of Types is split into two:

• IsSZArray - The Types that can only spawn one-dimensional zero-bounded arrays.
• Is???Array - The Types where the bounds are unrestricted.

So if we can decide on a name for "???" that properly captures this concept, are we good? (IsVariableBoundArray? IsArbitraryBoundArray?)

(Though I do think IsMDArray does capture the concept within the vernacular universe of .NET)

[JonHanna]

It's perhaps because I think of them as "non–SZ arrays" that to me !type.IsSZArray seems fine, but if there's a need for it IsMDArray seems about as good as one can get; clear enough to those who know the distinction and just obscure enough to avoid people thinking they know what it means when they don't!

[terrajobst]

So my understanding is we have a hierachy of these two classes of arrays:

IsArray
IsSZArray
IsVariableBoundArray

In that case I'd prefer IsVariableBoundArray.

[ghost]

CoreRT and CoreCLR implementation work done (might take a couple of days to propagate everywhere.)

Just needs to be added to CoreFx ref assemblies and some simple tests.

[karelz]

This should be fairly simple by following similar pattern in https://github.com/dotnet/corefx/pull/16996

[danmosemsft]

@karelz @stephentoub another that's already in coreclr but not exposed: IsVariableBoundArray.

[jlanng]

I'd be happy to surface this in CoreFX. @karelz

Should it also appear on TypeInfo and associated classes, in the same manner as IsSZArrray?

[karelz]

Should it also appear on TypeInfo and associated classes, in the same manner as IsSZArrray?

I think it should be in the same manner as IsSZArray -- @AtsushiKan @MichalStrehovsky

BTW: @AtsushiKan @MichalStrehovsky for future -- these questions could have been easily avoided if we had formal API proposal (see the example link there) with full API surface, class names, etc. I didn't want to be too bureaucratic to enforce it for 'obvious' things, but this just proves that I should be enforcing it more in future ;-)

[karelz]

@jlanng assigned to you.

[ghost]

TypeInfo derives from Type so it gets everything Type gets. There's no need to add new overrides or apis there in the future. TypeInfo is now a relic of the aborted .Net Core minimization of Reflection - it will slowly fade out of sight and mind.

[danmosemsft]

@jlanng we got the okay to submit this API for 2.0 if we get a PR up in the next 2 or 3 days. If you can't manage that, it's okay, it will just miss 2.0. :)