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

Consider changing NoReturn to not be a bottom type #4116

Closed
Wilfred opened this issue Oct 13, 2017 · 8 comments
Closed

Consider changing NoReturn to not be a bottom type #4116

Wilfred opened this issue Oct 13, 2017 · 8 comments

Comments

@Wilfred
Copy link

Wilfred commented Oct 13, 2017

NoReturn being a bottom type leads to mypy accepting code that I would consider to be invalid.

For example, saving the result of a NoReturn function to a variable:

import sys
from typing import List
from mypy_extensions import NoReturn

def never_returns() -> NoReturn:
    sys.exit(1)

def foo() -> None:
    x = [] # type: List[int]
    x = never_returns()

If mypy infers a type of NoReturn on my function body, it seems that I can write anything as the return type without getting a type error:

def foo1() -> dict:
    raise Exception('oh dear')

def foo2() -> list:
    sys.exit(1)

I'm struggling to think of any example where NoReturn being a bottom type allows me to do things that I couldn't otherwise do.

What's the rationale? Would you be open to changing this?

Originally discussed in #4066.

@emmatyping
Copy link
Collaborator

I think it makes sense to keep it as is, but perhaps emit a warning if you assign eg never_returns() to a variable, as that obviously means you are making a mistake.

@ilevkivskyi
Copy link
Member

Use case is practically every function. Assume that NoReturn is not a subtype of every type, then this is invalid:

def div(a: float, b:float) -> float:
    if b == 0:
        raise ZeroDivisionError
    else:
        return a / b

it should be:

def div(a: float, b:float) -> Union[float, NoReturn]:
    ...

And then this will propagate to all callers:

def inv(x: float) -> float:
    return div(1, x)  # Incompatible return, expected 'float', got 'Union[float, NoReturn]'

And if you take into account that practically every built-in and stdlib function can raise an exception for various reasons (index out of range, value less than zero, etc.), this means that every user function should be typed as returning Union[..., NoReturn], so no, I am not open to changing this. Something related was discussed in python/typing#71 and #1773 and as you can see most people are not enthusiastic about this.

@ethanhs I am OK with emitting a warning but it is not as simple to implement as it may seem, there are lots of cases to consider, unpacking a, b = c, chained assignments a = b = c, multi-assignment from union, explicit vs inferred type etc., and all of these can be combined. It just doesn't seem worth the effort.

@gvanrossum
Copy link
Member

Hm. We're using NoReturn because it's useful to state that a function never returns. We don't try to track whether a function may not return. So I'm not sure I buy this particular argument. (Though I still think NoReturn is a subtype of every type -- otherwise our type algebra just becomes too complicated.)

@emmatyping
Copy link
Collaborator

@ilevkivskyi this is true, I did not mean to make it seem like the changes for that are simple. I'm not particularly tied to it either, just something that might be useful.

I think we are agreed that NoReturn should remain as-is, so I am going to close this.

@Wilfred
Copy link
Author

Wilfred commented Oct 16, 2017

OK, that makes sense. Looking at other type systems that have a divergent type (e.g. Rust), they do exhibit the same behaviour of treating NoReturn as a bottom type. I wasn't advocating for a checked exception system.

Is it worth opening an issue for assigning a value from a NoReturn value? That would certainly fix the issue raised in my first example.

@ilevkivskyi
Copy link
Member

@Wilfred

Is it worth opening an issue for assigning a value from a NoReturn value? That would certainly fix the issue raised in my first example.

I think you can open an issue (at least to keep this idea on record), but it would be quite low priority.

@Wilfred
Copy link
Author

Wilfred commented Oct 30, 2017

Opened #4179.

@mthuurne
Copy link
Contributor

mthuurne commented Jun 9, 2020

Hm. We're using NoReturn because it's useful to state that a function never returns. We don't try to track whether a function may not return. So I'm not sure I buy this particular argument. (Though I still think NoReturn is a subtype of every type -- otherwise our type algebra just becomes too complicated.)

Probably the type algebra does need a bottom type, but I'm not convinced that typing.NoReturn should be equal to the bottom type.

Let's consider the following code:

from typing import NoReturn

def f() -> NoReturn:
    while True:
        pass

x = f() * 2

Because NoReturn is currently the bottom type, mypy considers this code correct. And from a mathematical point of view, it is. But from a user's perspective, it would be better if mypy considered the use of f's return value an error.

This example also shows that any use of the return value of f should be considered incorrect; the problem in an assignment would be in the evaluation of the right-hand side, not in the assigning itself.

Now let's introduce two new types: Bottom, which is an explicit bottom type, and NoValue, which is used to annotate the absence of a value. Then we could annotate f as follows:

def f() -> Union[NoValue, Bottom]:

And mypy would report:

Unsupported operand types for * ("NoValue" and "int")

Rename NoValue to NoReturn and make the union with the bottom type implicit and we end up with a situation where mathematics and user expectations align.

So I think it makes sense to make NoReturn a type with no values which does not inherit from anything, not even from object. The bottom type would continue to function as it does today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants