hide caller env from callee

[gelisam]

in a lambda, only the variables which are lexically visible at the definition site are visible, not the variables which are lexically visible at the call site. therefore, when invoking a closure, we should evaluate the body in the captured environment, not in a combined environment containing both the captured environment and the call site's environment.

also, Up was propagating the callee's environment to the caller, and it was sometimes accidentally used instead of the environment captured in Kont.

Summary by CodeRabbit

• Refactor
  • Streamlined state management in the evaluation process by reducing the number of parameters in the upward evaluation phase.
  • Simplified the application of macro actions by directly applying closures to results, eliminating unnecessary environment dependencies.
  • Updated error handling for a more general approach, improving clarity in error reporting during closure applications.

These improvements enhance the maintainability and consistency of the system while ensuring smoother internal processing without altering the outward functionality.

[coderabbitai]

Walkthrough

This pull request updates the evaluator and macro expander modules. In the evaluator, the upward evaluation state and error reporting have been simplified by removing the environment parameter from the Up constructor and related error cases. In the expander, the closure application in macro actions is now performed directly without accessing the environment, and error handling has been generalized.

Changes

File(s)	Change Summary
src/Evaluator.hs	Removed the environment parameter from the Up constructor in EState, updated the step function and related error handling cases to reflect this simplification.
src/Expander.hs	Modified interpretMacroAction to apply closures directly to the bound result, eliminating the dependency on environment retrieval and using more general error handling.

Possibly related PRs

• A CEK Evaluator #223: Updates to the evaluator's EState and step function in that PR have a strong code-level connection to the changes made in this pull request.

Suggested labels

CodeRabbit

Poem

I'm just a rabbit hopping through the code,
Shedding extra baggage on this lighter road.
With cleaner states and closures oh so neat,
I celebrate these changes with a joyful beat!
Hoppy lines and cheerful bytes make my day complete!

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc70ec3 and 261f6d8.

📒 Files selected for processing (1)

• src/Evaluator.hs (14 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (5)

• GitHub Check: macOS-latest / ghc 9.6
• GitHub Check: ubuntu-latest / ghc 9.2.5
• GitHub Check: ubuntu-latest / ghc 9.4
• GitHub Check: ubuntu-latest / ghc 9.6
• GitHub Check: stack / ghc 9.2.8

🔇 Additional comments (6)

src/Evaluator.hs (6)

175-175: Significant change: Removed environment from Up constructor.

The removal of the VEnv parameter from the Up constructor aligns with the PR objective of hiding the caller's environment from the callee. This change ensures that values being passed up the evaluation stack don't carry their evaluation environment with them.

---

509-511: Improved environment handling in closure application.

The changes to applyAsClosure now correctly handle the environment by:

1. Using only the closure's captured environment for first-order functions
2. Directly returning the result for higher-order primitives

---

524-526: Simplified yield function with better error handling.

The yield function has been simplified to handle only the essential cases:

1. Successful completion with a value
2. Error states
3. Impossible states (with a clear error message)

---

234-234: Consistent error handling in type mismatch cases.

The error handling for type mismatches has been updated to remove the environment parameter, maintaining consistency with the new Up constructor design.

Also applies to: 239-239, 244-244

---

201-203:

✅ Verification successful

Verify closure application behavior.

The changes to closure application ensure that the callee's environment is used correctly, but we should verify that this doesn't break any existing functionality.

Run this script to check for potential issues with closure applications:

---

🏁 Script executed:

#!/bin/bash
# Description: Search for all closure applications to ensure they're handled consistently
ast-grep --pattern 'apply :: Closure -> Value -> Either EState Value'
rg -A 5 'applyAsClosure'

Length of output: 913

---

Closure Application Behavior Verified

The shell script outputs confirm that the closure application—specifically the applyAsClosure call used with both InArg and InFun patterns—is handling environments consistently, as defined in the apply function at line 494. There are no deviations from the expected behavior detected.

---

221-221:

✅ Verification successful

Verify type case handling with environment.

The type case handling now passes the environment directly to the macro action. We should verify this doesn't cause any scoping issues.

---

🏁 Script executed:

#!/bin/bash
# Description: Check for consistent environment handling in type cases
rg -A 5 'MacroActionTypeCase'
ast-grep --pattern 'doTypeCase :: VEnv -> SrcLoc -> Ty -> [(TypePattern, Core)] -> Either EState Value'

Length of output: 1149

---

Type case handling verified: No scoping issues found.

The verification scripts confirm that the environment is consistently passed to the macro action in all relevant modules. Specifically, the use of MacroActionTypeCase env loc good cs in Evaluator.hs, its analogous usage in Expander.hs and Pretty.hs, and the signature of doTypeCase all support that the environment remains correctly scoped. No additional changes are needed here.

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[doyougnu]

LGTM. Just a few places to silence warnings. I had made these changes in the stack traces branch but didn't go as far as removing the env from Up. Great Work!

[gelisam]

I have made all the requested changes. merging!