Skip to content

Commit

Permalink
Fix capture logic for inner closures (#9754)
Browse files Browse the repository at this point in the history
# 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
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
  • Loading branch information
sophiajt committed Jul 20, 2023
1 parent bd9d865 commit d104efd
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 14 deletions.
47 changes: 33 additions & 14 deletions crates/nu-parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5894,25 +5894,44 @@ pub fn discover_captures_in_expr(
if let Some(block_id) = decl.get_block_id() {
match seen_blocks.get(&block_id) {
Some(capture_list) => {
output.extend(capture_list);
// Push captures onto the outer closure that aren't created by that outer closure
for capture in capture_list {
if !seen.contains(&capture.0) {
output.push(*capture);
}
}
}
None => {
let block = working_set.get_block(block_id);
if !block.captures.is_empty() {
output.extend(block.captures.iter().map(|var_id| (*var_id, call.head)));
for capture in &block.captures {
if !seen.contains(capture) {
output.push((*capture, call.head));
}
}
} else {
let mut seen = vec![];
seen_blocks.insert(block_id, output.clone());

let mut result = vec![];
discover_captures_in_closure(
working_set,
block,
&mut seen,
seen_blocks,
&mut result,
)?;
output.extend(&result);
let result = {
let mut seen = vec![];
seen_blocks.insert(block_id, output.clone());

let mut result = vec![];
discover_captures_in_closure(
working_set,
block,
&mut seen,
seen_blocks,
&mut result,
)?;

result
};
// Push captures onto the outer closure that aren't created by that outer closure
for capture in &result {
if !seen.contains(&capture.0) {
output.push(*capture);
}
}

seen_blocks.insert(block_id, result);
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/tests/test_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,3 +655,13 @@ fn def_with_in_var_mut_2() -> TestResult {
"3",
)
}

#[test]
fn properly_nest_captures() -> TestResult {
run_test(r#"do { let b = 3; def c [] { $b }; c }"#, "3")
}

#[test]
fn properly_nest_captures_call_first() -> TestResult {
run_test(r#"do { let b = 3; c; def c [] { $b }; c }"#, "3")
}

0 comments on commit d104efd

Please sign in to comment.