-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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-compiler-v2] clean up a few remaining issues in lambda parser/front-end code #15365
base: 09-27-add_parser_code_for_lambda_types
Are you sure you want to change the base?
[move-compiler-v2] clean up a few remaining issues in lambda parser/front-end code #15365
Conversation
⏱️ 31m total CI duration on this PR
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
3e3563e
to
dac085c
Compare
env.error( | ||
&loc, | ||
// TODO(LAMBDA) | ||
"Lambdas expressions with `store` ability currently may only be a simple call to an existing `public` function. This lambda expression requires defining a `public` helper function, which might affect module upgradeability and is not yet supported." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error message looks a little bit confusing for users (e.g what is a helper
function and how it may affect upgradeability). Maybe just remove
requires defining a public
helper function, which might affect module upgradeability` and just saying it is not supported yet?
env.error( | ||
&loc, | ||
&format!( | ||
"captured variable `{}` must have a value with `copy` ability", // TODO(LAMBDA) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a test case for this error?
Description
Various fixes:
Constraint::NoFunction
as it's too broad._ok
versions that compile up to "not implemented" errors.How Has This Been Tested?
Usual tests run, showing unchanged behavior on existing code. Tests under
/lambda/
without.lambda.
config show proper errors generated if to many functions are used._ok
tests now show code working up until "not implemented" errors.Key Areas to Review
Most complex stuff is in previous PR, but:
store
andcopy
properties in captured free variables?/lambda/storage/*_ok.move
tests look like reasonable code to expect from users?Type of Change
Which Components or Systems Does This Change Impact?
Checklist