Why does the compiler complain that 'not all code paths return a value' when I can clearly see that they do?

Question

I'm trying to figure out why the compiler has a problem with this function. It gives me the "Not all code paths return a value" error, however I cannot see a situation where control-flow would pass to the if( a ) expression without a being true (so the if( a ) is superfluous, but the compiler doesn't seem to recognize that).

public static Boolean Foo(Boolean x)
{
    Boolean a = false;
    if( x )
    {
        a = true;
    }
    else
    {
        try
        {
            SomethingThatMightThrow();
            Assert.IsFalse( a );
            return a;
        }
        catch(Exception)
        {
            a = true;
        }
    }

    if( a )
    {
        return x;
    }
}

The immediate fix is to simply remove the if( a ) guard statement completely and just return x immediately - but why does the compiler complain even though it should be able to statically prove all possible code paths will hit a return statement? Crucially, there are no loops, which are often the main reason for it failing to prove return-ness.

I'm using VS2015 Update 3.

Answer 1

There is a supported scenario were a is false when you reach the end of your function. That scenario is when you're debugging your code and use your debugger to set a to false.

The C# compiler rules are by design simple. In languages that C# borrows from, the problem of functions potentially not returning anything was a problem that could not be covered by compiler warnings. There were too many false positives, therefore warnings were tweaked to only warn about the obvious cases, introducing false negatives. C#'s rules are a compromise where false positives are acceptable if they're understandable to a human familiar with the rules, and false negatives are unacceptable. You're guaranteed that if your function has a code path which doesn't return a value, the compiler detects it.

One part of those simple rules is that the values of variables aren't considered. Even if a is statically guaranteed to be true, the compiler by design cannot make use of that fact.

@PetSerAl already quoted the relevant wording in the C# language specification:

8.1 End points and reachability

[...]

To determine whether a particular statement or end point is reachable, the compiler performs flow analysis according to the reachability rules defined for each statement. The flow analysis takes into account the values of constant expressions (§7.19) that control the behavior of statements, but the possible values of non-constant expressions are not considered. In other words, for purposes of control flow analysis, a non-constant expression of a given type is considered to have any possible value of that type.

This is part of the last currently published language spec, the one for C# 5.0. The version you're using, C# 6.0 (that's what VS2015 offers), doesn't have a published spec yet, so it's possible that the wording will be slightly different, but as your compiler has shown, effectively the same rule still applies.

Answer 2

It's run-time vs. compile-time

Your example is far too complicated. This won't compile either:

static int Test()
{
    bool f = true;
    if (f)
    {
        return 1;
    }
    else
    {
        //Not all code paths return a value
    }
}

On the other hand, this will:

static int Test()
{
    if (true)
    {
        return 1;
    }
    else
    {
        //No error
    }
}

I'm guessing that whatever validation mechanisms are in place do not have sufficient logic to infer the contents of a run-time variable. Compile-time variables are no problem.

Answer 3

I think the compiler does a very simple analysis on the code and thus the return must be explicitly given.

This might look like a wrong decision, but when dealing with complex code, returned value might not be clear. So, the programmer is forced to return it.

Your example can be reduced to a minimum like this:

public static Int32 Main(String[] args)
{
    var printUsage = true;
    if (printUsage)
    {
        return 0;
    }

    // return nothing, so compiler is not happy
}

while still getting the error.

NOTE: if you use Resharper, it will perform the analysis you want and warn you accordingly:

if (printUsage)         // Warning: expression is always true