Skip to content

Type checking stops without error messages after uninhabited type detected #3154

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

Closed
pkch opened this issue Apr 11, 2017 · 14 comments
Closed

Comments

@pkch
Copy link
Contributor

pkch commented Apr 11, 2017

Under some conditions (it seems the trigger is that generic method has arguments without type hints), a spurious uninhabited type appears:

T = TypeVar('T')
class X:
    def f(self) -> T: ...

C = TypeVar('C', bound=X)

def g(cls: Type[C]) -> None:
    reveal_type(cls()) # 'C`-1'
    reveal_type(cls().f) # 'def [T] () -> T`9'
    reveal_type(cls().f()) # '<uninhabited>'
    x: int = 'a' # no error because mypy thinks it's unreachable

The first revealed type is correct; the second I think is wrong; the third is definitely wrong. (Of course, the real problem is that any code after cls().f() is not type checked.)

The output is unaffected by the presence of --strict flag.

This problem disappears if method f is redefined as

class X:
    def f(self: T) -> T: ...
@pkch
Copy link
Contributor Author

pkch commented Apr 11, 2017

My bad, the fact that reveal_type(cls().f()) is <uninhabited> is correct. Since the generic method f only has return value typed, not its arguments, it will return something that will be compatible with anything; so the only thing it can return is bottom, i.e., uninhabited.

So the only issue is that once mypy detects this uninhabited type, it considers the rest of the current scope unreachable, and stops type checking.

Specifically, in the example below, the user gets no errors at all:

T = TypeVar('T')
class X:
    def f(self) -> T: ...

C = TypeVar('C', bound=X)

def g(cls: Type[C]) -> None:
    v = cls().f()
    x: int = 'a' # no error

@pkch pkch changed the title Generic method return type is revealed as uninhabited Type checking stops without error messages after uninhabited type detected Apr 11, 2017
@gvanrossum
Copy link
Member

That sounds like the binder has discovered some dead code for you. :-)

Agreed it's disconcerting that everything is silent -- that's a general concern I have about mypy's current approach to dead code (after assert False you can do 1 + '' with impunity, which is considered a feature in some situations, but not in others).

@pkch
Copy link
Contributor Author

pkch commented Apr 11, 2017

But in this case, I'd argue the code isn't dead, in the sense that it's definitely going to be reached in runtime.

@gvanrossum
Copy link
Member

But if mypy infers that a value of type "uninhabited" is returned somewhere, you can't blame it for thinking that code is unreachable, because (in the type system) there are no such values.

I think the def f(self) -> T: line should be flagged as an error though -- @sixolet and I walked through the idea of functions with type variables only in the return type recently and we decided that this particular case should be disallowed.

(Honestly, I think you still haven't quite got the hang of type variables. There really are only three places where they are currently valid: in generic classes, generic functions, and generic type aliases. Your example is neither, and things like def f() -> Callable[[T], T] are not yet implemented. See #3113 though.)

@pkch
Copy link
Contributor Author

pkch commented Apr 11, 2017

But if mypy infers that a value of type "uninhabited" is returned somewhere, you can't blame it for thinking that code is unreachable, because (in the type system) there are no such values

That would be the case if the type of cls().f() was really constrained to be <uninhabited>. Actually, it is not constrained at all: it can be anything from <uninhabited> all the way up to <object>. Given that unconstrained range, mypy arbitrarily chooses to reveal the lowest possible type from that range; but that's not a reason to conclude that the code after that line is unreachable.

In fact, in a similar situation with module-level functions, mypy reveals None (while not strictly correct, it at least doesn't cause the rest of the code to become unreachable):

T = TypeVar('T')
def f() -> T: ...

x = f()
# the code here reachable
reveal_type(x)  # Revealed type is 'builtins.None'

I think the def f(self) -> T: line should be flagged as an error though

I agree (the entire example I presented was a result of an accidental omission of a type variable for an argument my part; I was surprised mypy said nothing about it). I think the situations where some type variable can never be constrained are a sign of a programmer mistake. Since @sixolet is already working on the PR for that, I don't think there's a need to create an issue for it.

There really are only three places where type variables are currently valid: in generic classes, generic functions, and generic type aliases. Your example is neither.

As I read PEP 484 it seems to say that this is syntactically a valid generic function:

A type variable used in a method that does not match any of the variables that parameterize the class makes this method a generic function in that variable.

Of course, semantically, mypy is free to figure out that this annotation is meaningless, and report an error. But I don't think it's easy to disallow this structure just based on the syntax (e.g., we can't say that a generic function must have type parameters in its arguments, since that will change after #3113 is merged).

@ilevkivskyi
Copy link
Member

Just wanted to note that we already have #2885 for problems with def f() -> T: ... Not sure if this is a duplicate.

@pkch
Copy link
Contributor Author

pkch commented Apr 11, 2017

If #2885 is resolved, my particular example will no longer cause any problems.

I'd still prefer to avoid the combination of these two behaviors:

  1. <uninhabited> type is generated when mypy finds a type variable that cannot be constrained;

  2. mypy assumes that the code after the appearance of <uninhabited> is unreachable

But I think it's best to close this for now, since it would be hard to justify spending time on this without a specific problem to solve.

@pkch pkch closed this as completed Apr 11, 2017
@gvanrossum
Copy link
Member

@ddfisher ^^^ is this an intentional use of uninhabited or some kind of bug?

@ddfisher
Copy link
Collaborator

A bit of both, IMO:

  1. <uninhabited> type is generated when mypy finds a type variable that cannot be constrained -- bug
  2. mypy assumes that the code after the appearance of <uninhabited> is unreachable -- correct and intended
  3. mypy does no typechecking on unreachable code -- not sure if bug or feature, but either way I think this is difficult to disentangle from how type checking Unions is done.

@gvanrossum
Copy link
Member

OK, then (1) should be fixed. Re: (2), is NoReturn then equivalent to uninhabited? Re: (3) I am not too happy with that but changing it would be a huge hassle for our codebase so I guess that's also a feature...

@gvanrossum
Copy link
Member

Maybe reopen with new title or open a new issue for (1)?

@pkch
Copy link
Contributor Author

pkch commented Apr 13, 2017

Ok, I'll open a new issue, let me just try a few things first to make sure I understand the issue well.

@ilevkivskyi
Copy link
Member

@ddfisher

  1. <uninhabited> type is generated when mypy finds a type variable that cannot be constrained -- bug

This also produces scary error messages sometimes. Also it looks like we already have #3032, or is this a different issue?

@pkch
Copy link
Contributor Author

pkch commented Apr 14, 2017

Yes, it is the same.

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

No branches or pull requests

4 participants