Fix direction of variance check in delegate assignment

[jcouv]

I found this problem while debugging the integration of ILVerify into Roslyn. I copied the involved C# code below, but essentially, the usage of delegate returned by Bar (which is a Func<int, string>) in a Func<int, object>.
ILVerify previously thought that is illegal because it treated the return type string as the destination and object as the source for an assignment. But, actually, string is the source, and object is the destination.
The arguments are also in the wrong order (although in the other order).

I'm happy to add a unittest, but was unable to run tests so far.
In Test Explorer, I get Message: System.InvalidOperationException : No data found for ILVerify.Tests.ILMethodTester.TestMethodsWithInvalidIL.
I've compiled some IL files in repos\corert\src\ILVerify\tests\ILTests (for example, with ilasm AccessTests.il /dll /ERROR), but the test runner doesn't detect the resulting assembly somehow. Not debugged yet.

Also, I'm setting the project to target AnyCPU, which helps for integration with Roslyn.

using System;
using System.Collections.Generic;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

class TestCase
{
    static int test = 0;
    static int count = 0;

    public static async Task Run()
    {
        try
        {
            test++;
            var f = new Func<int, object>(checked(await Bar()));
            var x = f(1);
            if ((string)x != ""1"")
                count--;
        }
        finally
        {
            Driver.Result = test - count;
            Driver.CompleteSignal.Set();
        }
    }
    static async Task<Converter<int, string>> Bar()
    {
        count++;
        await Task.Delay(1);

        return delegate(int p1) { return p1.ToString(); };
    }
}

class Driver
{
    static public AutoResetEvent CompleteSignal = new AutoResetEvent(false);
    public static int Result = -1;
    public static void Main()
    {
        TestCase.Run();
        CompleteSignal.WaitOne();

        Console.Write(Result);
    }
}

Tagging @jkotas @ArztSamuel @VSadov

[ArztSamuel]

In Test Explorer, I get Message: System.InvalidOperationException : No data found for ILVerify.Tests.ILMethodTester.TestMethodsWithInvalidIL.

Have you rebuilt the solution after compiling the il files?

I completely agree with your changes.

I think the reason why I implemented it this way around is because of how this relation is defined in ECMA:
(II.14.6.1 Delegate signature compatibility)

A target method or delegate of type T is delegate-assignable-to a delegate of type D if
[...]
3. For each parameter type U of T, [...], and corresponding type V of D, U is assignable-to V.
4. The return type U of T and return type V of D, V is assignable-to U.

The target method T would be string(int) and the delegate D would be object(int) in this case, right? Hence according to the fourth rule these methods should not be delegate-assignable-to, since object is not assignable to string.

Is ECMA wrong here, or am I misunderstanding something?

[jcouv]

@ArztSamuel The ECMA spec does seem backwards. @VSadov @gafter FYI
Added a follow-up item to umbrella issue. Not yet sure what to do about such spec bugs.

Thanks, I'm now able to run some tests. I have 2 failures on AccessTests though. Does that repro for you?

[jcouv]

Added tests :-)

[ArztSamuel]

I have 2 failures on AccessTests though. Does that repro for you?

All tests should be passing. Could you specify the names of the failing tests?
Could it be that you still have a dll from another branch?

Edit: I think you probably didn't compile the other AccessTest assemblies (i.e. AccessTestsExtern and AccessTestsFriend). These are required for the friend / non-friend access tests.
All other tests are self-contained.

[ArztSamuel]

Just a nit, but doesn't this require the this pointer to be loaded first?

It's not really a problem, since this method is not verified anyway, but I think it is still good practice to keep the non-invalid IL code valid in these test files.

[jcouv]

Yes, looks like I mangled it. Thanks :-)

By the way, I'd love to sync up (email or skype) at some point, so that we're not duplicating efforts. Would you mind contacting me ([email protected])? Cheers

[ArztSamuel]

Sure, I'll send you a message.

[VSadov]

Technically the types should be "Compatible". In the spec "Compatible" is more strict than "Assignable", since "Assignable" allows some reinterpretations. I.E. an int32 is Assignable to int8 - one can be assigned to another with implicit value truncation.
However int32 and int8 are not "Compatible" and therefore int M1() cannot be used as Func<byte>

I am not sure what IsAssignable implements. It could as well implement "IsCompatible", then everything is correct.
Perhaps it is worth adding a test while touching this?

[jkotas]

I am sorry - I have not noticed that you have commented on this before merging. I agree it would be nice to add test for this case.

[jcouv]

Filed follow-up issue.

[VSadov]

I would not block merging on this. Just a good thing to have.

[VSadov]

The ECMA spec indeed has a bug in delegate-assignable-to definition.
The directions of required compatibility between parameters and between returns are reversed.