9

Using xUnit's Assert.Throws, I stumbled upon this (to me) hard to explain overload resolution issue. In xUnit, this method is marked obsolete:

[Obsolete("You must call Assert.ThrowsAsync<T> (and await the result) " +
           "when testing async code.", true)]        
public static T Throws<T>(Func<Task> testCode) where T : Exception 
{ throw new NotImplementedException(); }

The question is, why does an inline statement lambda (or expression) that simply throws an exception resolve to this overload (and therefore doesn't compile)?

using System;
using Xunit;
class Program
{
    static void Main(string[] args)
    {
        // this compiles (of course)
        // resolves into the overload accepting an `Action`
        Assert.Throws<Exception>(() => ThrowException());
        // line below gives error CS0619 'Assert.Throws<T>(Func<Task>)' is obsolete: 
        // 'You must call Assert.ThrowsAsync<T> (and await the result) when testing async code.'    
        Assert.Throws<Exception>(() => { throw new Exception(); });
    }

    static void ThrowException()
    {
        throw new Exception("Some message");
    }
}
jeroenh
  • 26,362
  • 10
  • 73
  • 104
  • 2
    what's the overload that this resolves into: `Assert.Throws(() => ThrowException());` – Selman Genç Aug 17 '18 at 08:48
  • Possibly related: https://stackoverflow.com/questions/20395826/explicitly-use-a-functask-for-asynchronous-lambda-function-when-action-overloa#20395969 – Stefan Aug 17 '18 at 08:49
  • @SelmanGenç the overload accepting an `Action` – jeroenh Aug 17 '18 at 08:51
  • 1
    a lambda that only throws could have *any* possible return type. I'm not surprised the system's confused. – Damien_The_Unbeliever Aug 17 '18 at 08:52
  • Are you sure the first line does not cause a warning, too?. The `ObsoleteAttribute` does not differentiate, so both lines should yield a warning. – Sefe Aug 17 '18 at 08:55
  • @Sefe no warning on my machine. It resolves to a different overload. – jeroenh Aug 17 '18 at 08:58
  • `Func` does take precedence over `Action` when resolving overloads. I remember seeing an answer to a similar question by @EricLippert, couldn't find it right away but it's out there. – noseratio Aug 17 '18 at 09:18
  • @Noseratio This is true for async functions. See here: https://github.com/dotnet/csharplang/blob/master/spec/expressions.md#inferred-return-type – FCin Aug 17 '18 at 09:31
  • @FCin, not only for async functions: https://stackoverflow.com/a/20418228/1768303 – noseratio Aug 17 '18 at 09:48

2 Answers2

4

I was able to reproduce this, given the function declarations:

static void CallFunction(Action action) { }

static void CallFunction(Func<Task> func) { }

And calling them:

CallFunction(() => ThrowException());
CallFunction(() => { throw new Exception(); });

The second one resolves into CallFunction(Func<Task> func) overload. The weird thing is if I change the body like this:

CallFunction(() => { int x = 1; });

It resolves to CallFunction(Action action) overload.

If the last statement in the body is a throw statement, I guess the compiler thinks method is returning something, and picks the closest -more specific- overload to that scenario which is Func<Task>.

The closest thing I found in the documentation is this:

7.5.2.12 Inferred return type

• If F is async and the body of F is either an expression classified as nothing (§7.1), or a statement block where no return statements have expressions, the inferred return type is System.Threading.Tasks.Task

The function here is a statement block, but it's not async though. Note that I'm not saying this exact rule applies here. Still I'm guessing it's related to this.

This article from Eric Lippert explains it better. (thanks for the comment @Damien_The_Unbeliever).

Selman Genç
  • 100,147
  • 13
  • 119
  • 184
  • 1
    The question is why it thinks it is returning something. – FCin Aug 17 '18 at 09:01
  • @FCin I know, I don't have a definitive answer for that, I'm looking into the specs to find something. – Selman Genç Aug 17 '18 at 09:09
  • @FCin - because a method that only throws is a *valid* implementation for every possible return type (and `void`) – Damien_The_Unbeliever Aug 17 '18 at 09:11
  • `() => { int x = 1; }` calls 1. `() => { int x = 1; throw new Exception(); }` calls 2. `() => { int x = 1; throw new Exception(); return; }` and `() => { throw new Exception(); return; }`call 1. – bolov Aug 17 '18 at 09:13
  • @Damien_The_Unbeliever So why didn't it choose `Action` over `Func`? Second question, shouldn't it print an error, because of ambigous overload? – FCin Aug 17 '18 at 09:15
  • 4
    @FCin - there is a reason, and I think it's to do with `Func` being a *better* match that `Action` if it can (as here) choose between them. I swear Eric Lippert has written about this. I found the start here - [Never say Never, part one](https://blogs.msdn.microsoft.com/ericlippert/2011/02/21/never-say-never-part-one/) – Damien_The_Unbeliever Aug 17 '18 at 09:20
  • I have added the closest explanation to this in docs. still, not the exact explanation but must be related to this somehow. – Selman Genç Aug 17 '18 at 09:26
  • @Damien_The_Unbeliever that article is on point, added to the answer. – Selman Genç Aug 17 '18 at 09:39
  • 1
    I've gone into more detail in my answer. The async part is an unfortunate red herring. Eric's article gives the explanation for why the conversion to `Func<...>` is valid - my answer explains that and also why it's a *better* conversion than the conversion to `Action`. – Jon Skeet Aug 17 '18 at 13:39
  • 2
    Basically the choice made by the designers here is that being compatible with a method that says it can return *something of some type* is "more specific" than a method which returns nothing at all. This is a pretty arbitrary choice! There's an argument to be made that we know the *exact behaviour* on return of a void method -- it returns nothing -- and we don't know the exact behaviour of a non-void method on return, because it can return any value of its type, and so the void method is more specific. Ultimately it's a judgment call by the designers. – Eric Lippert Aug 17 '18 at 17:40
3

Here's a complete example which doesn't involve Task, to remove any hint of asynchrony being involved:

using System;

class Program
{
    static void Method(Action action)
    {
        Console.WriteLine("Action");
    }

    static void Method(Func<int> func)
    {
        Console.WriteLine("Func<int>");
    }

    static void ThrowException()
    {
        throw new Exception();
    }

    static void Main()
    {
        // Resolvse to the Action overload
        Method(() => ThrowException());
        // Resolves to the Func<int> overload
        Method(() => { throw new Exception(); });        
    }
}

Using section numbering from ECMA 334 (5th edition), we're interested in section 12.6.4 - overload resolution. The two important steps are:

  • Identify applicable methods (12.6.4.2)
  • Identify the best method (12.6.4.3)

We'll look at each call in turn

Call 1: () => ThrowException()

Let's start with the first call, which has an argument of () => ThrowException(). To check for applicability, we need a conversion from that argument to either Action or Func<int>. We can check that without involving overloading at all:

// Fine
Action action = () => ThrowException();
// Fails to compile:
// error CS0029: Cannot implicitly convert type 'void' to 'int'
// error CS1662: Cannot convert lambda expression to intended delegate type because 
// some of the return types in the block are not implicitly convertible to the 
// delegate return type
Func<int> func = () => ThrowException();

The CS1662 error is a little unfortunately worded in this case - it's not that there's a return type in the block that's not implicitly convertible to the delegate return type, it's that there isn't a return type in the lambda expression at all. The spec way of preventing this is in section 11.7.1. None of the permitted conversions there work. The closest is this (where F is the lambda expression and D is Func<int>):

If the body of F is an expression, and either F is non-async and D has a non-void return type T, or F is async and D has a return type Task<T>, then when each parameter of F is given the type of the corresponding parameter in D, the body of F is a valid expression (w.r.t §12) that is implicitly convertible to T.

In this case the expression ThrowException is not implicitly convertible to int, hence the error.

All of this means that only the first method is applicable for () => ThrowException(). Our pick for "best function member" is really easy when the set of applicable function members only has a single entry...

Call 2: () => { throw new Exception(); }

Now let's look at the second call, which has () => { throw new Exception(); } as the argument. Let's try the same conversions:

// Fine
Action action = () => { throw new Exception(); };
// Fine
Func<int> func = () => { throw new Exception(); };

Both conversions work here. The latter one works because of this bullet from 11.7.1:

If the body of F is a statement block, and either F is non-async and D has a non-void return type T, or F is async and D has a return type Task<T>, then when each parameter of F is given the type of the corresponding parameter in D, the body of F is a valid statement block (w.r.t §13.3) with a nonreachable end point in which each return statement specifies an expression that is implicitly convertible to T.

I realize it sounds odd that this works, but:

  • The end point of the block is not reachable
  • There are no return statements, so the condition of "each return statement specifies [...]" is indeed met

To put it another way: you could use that block as the body of a method that's declared to return int.

That means both our methods are applicable in this case.

So which is better?

Now we need to look at section 12.6.4.3 to work out which method will actually be picked.

There are lots of rules here, but the one that decides things here is the conversion from the lambda expression to either Action or Func<int>. That's resolved in 12.6.4.4 (better conversion from expression):

Given an implicit conversion C1 that converts from an expression E to a type T1, and an implicit conversion C2 that converts from an expression E to a type T2, C1 is a better conversion than C2 if at least one of the following holds:

  • ...
  • E is an anonymous function, T1 is either a delegate type D1 or an expression tree type Expression<D1>, T2 is either a delegate type D2 or an expression tree type Expression<D2> and one of the following holds:
    • D1 is a better conversion target than D2
    • D1 and D2 have identical parameter lists, and one of the following holds:
      • D1 has a return type Y1, and D2 has a return type Y2, an inferred return type X exists for E in the context of that parameter list (§12.6.3.13), and the conversion from X to Y1 is better than the conversion from X to Y2
      • E is async [... - skipped because it's not]
      • D1 has a return type Y, and D2 is void returning

The part I've put in bold is the important bit. When you consider the following scenario:

  • E is () => { throw new Exception(); }
  • T1 is Func<int> (so D1 is Func<int> too)
  • T2 is Action (so D2 is Action too)

... then both D1 and D2 have empty parameter lists, but D1 has a return type int, and D2 is void returning.

Therefore the conversion to Func<int> is better than the conversion to Action... which means that Method(Action) is a better function member than Member(Func<int>) for the second call.

Phew! Don't you just love overload resolution?

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194