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

Fix capture logic for inner closures #9754

Merged
merged 1 commit into from Jul 20, 2023

Conversation

sophiajt
Copy link
Member

Description

This fixes the variable capture logic for closures in two cases:

  • Closures inside of closures did not properly register the closures (or lack thereof) in the outer closure
  • Closures which called their inner closures before definition did not properly calculate the closures of the outer closure

Example of the first case:

do { let b = 3; def c [] { $b }; c }

Example of the second case (notice c is called before it is defined):

do { let b = 3; c; def c [] { $b }; c }

User-Facing Changes

This should strictly allow closures to work more correctly.

Tests + Formatting

After Submitting

@amtoine
Copy link
Member

amtoine commented Jul 20, 2023

looks related to the end of the description of #9746

Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

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

i'm not sure i understand the parser well enough, by far, but the changes look good and i like the two new tests 😊

@sholderbach sholderbach added the scoping/name-resolution How Nu finds which variables/functions are in scope and to what they are bound label Jul 20, 2023
@fdncred
Copy link
Collaborator

fdncred commented Jul 20, 2023

Examples seem to work for me. I also tested this case, which is similar to the second case.

 do { let b = 3; print (c); def c [] { $b }; print (c) }
3
3

@sophiajt sophiajt merged commit d104efd into nushell:main Jul 20, 2023
19 checks passed
@sophiajt sophiajt deleted the fix_capture_logic branch July 20, 2023 19:10
@kubouch
Copy link
Contributor

kubouch commented Jul 21, 2023

I think there might still be some capturing bug left because this issue is still present: #5994

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scoping/name-resolution How Nu finds which variables/functions are in scope and to what they are bound
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants