classification
Title: TargetScopeError not raised for comprehension scope conflict
Type: behavior Stage: resolved
Components: Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: Damien George, barry, emilyemorehouse, gvanrossum, lukasz.langa, ncoghlan, serhiy.storchaka
Priority: release blocker Keywords: patch

Created on 2019-08-05 00:15 by ncoghlan, last changed 2019-08-25 14:43 by ncoghlan. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15131 merged ncoghlan, 2019-08-05 04:15
PR 15491 merged ncoghlan, 2019-08-25 14:10
Messages (16)
msg349012 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-05 00:15
While implementing PEP 572, Emily noted that the check for conflicts between assignment operators and comprehension iteration variables had not yet been implemented: https://bugs.python.org/issue35224#msg334331

Damien George came across this discrepancy while implementing assignment expressions for MicroPython.

The proposed discussion regarding whether or not the PEP should be changed didn't happen, and the PEP itself misses the genuinely confusing cases where even an assignment expression that *never executes* will still make the iteration variable leak:

>>> [i for i in range(5)]
[0, 1, 2, 3, 4]
>>> [i := 10 for i in range(5)]
[10, 10, 10, 10, 10]
>>> i
10
>>> [False and (i := 10) for i in range(5)]
[False, False, False, False, False]
>>> i
4

And that side effect happens even if the assignment expression is nested further down in an inner loop:

>>> [(i, j, k) for i in range(2) for j in range(2) for k in range(2)]
[(0, 0, 0), (0, 0, 1), (0, 1, 0), (0, 1, 1), (1, 0, 0), (1, 0, 1), (1, 1, 0), (1, 1, 1)]
>>> i
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'i' is not defined
>>> [(i, j, k) for i in range(2) for j in range(2) for k in range(2) if True or (i:=10)]
[(0, 0, 0), (0, 0, 1), (0, 1, 0), (0, 1, 1), (1, 0, 0), (1, 0, 1), (1, 1, 0), (1, 1, 1)]
>>> i
1

I'm at the PyCon AU sprints today, and will be working on a PR to make these cases raise TargetScopeError as specified in the PEP.
msg349022 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2019-08-05 04:23
Thanks for being part of the village raising this child!
msg349033 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-05 07:34
Added a PEP update as well: https://github.com/python/peps/pull/1140
msg349057 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2019-08-05 17:11
I know the PEP defines TargetScopeError as a subclass of SyntaxError, but it doesn't really explain why a subclass is necessary.  I think seeing "TargetScopeError" will be a head scratcher.  Why not just SyntaxError without introducing a new exception?
msg349084 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2019-08-05 22:32
[Barry]
> I know the PEP defines TargetScopeError as a subclass of SyntaxError, but it doesn't really explain why a subclass is necessary.  I think seeing "TargetScopeError" will be a head scratcher.  Why not just SyntaxError without introducing a new exception?

Hm, that's not a bad point. We report all sorts of things found by the bytecode compiler as SyntaxError.

OTOH This would require a PEP change, formal review, etc. (IMO much more so than adding the new edge case that Nick and Damien discovered.) Maybe the best way of doing this would be to implement TargetScopeError now, then start the debate about killing it, and try to get that in before beta4?
msg349092 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-06 00:33
I believe our main motivation for separating it out was the fact that even though TargetScopeError is a compile-time error, the affected code is syntactically fine - there are just issues with unambiguously inferring the intended read/write location for the affected target names. (Subclassing SyntaxError is then a pragmatic concession to the fact that "SyntaxError" also de facto means "CompilationError")

Searching for "Python TargetScopeError" will also get folks to relevant information far more quickly than searching for "Python SyntaxError" will.

Pre-seeding Stack Overflow with an answer to "What does TargetScopeError mean in Python?" would probably be a good idea though (similar to what I did for https://stackoverflow.com/questions/25445439/what-does-syntaxerror-missing-parentheses-in-call-to-print-mean-in-python )
msg349094 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2019-08-06 01:53
But we don't do that with any of the other (many) errors detected by later passes of the compiler. Those report dozens of SyntaxErrors, with good descriptive messages. Users can search the web for those messages too.

Also, I doubt that many people will ever get a TargetScopeError. The examples are all esoteric -- yes, the compiler needs to reject them, but no, they are not things one is likely to try intentionally. (The exception may be the forbidden form you're adding, [... for ... in (i := ...)].)
msg349100 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-06 05:46
> We report all sorts of things found by the bytecode compiler as SyntaxError.

Except of these which are reported with IndentationError or its subclass TabError.
msg349139 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2019-08-06 22:58
> OTOH This would require a PEP change, formal review, etc.

It would be trivial though.  There are only two references to TargetScopeError in the PEP.  One talks about adding the exception and the other just mentions it almost in passing as a subclass of SyntaxError.

I think it would be better to just use SyntaxError.
msg349141 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2019-08-07 00:02
If you and Nick both feel strongly about this please take it to python-dev,
I'm sure we'll get closure quickly there.
msg349632 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-14 02:39
The outcome of the python-dev discussion was that we agreed to switch to raising a plain SyntaxError, as that's what we do everywhere else that this kind of problem comes up (e.g. conflicts between global and nonlocal declarations, or between those and parameter declarations, as well as the various other cases of "that statement/expression is fine in isolation, but you can't use it *here*").

Both PRs have been updated accordingly, and the PEP PR has been merged.
msg350292 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-08-23 14:09
This is marked as a release blocker. The last beta is scheduled for Monday. Please decide how to proceed ASAP.
msg350310 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2019-08-23 15:48
The decision has been made to get rid of TargetScopeError and instead just use SyntaxError. PEP 572 was updated already. We're just waiting for someone (Serhiy?) to review Nick's patch, PR #15131.
msg350455 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-25 13:45
New changeset 5dbe0f59b7a4f39c7c606b48056bc29e406ebf78 by Nick Coghlan in branch 'master':
bpo-37757: Disallow PEP 572 cases that expose implementation details (GH-15131)
https://github.com/python/cpython/commit/5dbe0f59b7a4f39c7c606b48056bc29e406ebf78
msg350460 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-25 14:41
New changeset 6ca030765db49525f16b8fabff4153238148b58d by Nick Coghlan in branch '3.8':
[3.8] bpo-37757: Disallow PEP 572 cases that expose implementation details (GH-15491)
https://github.com/python/cpython/commit/6ca030765db49525f16b8fabff4153238148b58d
msg350461 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-25 14:43
Merged for 3.8b4 after Emily's review.

Thanks to all involved in the PEP update and change discussion!
History
Date User Action Args
2019-08-25 14:43:28ncoghlansetstatus: open -> closed
resolution: fixed
messages: + msg350461

stage: patch review -> resolved
2019-08-25 14:41:50ncoghlansetmessages: + msg350460
2019-08-25 14:10:49ncoghlansetpull_requests: + pull_request15178
2019-08-25 13:45:43ncoghlansetmessages: + msg350455
2019-08-23 15:48:46gvanrossumsetmessages: + msg350310
2019-08-23 14:09:32lukasz.langasetpriority: deferred blocker -> release blocker
nosy: + lukasz.langa
messages: + msg350292

2019-08-14 02:39:33ncoghlansetmessages: + msg349632
2019-08-07 00:02:11gvanrossumsetmessages: + msg349141
2019-08-06 22:58:15barrysetmessages: + msg349139
2019-08-06 05:46:15serhiy.storchakasetmessages: + msg349100
2019-08-06 01:53:06gvanrossumsetmessages: + msg349094
2019-08-06 00:33:42ncoghlansetmessages: + msg349092
2019-08-05 22:32:34gvanrossumsetmessages: + msg349084
2019-08-05 17:11:32barrysetnosy: + barry
messages: + msg349057
2019-08-05 09:22:27serhiy.storchakasetnosy: + serhiy.storchaka
2019-08-05 07:34:44ncoghlansetmessages: + msg349033
2019-08-05 04:23:52gvanrossumsetnosy: + gvanrossum
messages: + msg349022
2019-08-05 04:15:46ncoghlansetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request14871
2019-08-05 00:15:02ncoghlancreate