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
Function calls taking a generator as star argument can mask TypeErrors in the generator #49056
Comments
If we call some function f with a generator as star argument and this >>> def f(x): pass
...
>>> def broken(): raise TypeError
...
>>> f(*(broken() for x in (0,)))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: f() argument after * must be a sequence, not generator This is a misleading error message, as it's usually no problem to use a The problem seems to be around line 3710 of Python/ceval.c where the I'm not sure how to solve this. We probably can't distinguish a "good" |
The bpo-1615 has the same kind of problems: an AttributeError is masked |
I'm getting confused about whether it's actually desired behaviour that The error message seems to say it's not: "argument after * must be a |
I can confirm that (a) this exact behavior is happening and (b) it quite |
I added a simple check for iterables. This is not very elegant, but As I pointed out above, the actual error message "must be a sequence" is |
This patch leaks a reference on each call to PyObject_GetIter(). And I'm |
Sorry, I had meant to use PyIter_Check instead of PyObject_GetIter. I corrected the patch. |
I have also hit this error. I went to report it but found it already entered (good news), but not resolved from nearly a year ago (bad news). The error masked another bug that I had in my program and it took me quite awhile to figure out what the real problem was. I use *generator arguments quite a lot, so was surprised to see the error. So I, for one, can say that if you disable *generator arguments, you will break existing code. If anybody cares, I have verified that this error also appears in Python2.5 and Python2.4 and am attempting to add python2.5 to the Versions list. (And yes, *generators were allowed in Python2.4!) Is this headed for resolution? Progress on it seems to have stalled nearly a year ago. Can I vote to revive this? |
I verified with 3.1 the two OP cases and that generators work fine as long as they supply the correct number of values. def f(x): return x
def broken(): return 1
print(f(*(broken() for x in (0,))))
# prints 1 Change (0,) to (0,1) the normal arg num mismatch message appears. test_extcall tests version of Nothing() that follow both the old and new iteration protocol. It is possible that 'sequence' in meant in the broader sense of finite iterable rather that the narrow sense of Since that is confusing, I would replace 'sequence' with 'finite iterable'. (Infinite iterables, obviously, are bad, just as in any other uncontrolled situation, such as "a,*b = itertools.count()".) So, combine the correction and the suggestion above with original and diff against current trunk (py3k branch) if you can or at least 3.1.2. |
IIUC, the only change you suggest for my patch is using "finite iterable" instead of "sequence" or "iterable", right? I've looked at the docs and there seems to be no precedent for "finite iterable". I think it's just as obvious that the iterable has to yield a correct (and finite) number of parameters as the fact that "list(itertools.count())" is a bad idea. So for consistency I would like to keep "iterable". |
Ok, leave iterable as is. More important, you have two disjoint patches that I believe should be combined. |
Attaching a combined patch against the current py3k. |
I think the patch isn't entirely correct. It uses PyIter_Check for detecting the case when an *iterable* raises TypeError, but that function actually checks for an *iterator*. The check on the tp_iter member mentioned by Amaury Forgeot d'Arc probably would be better, but even that wouldn't detect every iterable: "Its presence normally signals that the instances of this type are iterable (although sequences may be iterable without this function)." (http://docs.python.org/dev/py3k/c-api/typeobj.html#PyTypeObject.tp_iter) (Apparently any object with a __getitem__ is iterable. By the way, collections.abc.Iterable also doesn't detect this case.) |
I'm attaching an updated patch. Instead !PyIter_Check() this patch checks for tp_iter == NULL && !PySequence_Check. If this condition is false, PyObject_GetIter has a chance to succeed (and if it fails, we shouldn't mask the exception). I also added more tests which show why the previous patch was incorrect. |
bpo-11944 is probably a duplicate of this and should be checked when this is fixed |
I haven’t tried to understand what the patches do, but bpo-5218 looks like a very similar problem with a patch including a test case. |
bpo-13904 is another dup, with patches to test and ceval. I asked that they be reloaded to this issue. |
bpo-5218 is a 4th duplicate, also with a patch to the ceval loop and test. |
Sorry Martin, I see you already said that. Anyway, I closed other three in favor of this one. |
Sounds like we just need someone comfortable with modifying ceval.c to apply this ;) |
I am posting a new version of Daniel’s patch as bpo-4806-star-TypeError.v2.patch:
I am comfortable with the code changes previous patch as well as mine. Checking the “tp_iter” field and PySequence_Check() is basically what happens behind the scenes, via PyObject_GetIter() anyway. So I think either patch is valid and a worthwhile improvement. |
Up for this bug. I got this bug in my code and loose one hour to understand what happened. Please review Martin's patch ! |
I think this is ready to push for Python 3. Python 2 might need an extra Py_TPFLAGS_HAVE_ITER check, perhaps reusing some code from the bpo-5218 patch. |
New changeset 1a8dc350962b by Martin Panter in branch '3.5': New changeset 1261f5f6fc95 by Martin Panter in branch 'default': |
In Python 2, the old-style “instance” objects are making it hard to fix this like in Python 3. And completely removing the custom error message seems a bit drastic for 2.7. Instead, I propose to just check for a generator type after the TypeError is caught, and don’t bother checking for other kinds of iterables and sequences. |
It gets even weirder with coroutines involved:
Because coroutines are executed later, the last gather() doesn't show the error message. So it tricked you into thinking that the second version is ok while the first one is not. But of course, both are ok, the problem lies in a bug deeper in your coroutine triggering the TypeError. |
Michel: I suspect your code doesn’t actually get up to handling any coroutines, and the problem is in your generator expression. What is “l”, and what are the items in it? The bug should already be fixed ready for the 3.5.2 and 3.6 releases, but I can produce this on 3.5.0: >>> l = [None]
>>> f = (coro() for coro in l)
>>> asyncio.gather(*f)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: gather() argument after * must be a sequence, not generator In the current 3.6 code the bug is fixed: >>> asyncio.gather(*f)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<stdin>", line 1, in <genexpr>
TypeError: 'NoneType' object is not callable The reason why the second call does nothing interesting is because the generator expression has already die because of the exception, and there is nothing left to iterate: >>> remaining = [*f]
>>> remaining
[]
>>> asyncio.gather(*remaining)
<Future finished result=[]> |
We fixed our bug days ago, but I would have expected [*gen] to have triggered an exception before it even got to gather(). The real code was something like:
ensure_awaitable() is just using inspect to turn coroutine functions into coroutines, and wraps non coroutine callables with asyncio.coroutine(callable)(). In the end, ensure_awaitable did raise TypeError if the argument passed was not a callable. |
New changeset eef8f72ddb00 by Martin Panter in branch '2.7': |
I believe I encountered this issue today on Python 2.7.14 (https://ci.appveyor.com/project/jaraco/jaraco-windows/build/31/job/lenh5l4dcmj137b9). In this case, I have an iterable (in itertools.imap) that raises a TypeError when evaluated, not a generator. The issue doesn't happen on Python 3, where 'map' is used instead of itertools.imap. Does this patch need to be extended to support any iterable? |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: