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

gh-104602: ensure all cellvars are known up front #104603

Merged
merged 5 commits into from
May 19, 2023
Merged

Conversation

carljm
Copy link
Member

@carljm carljm commented May 18, 2023

Because u_cellvars and u_freevars share the same index-space and must not reuse the same indices, all cellvars must be populated in compiler_enter_scope, and never lazily. (So that freevars can begin at the right index offset.)

This means that compiler_enter_scope must already know about all cellvars up front, including those in inlined comprehensions.

But we can't promote all inlined-comprehension cells to be handled as cells throughout the function; if the name is global in outer scope, it must stay that way in order to resolve names correctly. So we need the symtable to record the fact that a symbol is a cellvar in an inlined scope, so compiler_enter_scope can include it in u_cellvars.

This issue only shows up in the case where we have an inlined comprehension containing both a cell var and a free var, and the cell var is a global in the outer scope.

@JelleZijlstra
Copy link
Member

(Fixed the trivial merge conflict with #104528, will review the code tomorrow)

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good, left a suggestion for improving the test

Lib/test/test_listcomps.py Outdated Show resolved Hide resolved
* main:
  pythongh-74690: Don't set special protocol attributes on non-protocol subclasses of protocols (python#104622)
  pythongh-104623: Update Windows installer to use SQLite 3.42.0 (python#104625)
  pythongh-104050: Add more type annotations to Argument Clinic (python#104628)
  pythongh-104629: Don't skip test_clinic if _testclinic is missing (python#104630)
  pythongh-104549: Set __module__ on TypeAliasType (python#104550)
  pythongh-104050: Improve some typing around `default`s and sentinel values (python#104626)
  pythongh-104146: Remove unused vars from Argument Clinic (python#104627)
  pythongh-104615: don't make unsafe swaps in apply_static_swaps (python#104620)
  pythonGH-104484: Add case_sensitive argument to `pathlib.PurePath.match()` (pythonGH-104565)
  pythonGH-96803: Document and test new unstable internal frame API functions (pythonGH-104211)
  pythonGH-104580: Don't cache eval breaker in interpreter (pythonGH-104581)
@carljm carljm enabled auto-merge (squash) May 18, 2023 23:44
@carljm carljm linked an issue May 18, 2023 that may be closed by this pull request
@carljm carljm merged commit 86e6f16 into python:main May 19, 2023
@carljm carljm deleted the nestedcomps branch May 19, 2023 00:08
JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this pull request May 19, 2023
This threw a SystemError before python#104603. Adding a separate test
because this was a different failure mode than the other two new
tests from python#104603, both of which used to segfault.
carljm pushed a commit that referenced this pull request May 19, 2023
This threw a SystemError before #104603. Adding a separate test
because this was a different failure mode than the other two new
tests from #104603, both of which used to segfault.
carljm added a commit to gsallam/cpython_with_perfmap_apii that referenced this pull request May 20, 2023
* main: (30 commits)
  pythongh-103987: fix several crashes in mmap module (python#103990)
  docs: fix wrong indentation causing rendering error in dis page (python#104661)
  pythongh-94906: Support multiple steps in math.nextafter (python#103881)
  pythongh-104472: Skip `test_subprocess.ProcessTestCase.test_empty_env` if ASAN is enabled (python#104667)
  pythongh-103839: Allow building Tkinter against Tcl 8.7 without external libtommath (pythonGH-103842)
  pythongh-85984: New additions and improvements to the tty library. (python#101832)
  pythongh-104659: Consolidate python examples in enum documentation (python#104665)
  pythongh-92248: Deprecate `type`, `choices`, `metavar` parameters of `argparse.BooleanOptionalAction` (python#103678)
  pythongh-104645: fix error handling in marshal tests (python#104646)
  pythongh-104600: Make type.__type_params__ writable (python#104634)
  pythongh-104602: Add additional test for listcomp with lambda (python#104639)
  pythongh-104640: Disallow walrus in comprehension within type scopes (python#104641)
  pythongh-103921: Rename "type" header in argparse docs (python#104654)
  Improve readability of `typing._ProtocolMeta.__instancecheck__` (python#104649)
  pythongh-96522: Fix deadlock in pty.spawn (python#96639)
  pythonGH-102818: Do not call `PyTraceBack_Here` in sys.settrace trampoline.  (pythonGH-104579)
  pythonGH-103545: Add macOS specific constants for ``os.setpriority`` to ``os`` (python#104606)
  pythongh-104623: Update macOS installer to SQLite 3.42.0 (pythonGH-104624)
  pythongh-104619: never leak comprehension locals to outer locals() (python#104637)
  pythongh-104602: ensure all cellvars are known up front (python#104603)
  ...
@JelleZijlstra JelleZijlstra self-assigned this Apr 19, 2024
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.

PEP 709: Crash with lambda + nested scope
3 participants