Skip to content

Conversation

@jurgenvinju
Copy link
Member

@jurgenvinju jurgenvinju commented Nov 5, 2025

This PR does some clean up that helps in debugging; making sure some assumptions about the contents of environments are true: some old code is removed that might break those assumptions, but it doesn't currently because it is never used.

The one change in the test code makes sure getRoot is called less often, which avoids running into the issue at hand, but doesn't solve it per se.

@codecov
Copy link

codecov bot commented Nov 5, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 47%. Comparing base (934a820) to head (ae56fe4).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
src/org/rascalmpl/interpreter/env/Environment.java 0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##              main   #2511   +/-   ##
=======================================
  Coverage       47%     47%           
+ Complexity    6708    6707    -1     
=======================================
  Files          791     791           
  Lines        65187   65160   -27     
  Branches      9751    9751           
=======================================
+ Hits         30925   30927    +2     
+ Misses       31849   31820   -29     
  Partials      2413    2413           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DavyLandman DavyLandman marked this pull request as draft November 5, 2025 21:23
@jurgenvinju jurgenvinju marked this pull request as ready for review November 17, 2025 15:01
Copy link
Member Author

@jurgenvinju jurgenvinju left a comment

Choose a reason for hiding this comment

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

I think it this is a pure refactoring with no impact on runtime semantics in the Rascal project.

  • The removed code was dead;
  • env.getStore() is a faster short-hand for env.getRoot().getStore()ifenvis aModuleEnvironment`. A fast-path optimization by receiver specialization.
  • Made all protected fields private in ModuleEnvironment without compilation issues.

Because we remove shallow cloning as a feature from ModuleEnvironment this increases the strength of our theory that only one ModuleEnvironment is present in memory for every module at a specific moment in time. This helps in the analysis of watch/reload, as well as the semantics of extend.

@jurgenvinju jurgenvinju merged commit a75b1b1 into main Nov 21, 2025
1 check passed
@jurgenvinju jurgenvinju deleted the fix/issue-2508 branch November 21, 2025 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant