Add IsGenericTypeParameter and IsGenericMethodParameter to System.Type

[ghost]

Proposed apis

class Type
{
    public virtual bool IsGenericTypeParameter => IsGenericParameter && DeclaringMethod == null;
    public virtual bool IsGenericMethodParameter => IsGenericParameter && DeclaringMethod != null;
}

class TypeDelegator (and TypeBuilder,GenericTypeParameterBuilder,EnumBuilder)
{
    public override bool IsGenericTypeParameter { get; }
    public override bool IsGenericMethodParameter { get; }
}

These properties distinguish between generic parameters introduced by types and generic parameters introduced by methods.

Rationale

Currently, the only way to distinguish generic parameters on types from generic parameters on methods is by testing whether DeclaringMethod returns null.

This idiom will not work with the recently added Signature Types (https://github.com/dotnet/corefx/issues/16567) as these types do not have a MethodInfo to return and will always throw on a DeclaringMethod call. While one could work around this today by writing this:

IsGenericMethodParameter => IsGenericParameter && (IsSignatureType || DeclaringMethod != null)

this is not very intuitive, nor is it future-proof as it's not inconceivable that in the future, we may add the ability to create signature types representing unbounded generic type parameters.

Even for regular types, these new properties are more succinct, expressive and allow properly factored underlying implementations (CoreRT) to override with the maximally performant "return true/false" implementations, eliminating a virtual call. Signature Types, of course, will have to override.

Naming

Naming is consistent with the recently added

Type MakeGenericMethodParameter(int position)

api.

[terrajobst]

Video

Looks good, but we don't understand why the properties have to be virtual. It seems they are in nature static. At the end you explain that but how would we eliminate the virtual method call?

[ghost]

"eliminating a virtual call" referred to the fact that the CoreRT implemention would result in one virtual call rather than two (IsGenericParameter + DeclaringMethod).

For this particular case, the apis have to be overridable (or hard-coded to cast-check for SignatureType) because one the key reasons they're being proposed is that it's not safe to call DeclaringMethod on Signature Types.

[karelz]

Sounds reasonable to me. This is a good candidate for up-for-grabs IMO, do you want to mark it so @atsushikan?

[ghost]

I've already got the changes locally and it touches files that are only built in closed-source projects.

[ghost]

It looks like there might be a up-for-grabs on the CoreCLR side but it'll have to wait until I merge my CoreRT stuff and it gets propagated over to CoreCLR through the shared partition. The shared code is actually enough to make the CoreCLR side functional (we can go ahead and expose the api in CoreFx etc.) but the perf there won't be any better than the idiomatic approach today and there's opportunity to improve it there (i.e. don't allocate a MethodInfo just to determine that there is one...)

[karelz]

FYI: The API review discussion was recorded - see https://youtu.be/m4BUM3nJZRw?t=5730 (3 min duration)

[ghost]

The apis are now exposed in the contracts and functioning on CoreCLR and CoreRT.

There is an optional "up-for-grabs" perf opportunity in CoreCLR. CoreCLR right now defaults to the fallback implementation on Type.cs, which works but invokes three virtual calls and the unnecessary retrieval of a MethodInfo. It'd be nice to override these two apis here:

https://github.com/dotnet/coreclr/blob/076a61ef2eb3baf69182139f36b948f104e3b740/src/mscorlib/src/System/RtType.cs#L3750

with a custom version that makes just one FCall. The first portion of this FCall:

https://github.com/dotnet/coreclr/blob/076a61ef2eb3baf69182139f36b948f104e3b740/src/vm/runtimehandles.cpp#L1236

contains the logic needed to determine that (1) the type is a generic parameter and (2) which kind it is.

[ghost]

Closing deadwood issues as part of ownership transfer.