Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Recursive let #681

Merged
merged 4 commits into from
Apr 27, 2022
Merged

Recursive let #681

merged 4 commits into from
Apr 27, 2022

Conversation

ErinvanderVeen
Copy link
Contributor

Resolves #525. Unfortunately, this PR adds "rec" as a keyword, meaning that tests that use "rec" as a shorthand for "record" were changed to use "r" instead.

@ErinvanderVeen ErinvanderVeen self-assigned this Apr 20, 2022
@github-actions github-actions bot temporarily deployed to pull request April 20, 2022 11:25 Inactive
@ErinvanderVeen ErinvanderVeen marked this pull request as draft April 20, 2022 13:01
@github-actions github-actions bot temporarily deployed to pull request April 20, 2022 13:16 Inactive
Copy link
Member

@yannham yannham left a comment

Choose a reason for hiding this comment

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

The implementation looks good, but this PR could benefit from a tad more tests and documentation, see inline comments.

Copy link
Member

@yannham yannham left a comment

Choose a reason for hiding this comment

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

For the free variable tests, I was thinking about adding something like:

#[test]
fn recursive_let() {
    assert!(check_stat_vars(
        "{
          a = let rec b = b + a + h in b,
          b = let rec a = a + b in f (a + 1) z,
          c = let rec foo = b in let rec bar = c in if a.r then [b] else {foo = c}
        }",
        HashMap::from([
            ("a", vec!["a"]),
            ("b", vec!["b"]),
            ("c", vec!["a", "b", "c"])
        ])
    ));
}

in tests/free_var.rs. This checks that the free_var transform correctly handles the case of recursive lets bindings.

@github-actions github-actions bot temporarily deployed to pull request April 21, 2022 13:27 Inactive
@ErinvanderVeen ErinvanderVeen marked this pull request as ready for review April 22, 2022 05:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add recursive Let-bindings
3 participants