Skip to content

Conversation

@ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented Aug 5, 2019

  • switch from the new TargetScopeError to a plain SyntaxError
    as per referenced python-dev discussion
  • clarify rationale for those extra SyntaxError cases (we don't
    want current CPython implementation details to implicitly leak into
    the "do what CPython does" de facto language specification)
  • broaden one of the error cases to handle the fact that CPython's
    symbol table analysis for a comprehension involves two different
    scopes and hence makes it difficult to detect when the target of
    a named expression in the iterable expression gets re-used as an
    iteration variable in the comprehension
  • note that even dead code affects the symbol analysis pass

- clarify rationale for the extra TargetScopeError cases (we don't
  want current CPython implementation details to implicitly leak into
  the "do what CPython does" de facto language specification)
- broaden one of the error cases to handle the fact that CPython's
  symbol table analysis for a comprehension involves two different
  scopes and hence makes it difficult to detect when the target of
  a named expression in the iterable expression gets re-used as an
  iteration variable in the comprehension
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! I have some nits.

Also, I wonder if we could smuggle a reference to "walrus operator" into the abstract, to aid search engines?

- drop TargetScopeError from the PEP in favour of SyntaxError
- use the same marker for invalid constructs as the rest of the PEP
- clarify that the ban on named expressions in comprehension iterable
  expressions is consistent (rather than only being on the first one)
- provide a bit more detail on the CPython constraint that keeps us
  from implementing the "only complain on name conflicts" rule for
  iterable expressions
- add a new note on naming that covers both the informal name and
  the internal name in the reference implementation
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I am happy now! Not sure if @tim-one or @Rosuav want to chime in still?

CC: @emilyemorehouse

@Rosuav
Copy link
Contributor

Rosuav commented Aug 10, 2019

All good from my end!

@ncoghlan ncoghlan changed the title PEP 572: Update TargetScopeError section based on implementation PEP 572: TargetScopeError->SyntaxError as per python-dev discussion Aug 13, 2019
@ncoghlan ncoghlan changed the title PEP 572: TargetScopeError->SyntaxError as per python-dev discussion PEP 572: TargetScopeError->SyntaxError, other SyntaxError adjustments Aug 14, 2019
@ncoghlan ncoghlan merged commit cd292d8 into python:master Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants