Type.SatisfiesGenericConstraints proposal

[jbogard]

Background and Motivation

Today, the only way at runtime to determine if a generic type definition can successfully close based on a set of generic parameters is to call Type.MakeGenericType and catch any exceptions:

var genericTypeDefinition = typeof(List<>);
Type closedType = null;
try {
    closedType = genericTypeDefinition.MakeGenericType(typeof(int));
} catch (ArgumentException) { }
bool canClose = closedType == null;

In libraries that use open generic types heavily (such as DI containers), either the logic for determining whether or not an open generic type can be closed is duplicated, or this exception is caught. Many times, the libraries will do both - cover the basic cases but still rely on exceptions.

This is especially useful in generic constraints, where I have a collection of open generic types, and I only want to resolve the closed types that satisfy the generic constraint at runtime.

Proposed API

Expose an API to check if a given set of types can successfully satisfy the generic constraints of an open generic type:

namespace System
{
    public abstract class Type 
    {
+       public virtual bool SatisfiesGenericConstraints(params Type[] typeArguments);
    }

Or can check a specific generic parameter:

namespace System
{
    public abstract class Type 
    {
+       public virtual bool SatisfiesConstraints(Type parameter);
    }

Usage Examples

Simple example:

class MyType<T> where T : class
{
}

typeof(MyType<>).GetGenericArguments()[0].SatisfiesConstraints(typeof(int)) // This should be false
typeof(MyType<>).SatisfiesGenericConstraints(typeof(int)) // This should be false.

Risks

This API is not exposed directly in the CLR.

[jkotas]

Moved the API proposal to CoreFX (https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md)

[steveharter]

Can you provide details on the scenario where you don't know if MakeGenericType will succeed? Normally when calling that API you know the generic arguments.

[jbogard]

@steveharter literally every single dependency injection container you look at won't know those generic arguments until resolution time, so all of them call MakeGenericType in a try-catch block. The only one that doesn't is the MS Ext DI container, and it will allow an unhandled runtime exception, blowing up the application.

I have a PR to fix this behavior: https://github.com/dotnet/extensions/pull/536 but obviously that is not ideal, I would rather use the TryXyz behavior.

Some DI containers try to reverse engineer the constraint check, but as you can imagine, it's horribly ugly and error-prone. So the @jskeet approved method is to call MakeGenericType and catch the exception.

[jkotas]

From the runtime point of view, trying to create random generic instantiations without knowing that they are valid is a bad pattern.

Note that the fact MakeGenericType happens to succeed does not guarantee that the instantiation is valid. For example, Vector<T> instantiations are valid for certain Ts only that is not reflected in the type constrains; or the C# unmanaged constrain is not checked by the runtime at all.

I think that this issue can be closed. The DI systems that would like to use this pattern can keep using try / catch.

[dotnetjunkie]

@jkotas

From the runtime point of view, trying to create random generic instantiations without knowing that they are valid is a bad pattern.

As @jbogard already mentioned, for DI Containers it is very common to build generic types. Simple Injector for instance applies decorators conditionally and filters collections based on generic type constraints. And generic type constrains can become very, very complex, but they can do so because of valid reasons.

The DI systems that would like to use this pattern can keep using try / catch.

I would argue against this. The lack of a Type.TryMakeGenericType forces DI libraries to swallow exceptions. Swallowing exceptions still causes developers to be prompted with an exception window in the Visual Studio debugger, because of the default VS settings. This leads to confusing situations, because its not always clear that the exception can be ignored, and it is a productivity killer for the debugger to break in the middle of a debugging session.

To prevent getting these first-chance exceptions, Simple Injector currently tries to do the complete analysis of generic type constraints in order to prevent developers from getting a popup they can ignore. This, however, causes a lot of complexity, because, as I already stated, generic type constraints can become extremely complex.

But Simple Injector is by far the only library that does this. All DI Containers have this problem, and MS.DI will likely get this problem in the future when it becomes more mature. And there are likely other types of libraries that apply this kind of filtering based on generic type constraints. In other words, the addition of a Type.TryMakeGenericType is highly appreciated.

[jkotas]

Swallowing exceptions still causes developers to be prompted with an exception window in the Visual Studio debugger

I do not believe that it is the case. The default setting for Visual Studio debugger is to continue on exceptions (the exception gets printed into debug output window).

For reference: https://github.com/dotnet/runtime/issues/21785 has a long discussion on merits of adding Try variants for existing APIs.

[dotnetjunkie]

I certainly do understand the consequences of having Try* APIs, which has its downsides. But let's take a step back and look at what's actually requested here, where a TryMakeGenericType is just one of the solutions.

The actual problem is that we want to be able to verify whether the supplied types match a generic type's generic type constraints. Whether this is done using Type.TryMakeGenericType, Type.CanMakeGenericType, GenericTypeConstraintVerifier.VerifyTypeConstraintsFor, or anything else, is IMO not that relevant. We want to be able to reliably knowing up front whether we can safely call Type.MakeGenericType, without it throwing because of type constraint mismatches. You've got carte blanche in designing that API ;-)

[seesharper]

+1

[steveharter]

We need an actual API proposal in order for this to get approved. Can @jbogard or other propose the APIs?

However, the window for making API reviews is essentially closed for 5.0, so moving to Future.

[jbogard]

@steveharter updated the original comment.

[davidfowl]

I think we want the alternative proposal:

namespace System
{
    public abstract class Type {
        public virtual Type MakeGenericType(params Type[] typeArguments);
+        public virtual bool CanMakeGenericType(params Type[] typeArguments);
     }

I can take this to API review.

[jkotas]

I assume that this API is meant to just check the generic constraints. I would call it SatisfiesConstraints to make it clear what it does.

There is a parallel MethodInfo.MakeGenericMethod that should get symmetric treatment.

The implementation of this method is going to do foreach (var parameter in GetGenericArguments()) { if (!parameter.SatisfiesConstraints(...) return false; }. Would it make sense to have SatisfiesConstraints on the generic argument instead? It would allow you to figure which generic argument is not satisfying the constraints.

[huysentruitw]

IMO SatisfiesConstraints does not make it more clear, unless you'd call it SatisfiesGenericTypeConstraints. I think from the API consumer side, the CanMakeXyz that takes the same parameters as the MakeXyz would be more clear.

[svick]

@jkotas If you're suggesting the usage to be e.g. typeof(List<>).SatisfiesConstraints(typeof(int)), then I think that's more confusing. I would initially read it as the nonsensical "Does List<> satisfy the int constraint?" On the other hand, I think CanMakeGenericType is more understandable. Specifically, I would read typeof(List<>).CanMakeGenericType(typeof(int)) as "Can List<> make generic type using int?"

Also, CanMakeGenericType clearly indicates it's a companion API to MakeGenericType, SatisfiesConstraints does not make that association obvious.

[jkotas]

MakeGenericType can fail for number of reasons. Do you expect CanMakeGenericType to cover all reasons why it can fail, or just the constraints?

[huysentruitw]

From the CanMakeXyz perspective, it should return false for any failure reason. If that is what we want in the way the DI container will use it, is another question indeed.

[davidfowl]

I agree with @jkotas that we really only care about constraints. For example, another reason it can fail today is for generic arity mismatches (we don't care as much about that).

namespace System
{
    public abstract class Type 
    {
+       public virtual bool SatisfiesGenericConstraints(params Type[] typeArguments);
    }

I do think we want something that's simple to use so something like this would be preferred. The other API we could consider adding is the one that works on generic parameters:

The implementation of this method is going to do foreach (var parameter in GetGenericArguments()) { if (!parameter.SatisfiesConstraints(...) return false; }. Would it make sense to have SatisfiesConstraints on the generic argument instead? It would allow you to figure which generic argument is not satisfying the constraints.

namespace System
{
    public abstract class Type 
    {
+       public virtual bool SatisfiesConstraints(Type parameter);
    }

Example:

class MyType<T> where T : class
{
}

typeof(MyType<>).GetGenericArguments()[0].SatisfiesConstraints(typeof(int)) // This should be false
typeof(MyType<>).SatisfiesGenericConstraints(typeof(int)) // This should be false.

[steveharter]

@jbogard does the API proposal from @davidfowl completely address your scenario? (not to throw due to generic constraints)

If so, please update the main description to the proposal and we'll mark it "ready for review" if it looks good. Thanks

[ghost]

This issue has been marked needs-author-action and may be missing some important information.

[jbogard]

Yep it does, thanks!

[KennethHoff]

@steveharter are we missing anything before marking it as "ready for review", or has this simply been forgotten about? @jbogard has approved his part

[aldrashan]

Any movement on this?