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

[metadata] Fields whose types are gparams with a reference type const… #1211

Merged
merged 1 commit into from
Aug 7, 2019

Conversation

UnityAlex
Copy link
Collaborator

@UnityAlex UnityAlex commented Jul 30, 2019

…raint aren't blittlable. (mono#15761)

  • [metadata] Fields whose types are gparams with a reference type constraint
    aren't blittlable.
    Don't try to layout the field to find out if it's blittable.
    For gshared gparams, follow the blittability of the constraint.

Fixes certain recursive examples.

using System;

namespace TestRecursiveType
{
    class Program
    {
        static void Main(string[] args)
        {
            SubClass subC = new SubClass();
            Console.WriteLine(subC.GetTest());
        }
    }

    public struct ValueTest<U>
    {
        // When U is instantiated with T, from BaseClass, we know it'll be a
	// reference field, so we know the instantiation ValueTest<T> won't
	// be blittable.
        public readonly U value;
    }

    public abstract class BaseClass<T> where T : BaseClass<T>
    {
        public ValueTest<T> valueTest = default(ValueTest<T>);
    }

    public class SubClass : BaseClass<SubClass>
    {
        private String test = "test";

        public string GetTest()
        {
            return test;
        }
    }
}

Fixes mono#15760
Unity Fixes https://fogbugz.unity3d.com/f/cases/1147579/
Unity Release Note: Fixes issue where recursive type definitions would fail to compile successfully


The failure is happening when we are doing mono_class_setup_fields ("BaseClass") which needs to decide for each field whether it is blittable or not. So therefore we are trying to decide if ValueTest (that is: the ValueTest field inside BaseClass) is blittable or not.

So we instantiate U with T.
Now to decide whether VaueTest is blittable or not, we look at every field.
So then we look at T value.
To decide if T is blittable we first check if it's a reference type.

That check is currently inadequate for generic parameters - what the PR adds is the ability to see if theres a T : class constraint or a T : C constraint - where C is some class. As soon as we know that T's constraint will force it to be a reference type we can definitely say that T won't be blittable without having to initialize C, at all.

Previously, Mono would see that T is some kind of type for which it couldn't definitively decide that it's a reference type and it would call: mono_class_setup_fields (field_class) which would then try to setup the fields of the parent class BaseClass. And that would hit the recursion check.

Unity cherry-pick note: Needed to bring MONO_CLASS_IS_INTERFACE_INTERNAL
and mono_class_get_valuetype_class forward

…raint aren't blittlable. (mono#15761)

* [metadata] Fields whose types are gparams with a reference type constraint
aren't blittlable.
Don't try to layout the field to find out if it's blittable.
For gshared gparams, follow the blittability of the constraint.

Fixes certain recursive examples.

```
using System;

namespace TestRecursiveType
{
    class Program
    {
        static void Main(string[] args)
        {
            SubClass subC = new SubClass();
            Console.WriteLine(subC.GetTest());
        }
    }

    public struct ValueTest<U>
    {
        // When U is instantiated with T, from BaseClass, we know it'll be a
	// reference field, so we know the instantiation ValueTest<T> won't
	// be blittable.
        public readonly U value;
    }

    public abstract class BaseClass<T> where T : BaseClass<T>
    {
        public ValueTest<T> valueTest = default(ValueTest<T>);
    }

    public class SubClass : BaseClass<SubClass>
    {
        private String test = "test";

        public string GetTest()
        {
            return test;
        }
    }
}
```

Fixes mono#15760

---

The failure is happening when we are doing mono_class_setup_fields ("BaseClass<T>") which needs to decide for each field whether it is blittable or not. So therefore we are trying to decide if ValueTest<T> (that is: the ValueTest<U> field inside BaseClass<T>) is blittable or not.

So we instantiate U with T.
Now to decide whether VaueTest<T> is blittable or not, we look at every field.
So then we look at T value.
To decide if T is blittable we first check if it's a reference type.

That check is currently inadequate for generic parameters - what the PR adds is the ability to see if theres a T : class constraint or a T : C constraint - where C is some class. As soon as we know that T's constraint will force it to be a reference type we can definitely say that T won't be blittable without having to initialize C, at all.

Previously, Mono would see that T is some kind of type for which it couldn't definitively decide that it's a reference type and it would call: mono_class_setup_fields (field_class) which would then try to setup the fields of the parent class BaseClass<T>. And that would hit the recursion check.

Unity cherry-pick note: Needed to bring MONO_CLASS_IS_INTERFACE_INTERNAL
and mono_class_get_valuetype_class forward
@UnityAlex UnityAlex requested a review from joncham July 30, 2019 19:29
@UnityAlex
Copy link
Collaborator Author

Would probably be a good idea to target this change for 2019.4 since it's a fairly large change for a fairly corner-casey issue.

@UnityAlex UnityAlex merged commit 3af2b9f into unity-master Aug 7, 2019
@UnityAlex UnityAlex deleted the fix-recursive-type-exception branch August 7, 2019 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeLoadException thrown for recursive type that .NET allows
3 participants