Not all code paths return, but compiler treats it as if all paths return

Question

I can't think of a good title, but my question is not as naive as it appears.

Consider this:

public static void ExitApp(string message)
{
    // Do stuff
    throw new Exception(...);
}

OR

public static void ExitApp(string message)
{
    // Do stuff
    System.Environment.Exit(-1);
}

Neither of these methods will ever return. But when you invoke these methods elsewhere:

public int DoStuff()
{
    // Do stuff
    if (foo == 0)
    {
        throw new Exception(...);
    }
    else if (foo == 1)
    {
        // Do other stuff
        return ...;
    }
    else
    {
        ExitApp("Something borked");
    }
}

Try to compile that and you will get a "not all code paths return a value" in DoStuff. It seems silly to trail the call to ExitApp with an Exception just to satisfy the compiler even though I know that it's good. There seems to be nothing in ExitApp() that I can do to indicate it will never return.

How can I indicate to the compiler that ExitApp never returns and, thus, that DoStuff's else block will never return either? It seems like a rather simple bug that it's path checking fails to account for.

Even if I only use the first ExitApp (throws the exception) and that method returns an int the path checker is smart enough to realize that it will never return so it doesn't complain about the int type. This compiles file:

public static int ExitApp(string message)
{
    // Do stuff
    throw new Exception(...);
}

However, given that it knows this ExitApp will never return an int it does not extrapolate that to DoStuff() so I'm inclined to believe there is no solution to my question. My only choice is to throw an exception after calling ExitApp.

public int DoStuff()
{
    ...
    else
    {
        ExitApp("Something borked");
        throw new NotImplementedException("Should not reach this");
    }
}

Is there a reason for this behavior by the compiler?

Answer 1

The C# compiler doesn't support exception reporting, like the Java compiler does. Because of this, the compiler doesn't know (outside of the context of the method itself) that the method is guaranteed to throw an exception on every invocation.

Answer 2

I have an exception defined for this purpose: UnreachableException. It might seem superfluous, but it's an easy way to say "Hey, person reading this, this line should never be executed!". I usually use it for the default case of some switch statements, but it applies here as well.

Just throw one after the ExitApp line.

public void int DoStuff()
{
    // Do stuff
    if (foo == 0)
    {
        throw new Exception(...);
    }
    else if (foo == 1)
    {
        // Do other stuff
        return ...;
    }
    else
    {
        ExitApp("Something borked");
        throw new UnreachableException();
    }
}

The actual reason the language doesn't support declaring a method which always throws is just: it's not worth it. The language developers don't have unlimited time to apply every feature we can think of. They have to prioritize.

I'm betting this is the first time you've run into this situation, and look: explicitly throwing an exception deals with the issue. Why would they bother dealing with such a rare, easy to bypass case? They could be spending that time implementing optional parameters, dynamic, or a bunch of other things that will be more useful and used more often than being able to say a function always throws an exception.

That's not to say it will never be implemented. This type of method information is exactly the type of thing contracts are great at specifying. So maybe it will be included with code contracts.

Answer 3

For the second case, System.Environment.Exit is part of the framework, not the C# language. The C# compiler doesn't "know" it's a non-returning function.

This is unfortunate. Visual C++ supports a __declspec(noreturn), but I don't know of any C# like construct. I typically put a comment saying "Unreachable code" and an assert in these cases, and put it a return or a throw to make the compiler happy.

Answer 4

Yes a very good reason.

It means that you can make changes to ExitApp's implementation without suddenly having compiler errors turning up all over your application.

Answer 5

C# choose not to implement checked exceptions, so although the compiler could choose to do the static analysis and figure out that ExitApp or MethodThrows never returns, the team spent it's energy elsewhere.

Answer 6

The compiler has no way of knowing that the behavior of System.Environment.Exit() causes it to not return.

It is complaining because it assumes the function will return and execution will continue.

Just add a simple return -1; statement after the Exit() call.

Answer 7

The simple answer is: the compiler is not that clever. It makes the basic assumption that any method call within a certain method (DoStuff in your case) will eventually complete in the general case (although it may sometimes throw an assumption) - a very fair one in general, though clearly not every case as you point out. Saying that, I don't however see it as a problem that you have to add a line of code after the call to ExitApp. Either of the following would do the job fine and should never get called:

return 0;

// More than necessary, but potentially useful for spotting an undersirable case when your called method *does* return (undesirably in this situation).
throw new Exception("This should never happen because the previous call should never return");

Answer 8

The compiler has no consistent way to infer what your ExitApp() method will do. Although it could analyse the source code and "guess" that it will never return there could just as easily be cases where the source isn't available and it wouldn't know. The only reasonable and consistent approach is for it not to analyse your code.

Because of this you've either got to add an exception or dummy return value.

Answer 9

I'm not going to repeat what's already been said here before, but if you're looking for a way such that the compiler will not bother you in regards to this error, you could do the following:

public int DoStuff()
{
    var result = 0; //put default result here
    // Do stuff
    if (foo == 0)
    {
        throw new Exception(...);
    }
    else if (foo == 1)
    {
        // Do other stuff
        result = ...;
    }
    else
    {
        ExitApp("Something borked");
    }
    return result;
}

The fact of the matter is that the compiler just isn't smart enough to understand in your case that all paths return values, so its better to return the value outside of your conditional structure.