dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

[mono] Lack of generic parameter constraint checks between derived and parent interfaces #99819

Open fanyang-mono opened 6 months ago

fanyang-mono commented 6 months ago

Mono doesn't complain for these two cases. One idea to make it work is to inflate the parent interfaces with the concrete type and validate the parent interface generic parameters via is_valid_generic_instantiation, like class inheritance does in mono_class_init_internal. Another idea is that maybe somewhere down the line from mono_class_implements_interface is a good place to add this check and call mono_class_implements_interface during class creation for classes which implements interfaces.

.class public auto ansi abstract sealed beforefieldinit Exec
    extends [System.Runtime]System.Object
{
    .method public hidebysig static
        string test2() cil managed
    {
        ldtoken class I4`1<valuetype S1>
        call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
        callvirt instance string [System.Runtime]System.Object::ToString()
        ret
    }
}

.class interface public auto ansi abstract beforefieldinit I3`1<(ICanCount) T>
{
}

.class interface public auto ansi abstract beforefieldinit I4`1<W>
    implements class I3`1<!W>
{
}

.class interface public auto ansi abstract beforefieldinit ICanCount
{
    // Methods
    .method public hidebysig newslot abstract virtual 
        instance int32 Count () cil managed 
    {
    } // end of method ICanCount::Count

}

.class public sequential ansi sealed beforefieldinit S1
    extends [System.Runtime]System.ValueType
    implements ICanCount
{
    .pack 0
    .size 1

    // Methods
    .method public final hidebysig newslot virtual 
        instance int32 Count () cil managed 
    {
        // Method begins at RVA 0x2050
        // Code size 2 (0x2)
        .maxstack 8

        IL_0000: ldc.i4.1
        IL_0001: ret
    } 

}
.class public auto ansi abstract sealed beforefieldinit Exec
    extends [System.Runtime]System.Object
{
    .method public hidebysig static
        string test() cil managed
    {
        ldtoken class I2`1<valuetype S>
        call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
        callvirt instance string [System.Runtime]System.Object::ToString()
        ret
    }
}

.class interface private auto ansi abstract beforefieldinit I1`1<valuetype .ctor ([System.Runtime]System.ValueType) T>
{
}

.class interface private auto ansi abstract beforefieldinit I2`1<W>
    implements class I1`1<!W>
{
} 

.class private sequential ansi sealed beforefieldinit S
    extends [System.Runtime]System.ValueType
{
    .pack 0
    .size 1

}
fanyang-mono commented 6 months ago

cc/ @lambdageek

lambdageek commented 6 months ago

We need to be careful in two ways:

  1. that we don't introduce a check that can get into a cycle when the interface constraints are cyclic (interface I<T> where T : I<T> scenarios)
  2. that we don't introduce a check that needs to eagerly initialize a ton of interfaces at startup (for example INumber<T>, IBinaryNumber and friends on the basic integer types)

My feeling is that checking the constraints in mono_class_init_internal (the same as for generic instance parent/child classes) is going to be a bit too eager. Maybe we can push it off until a vtable is needed for some class and then check all the interfaces that it implements.

steveisok commented 1 month ago

Moving to future and we should revisit if C# expands usage of ref structs.