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

NotImplementedError in map_actuals_to_formals #1553

Closed
The-Compiler opened this issue May 18, 2016 · 22 comments
Closed

NotImplementedError in map_actuals_to_formals #1553

The-Compiler opened this issue May 18, 2016 · 22 comments
Labels

Comments

@The-Compiler
Copy link
Contributor

The-Compiler commented May 18, 2016

When I run mypy (0.4 or git master with Python 3.5) with --check-untyped-defs over qutebrowser, I get:

Traceback (most recent call last):
  File "./.tox/mypy/bin/mypy", line 6, in <module>
    main(__file__)
  File ".../mypy/main.py", line 54, in main
    res = type_check_only(sources, bin_dir, options)
  File ".../mypy/main.py", line 102, in type_check_only
    python_path=options.python_path)
  File ".../mypy/build.py", line 209, in build
    dispatch(sources, manager)
  File ".../mypy/build.py", line 1321, in dispatch
    process_graph(graph, manager)
  File ".../mypy/build.py", line 1452, in process_graph
    process_stale_scc(graph, scc)
  File ".../mypy/build.py", line 1482, in process_stale_scc
    graph[id].type_check()
  File ".../mypy/build.py", line 1301, in type_check
    manager.type_checker.visit_file(self.tree, self.xpath)
  File ".../mypy/checker.py", line 419, in visit_file
    self.accept(d)
  File ".../mypy/checker.py", line 460, in accept
    typ = node.accept(self)
  File ".../mypy/nodes.py", line 650, in accept
    return visitor.visit_class_def(self)
  File ".../mypy/checker.py", line 1049, in visit_class_def
    self.accept(defn.defs)
  File ".../mypy/checker.py", line 460, in accept
    typ = node.accept(self)
  File ".../mypy/nodes.py", line 715, in accept
    return visitor.visit_block(self)
  File ".../mypy/checker.py", line 1144, in visit_block
    self.accept(s)
  File ".../mypy/checker.py", line 460, in accept
    typ = node.accept(self)
  File ".../mypy/nodes.py", line 531, in accept
    return visitor.visit_decorator(self)
  File ".../mypy/checker.py", line 1934, in visit_decorator
    e.func.accept(self)
  File ".../mypy/nodes.py", line 462, in accept
    return visitor.visit_func_def(self)
  File ".../mypy/checker.py", line 573, in visit_func_def
    self.check_func_item(defn, name=defn.name())
  File ".../mypy/checker.py", line 631, in check_func_item
    self.check_func_def(defn, typ, name)
  File ".../mypy/checker.py", line 739, in check_func_def
    self.accept_in_frame(item.body)
  File ".../mypy/checker.py", line 475, in accept_in_frame
    answer = self.accept(node, type_context)
  File ".../mypy/checker.py", line 460, in accept
    typ = node.accept(self)
  File ".../mypy/nodes.py", line 715, in accept
    return visitor.visit_block(self)
  File ".../mypy/checker.py", line 1144, in visit_block
    self.accept(s)
  File ".../mypy/checker.py", line 460, in accept
    typ = node.accept(self)
  File ".../mypy/nodes.py", line 753, in accept
    return visitor.visit_assignment_stmt(self)
  File ".../mypy/checker.py", line 1153, in visit_assignment_stmt
    self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None)
  File ".../mypy/checker.py", line 1203, in check_assignment
    self.infer_variable_type(inferred, lvalue, self.accept(rvalue),
  File ".../mypy/checker.py", line 460, in accept
    typ = node.accept(self)
  File ".../mypy/nodes.py", line 1185, in accept
    return visitor.visit_call_expr(self)
  File ".../mypy/checker.py", line 1992, in visit_call_expr
    return self.expr_checker.visit_call_expr(e)
  File ".../mypy/checkexpr.py", line 141, in visit_call_expr
    return self.check_call_expr_with_callee_type(callee_type, e)
  File ".../mypy/checkexpr.py", line 192, in check_call_expr_with_callee_type
    e.arg_names, callable_node=e.callee)[0]
  File ".../mypy/checkexpr.py", line 226, in check_call
    lambda i: self.accept(args[i]))
  File ".../mypy/checkexpr.py", line 1570, in map_actuals_to_formals
    raise NotImplementedError()
NotImplementedError: 

*** INTERNAL ERROR ***

qutebrowser/browser/webpage.py:344: error: Internal error -- please report a bug at https://github.com/python/mypy/issues

I wasn't able to construct a minimal example which fails in the same way, but it fails on this line:

    @pyqtSlot('QWebFrame*', 'QWebPage::Feature')
    def on_feature_permission_requested(self, frame, feature):
        """Ask the user for approval for geolocation/notifications."""
        options = {
            QWebPage.Notifications: ('content', 'notifications'),
            QWebPage.Geolocation: ('content', 'geolocation'),
        }
        config_val = config.get(*options[feature])  # <-----
        # [...]

Some interesting locals from a --pdb session:

>>> argt.items
[builtins.str, builtins.str]
>>> j
1
>>> ncallee
2
>>> callee_kinds
[2, 4]

Seems like the 4 for the current argument (j=1) is nodes.UNBOUND_TVAR.

The-Compiler added a commit to qutebrowser/qutebrowser that referenced this issue May 18, 2016
@gvanrossum
Copy link
Member

Here's a smaller repro:

def xget(section, subsection):
    pass

def aget(*args, **kwds):
    return xget(*args, **kwds)

def foo(i):
    args = ('x', 'y')
    return aget(*args)  # <----- error

The explicit raise NotImplementedError() in the mypy code suggests that Jukka was aware that he left some complicated case unhandled. Probably not rocket science to fix it.

@ddfisher ddfisher added this to the Future milestone May 19, 2016
@refi64
Copy link
Contributor

refi64 commented May 24, 2016

Has this been fixed already? Running Mypy on @gvanrossum's example now prints nothing.

@gvanrossum
Copy link
Member

@The-Compiler Can you confirm? Indeed my little example doesn't crash any more. Most likely @rwbarton fixed this as a side effect of one of his other fixes.

@The-Compiler
Copy link
Contributor Author

Unfortunately I can still reproduce this with the current master and my code - no time right now to investigate more though, sorry!

@gvanrossum
Copy link
Member

gvanrossum commented May 24, 2016 via email

@The-Compiler
Copy link
Contributor Author

Indeed. You should be able to clone https://github.com/The-Compiler/qutebrowser, check out the mypy branch, replace mypy-lang==0.4 by git+https://github.com/python/mypy.git in tox.ini and run tox -e mypy. When you do so, the issue will not happen because I worked around it in qutebrowser/qutebrowser@996ebbd. As soon as you do git revert 996ebbdb142070d13765d7ac619fc09553670bf1 you should see it again.

@tharvik
Copy link
Contributor

tharvik commented May 30, 2016

I just found a smaller repro:

t = (1, 2)
'{} {}'.format(*t)

@euresti
Copy link
Contributor

euresti commented Jun 2, 2016

Here's a python2 repro we hit:

from typing import Any

def good_log(string, *n):
    # type: (str, *Any) -> None
    string %= n

def bad_log(string, *n, **kw):
    # type: (str, *Any, **Any) -> None
    string %= n

def testit():
    # type: () -> None
    good_log("Numbers: (%r %r, %r %r)", *(1, 2, 3, 4))

    bad_log("Numbers: (%r %r, %r %r)", *(1, 2, 3, 4))

Notice how the existence of the **kw is what causes this to happen.

@refi64
Copy link
Contributor

refi64 commented Jun 2, 2016

Smallest test case yet:

def f(**kw): pass
f(*(1, 2))

@gvanrossum
Copy link
Member

gvanrossum commented Jun 2, 2016 via email

@refi64
Copy link
Contributor

refi64 commented Jun 2, 2016

Simply commenting out these lines seems to fix the issue and not introduce any worse bugs, though I can't be sure...

@gvanrossum
Copy link
Member

gvanrossum commented Jun 2, 2016 via email

@refi64
Copy link
Contributor

refi64 commented Jun 3, 2016

Adding print(map, i, j, argt) right before the raise gives:

[[]] 0 0 Tuple[builtins.int, builtins.int]

Seems to be that mypy is trying to map the *(1, 2) to the **kw, which is causing the callee_kinds[j] != nodes.ARG_STAR2 check to fail, ending up in the NotImplementedError.

@rwbarton
Copy link
Contributor

rwbarton commented Jun 3, 2016

I think we shouldn't be advancing through *args of the callee in one-to-one correspondence with the components of the Tuple argument. If we encounter *args in the callee, it should receive all the left-over fields of the Tuple. I believe that just means that the *args callee argument should be mapped to the *args argument of the caller, but I'm not sure.

Another case that mypy crashes on is when there's no *args in the callee and we reach a **kwargs argument. That should be a "too many arguments" error.

@gvanrossum
Copy link
Member

Actually, it's possible that the *args in the call site doesn't match the *args in the definition, like this:

def foo(a, *args):
    return dict(a=a, args=args)
args = (1, 2, 3)
foo(*args)  # {'a': 1, 'args': (2, 3)}
foo(1, 2, *args)  # {'a': 1, 'args': (2, 1, 2, 3)}

But apart from that Reid's right!

@rwbarton
Copy link
Contributor

rwbarton commented Jun 4, 2016

Hm, I was hoping that matching up the *args of the callee with the *args of the call would be okay even though not all of the fields of the *args of the call might actually get passed to *args in the callee (as in your first example above, where one of the fields goes to a instead). I thought it might be okay because argument compatibility checking for *args vs. *args only needs to consider the types of the elements of the sequence, not the length of the sequence. But this doesn't quite cover every case, because the actual *args might be a heterogeneous Tuple. For example we could adjust the example above to:

def foo(a: int, *args: str) -> None: ...
args = (1, 'b', 'c')  # type: Tuple[int, str, str]
foo(*args)  # Should be allowed

In order to support this we'd have to understand that callee argument 2 corresponds to a subsequence of caller argument 1. We could just not handle this case for now though (we would reject some valid programs like the above).

@gvanrossum
Copy link
Member

gvanrossum commented Jun 4, 2016 via email

@gvanrossum gvanrossum modified the milestones: 0.4.x, Future Jul 3, 2016
@gvanrossum
Copy link
Member

Moving the milestone closer as this keeps coming up.

@jirutka
Copy link

jirutka commented Jul 17, 2016

Even smaller test case:

tail = ['b', 'c']
['a', *tail]

This bug is probably related to #704.

@gvanrossum
Copy link
Member

@jirutka: No, that's a repro for the issue mentioned in #704 but the traceback is different from what this issue is about. You can either add your repro to #704 or file a new issue to call it out (since #704 is really about introducing support for PEP 448 -- if mypy just gave an error saying "Syntax error" instead of a crash that would be a start).

@jirutka
Copy link

jirutka commented Jul 17, 2016

Okay, I’ve opened new issue #1890.

gvanrossum pushed a commit that referenced this issue Jul 18, 2016
@gvanrossum
Copy link
Member

I'm tired of this particular crash (it hits a lot of people) so in #1891 I'm applying what is essentially @kirbyfan64's proposal: just delete the raise NotImplementedError(). I'm breaking out of the for-loop instead.

Skimming the code it looks like there are other cases that aren't handled properly. But my first instinct is to try to raise an error, which is impossible, because map_actuals_to_formals() has no argument giving access to the Errors instance in use, nor the context needed for the line number.

So I'm content with fixing the crash -- I'm going to report other error cases (if I can find any) in a new issue.

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

No branches or pull requests

8 participants