Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[move] Add function type layout #15442

Closed

Conversation

georgemitenkov
Copy link
Contributor

Description

How Has This Been Tested?

Key Areas to Review

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Move Compiler
  • Other (specify)

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

brmataptos and others added 23 commits November 30, 2024 20:24
…unnecessarily; avoid generating errors/warnings about default Loc
…ionExpr to Value::Function, got things mostly back in shape
…constraints to avoid reducing set of function type abilities when unifying types
[move-vm] Types and opcodes for closures

This PR implements the extensions needed in the file format for representing closures:

- The type (SignatureToken) has a new variant `Function(args, result, abilities)` to represent a function type
- The opcodes are extendeed by operations `ClosPack`, `ClosPackGeneric`, and `ClosEval`

This supports bit masks for specifyinng which arguments of a function are captured when creating a closure.

Bytecode verification is extended to support the new types and opcodes. The implementation of consistency, type, and reference safety should be complete. What is missing are:

- File format serialization
- Interpreter and value representation
- Value serialization
- Connection of the new types with the various other type representations
…rser work:

- `LdFunction`, `LdFunctionGeneric` = generate a closure from a defined function
- `EarlyBind` = bind more arguments to any closure
- `Invoke` = call a closure with final arguments
Add some description/semantics to `file_format.rs` to better describe
the implementation which will be needed.

Get rid of complex Mask calculations in favor of simpler early bninding of
`k` initial arguments.

Hopefully keep all bytecode verifier operations working the same.
Copy link

trunk-io bot commented Dec 2, 2024

⏱️ 25m total CI duration on this PR
Job Cumulative Duration Recent Runs
rust-move-tests 9m 🟥
check-dynamic-deps 6m 🟩🟩
rust-move-tests 4m
rust-cargo-deny 4m 🟩🟩
general-lints 50s 🟩🟩
semgrep/ci 50s 🟩🟩
file_change_determinator 24s 🟩🟩
permission-check 6s 🟩🟩
permission-check 4s 🟩🟩
check-branch-prefix 1s 🟩

settingsfeedbackdocs ⋅ learn more about trunk.io

@@ -348,6 +348,12 @@ fn native_format_impl(
out.push('}');
},

MoveTypeLayout::Function(..) => {
// We can not allow prints for now, but ideally we should map to a function nae here,
Copy link

Choose a reason for hiding this comment

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

There appears to be a typo in the comment: function nae should be function name

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@georgemitenkov georgemitenkov force-pushed the george/function-layouts branch from a6f102a to 2aa322c Compare December 2, 2024 17:13
@brmataptos brmataptos force-pushed the 11-23-vm_changes_merged_in branch 2 times, most recently from 1d8a057 to d382ca0 Compare December 4, 2024 07:23
@brmataptos brmataptos force-pushed the 11-23-vm_changes_merged_in branch 2 times, most recently from 2fa8bde to d7ded7a Compare December 4, 2024 22:50
@georgemitenkov georgemitenkov deleted the george/function-layouts branch December 5, 2024 09:12
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.

3 participants