Skip to content

Conversation

@CZX123
Copy link
Contributor

@CZX123 CZX123 commented Apr 6, 2024

Description

This PR fixes many issues related to the new heap addition.

Changes

  • Created duplicate Closure class for use in the CSE Machine
  • Made closureToJS use CSE Machine instead of the old interpreter
    • This will allow the built-in function apply_in_underlying_javascript to correctly display the environment and ensure objects get added to the heap correctly
  • Added new declaredName property in Closure, which stores the name of the constant variable the closure is assigned to
    • This is used for apply_in_underlying_javascript to create environment frames with better names
    • functionName will now always be in the form of x => ...
  • Correctly added the argument array of a variadic closure to the heap
  • Added environment property to functions returned from calling stream
  • Added move method in Heap for use in the frontend
  • Moved Stack and IStack to a separate file to avoid circular dependency issues

CZX123 added 3 commits April 6, 2024 19:33
- make closureToJS use CSE machine instead of interpreter
- add variadic function array to heap
- handle functions from `stream`
- added heap move method
@CZX123 CZX123 self-assigned this Apr 6, 2024
@coveralls
Copy link

coveralls commented Apr 6, 2024

Pull Request Test Coverage Report for Build 8608170634

Details

  • 90 of 95 (94.74%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.07%) to 81.932%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/cse-machine/stack.ts 12 13 92.31%
src/cse-machine/closure.ts 46 50 92.0%
Totals Coverage Status
Change from base Build 8608095546: 0.07%
Covered Lines: 10687
Relevant Lines: 12693

💛 - Coveralls

@CZX123 CZX123 marked this pull request as ready for review April 8, 2024 14:49
@CZX123 CZX123 mentioned this pull request Apr 8, 2024
1 task
Copy link
Member

@martin-henz martin-henz left a comment

Choose a reason for hiding this comment

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

Good work. Quite a few cleanups and improvements.

Copy link
Member

Choose a reason for hiding this comment

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

Great to see "closure" in this folder instead of using the legacy interpreter implementation.

@martin-henz martin-henz merged commit dfce110 into master Apr 9, 2024
@martin-henz martin-henz deleted the cse-heap-updates branch April 9, 2024 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

3 participants