Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fanyang-mono opened this issue Mar 15, 2024 · 3 comments

Comments

@fanyang-mono
Copy link
Member

fanyang-mono commented Mar 15, 2024

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 fanyang-mono added this to the Future milestone Mar 15, 2024
@fanyang-mono
Copy link
Member Author

cc/ @lambdageek

@lambdageek
Copy link
Member

lambdageek commented Mar 15, 2024

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 steveisok modified the milestones: 9.0.0, Future Aug 5, 2024
@steveisok
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants