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-compiler-v2] clean up a few remaining issues in lambda parser/front-end code #15365

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d39bbbf
MVP lambda support in parser and expression builder
brmataptos Oct 30, 2024
9447812
fix issue with expected_type and curry construction with local variab…
brmataptos Oct 31, 2024
889d1f9
undo some debugging tweaks to type printing that increased diff size …
brmataptos Oct 31, 2024
48129ae
run rust_lint.sh, move_pr -2 -i
brmataptos Oct 31, 2024
9646e96
inverted mask sense, changed Curry expr to Operation::Bind, MoveFunct…
brmataptos Nov 14, 2024
54f85e8
fix a merge conflict that duplicated code, and check bound variables …
brmataptos Nov 17, 2024
f5a4baa
undo underscore support
brmataptos Nov 19, 2024
239e6e1
handle more function values in `LambdaLifter::try_to_reduce_lambda_to…
brmataptos Nov 21, 2024
744b79d
add `AbilitySet::MAXIMAL_FUNCTIONS` and use it in some function type …
brmataptos Nov 21, 2024
13dd2e0
fix lint
brmataptos Nov 21, 2024
386486c
rename Bind to EarlyBind and remove mask in favor of just being able …
brmataptos Nov 22, 2024
9bbc396
address rest of comments on PR as of 11/26
brmataptos Nov 27, 2024
5c0e9f0
address a few remaining comments from Vineeth that I missed before
brmataptos Nov 27, 2024
2fd0d13
suppress `drop` ability in function types, since it's common; fix par…
brmataptos Nov 27, 2024
6bd922b
fix move-model test output that slightly changes
brmataptos Nov 27, 2024
f9c8c47
add a test for not dropping an unused function value
brmataptos Dec 1, 2024
4cf9a1a
address a final few comments, mostly code cleanup in lambda_lifter
brmataptos Dec 1, 2024
3fc1956
fix clippy warnings
brmataptos Dec 1, 2024
d434caa
tune error message a bit mroe
brmataptos Dec 1, 2024
8b37927
address PR 15365 comments as of 11/26
brmataptos Nov 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion third_party/move/evm/move-to-yul/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ impl<'a> Context<'a> {
Tuple(_)
| TypeParameter(_)
| Reference(_, _)
| Fun(_, _)
| Fun(..)
| TypeDomain(_)
| ResourceDomain(_, _, _)
| Error
Expand Down
2 changes: 1 addition & 1 deletion third_party/move/evm/move-to-yul/src/solidity_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ impl SolidityType {
},
TypeParameter(_)
| Reference(_, _)
| Fun(_, _)
| Fun(..)
| TypeDomain(_)
| ResourceDomain(_, _, _)
| Error
Expand Down
10 changes: 9 additions & 1 deletion third_party/move/move-binary-format/src/file_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -851,11 +851,19 @@ impl AbilitySet {
);
/// The empty ability set
pub const EMPTY: Self = Self(0);
/// Abilities for `Functions`
/// Minimal abilities for all `Functions`
pub const FUNCTIONS: AbilitySet = Self(Ability::Drop as u8);
/// Maximal abilities for all `Functions`. This is used for identity when unifying function types.
pub const MAXIMAL_FUNCTIONS: AbilitySet = Self::PUBLIC_FUNCTIONS;
/// Abilities for `Bool`, `U8`, `U64`, `U128`, and `Address`
pub const PRIMITIVES: AbilitySet =
Self((Ability::Copy as u8) | (Ability::Drop as u8) | (Ability::Store as u8));
/// Abilities for `private` user-defined/"primitive" functions (not closures).
/// These can be be changed in module upgrades, so should not be stored
pub const PRIVATE_FUNCTIONS: AbilitySet = Self((Ability::Copy as u8) | (Ability::Drop as u8));
/// Abilities for `public` user-defined/"primitive" functions (not closures)
pub const PUBLIC_FUNCTIONS: AbilitySet =
Self((Ability::Copy as u8) | (Ability::Drop as u8) | (Ability::Store as u8));
/// Abilities for `Reference` and `MutableReference`
pub const REFERENCES: AbilitySet = Self((Ability::Copy as u8) | (Ability::Drop as u8));
/// Abilities for `Signer`
Expand Down
55 changes: 44 additions & 11 deletions third_party/move/move-compiler-v2/src/bytecode_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Parts of the project are originally copyright © Meta Platforms, Inc.
// SPDX-License-Identifier: Apache-2.0

use crate::{experiments::Experiment, Options};
use codespan_reporting::diagnostic::Severity;
use ethnum::U256;
use itertools::Itertools;
Expand Down Expand Up @@ -324,6 +325,14 @@ impl<'env> Generator<'env> {
let loc = env.get_node_loc(id);
env.diag(severity, &loc, msg.as_ref())
}

fn check_if_lambdas_enabled(&self) -> bool {
let options = self
.env()
.get_extension::<Options>()
.expect("Options is available");
options.experiment_on(Experiment::LAMBDA_VALUES)
}
}

// ======================================================================================
Expand Down Expand Up @@ -480,14 +489,23 @@ impl<'env> Generator<'env> {
self.emit_with(*id, |attr| Bytecode::SpecBlock(attr, spec));
},
// TODO(LAMBDA)
ExpData::Lambda(id, _, _) => self.error(
ExpData::Lambda(id, _, _, _, _) =>
self.error(
*id,
"Function-typed values not yet supported except as parameters to calls to inline functions",
if self.check_if_lambdas_enabled() {
"Function-typed values not yet implemented except as parameters to calls to inline functions"
} else {
"Function-typed values not yet supported except as parameters to calls to inline functions"
}
),
// TODO(LAMBDA)
ExpData::Invoke(_, exp, _) => self.error(
exp.as_ref().node_id(),
"Calls to function values other than inline function parameters not yet supported",
ExpData::Invoke(id, _exp, _) => self.error(
*id,
if self.check_if_lambdas_enabled() {
"Calls to function values other than inline function parameters not yet implemented"
} else {
"Calls to function values other than inline function parameters not yet supported"
}
),
ExpData::Quant(id, _, _, _, _, _) => {
self.internal_error(*id, "unsupported specification construct")
Expand Down Expand Up @@ -564,6 +582,18 @@ impl<'env> Generator<'env> {
Constant::Bool(false)
}
},
// TODO(LAMBDA)
Value::Function(_mid, _fid) => {
self.error(
id,
if self.check_if_lambdas_enabled() {
"Function-typed values not yet implemented except as parameters to calls to inline functions"
} else {
"Function-typed values not yet supported except as parameters to calls to inline functions"
}
);
Constant::Bool(false)
},
}
}
}
Expand Down Expand Up @@ -785,6 +815,15 @@ impl<'env> Generator<'env> {
Operation::MoveFunction(m, f) => {
self.gen_function_call(targets, id, m.qualified(*f), args)
},
// TODO(LAMBDA)
Operation::EarlyBind => self.error(
id,
if self.check_if_lambdas_enabled() {
"Function-typed values not yet implemented except as parameters to calls to inline functions"
} else {
"Function-typed values not yet supported except as parameters to calls to inline functions"
},
),
Operation::TestVariants(mid, sid, variants) => {
self.gen_test_variants(targets, id, mid.qualified(*sid), variants, args)
},
Expand Down Expand Up @@ -813,12 +852,6 @@ impl<'env> Generator<'env> {

Operation::NoOp => {}, // do nothing

// TODO(LAMBDA)
Operation::Closure(..) => self.error(
id,
"Function-typed values not yet supported except as parameters to calls to inline functions",
),

// Non-supported specification related operations
Operation::Exists(Some(_))
| Operation::SpecFunction(_, _, _)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ fn find_possibly_modified_vars(
_ => {},
}
},
Lambda(node_id, pat, _) => {
Lambda(node_id, pat, _, _, _) => {
// Define a new scope for bound vars, and turn off `modifying` within.
match pos {
VisitorPosition::Pre => {
Expand Down Expand Up @@ -978,7 +978,8 @@ impl<'env> ExpRewriterFunctions for SimplifierRewriter<'env> {
let ability_set = self
.env()
.type_abilities(&ty, self.func_env.get_type_parameters_ref());
ability_set.has_ability(Ability::Drop)
// Don't drop a function-valued expression so we don't lose errors.
!ty.has_function() && ability_set.has_ability(Ability::Drop)
} else {
// We're missing type info, be conservative
false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ impl<'env, 'params> SymbolVisitor<'env, 'params> {
Pre | Post | BeforeBody | MidMutate | BeforeThen | BeforeElse
| PreSequenceValue => {},
},
Lambda(_, pat, _) => {
Lambda(_, pat, _, _, _) => {
match position {
Pre => self.seen_uses.enter_scope(),
Post => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use move_model::{
model::{FunId, FunctionEnv, GlobalEnv, Loc, ModuleEnv, NodeId, Parameter, QualifiedId},
ty::Type,
};
use std::{collections::BTreeSet, iter::Iterator, ops::Deref, vec::Vec};
use std::{collections::BTreeSet, iter::Iterator, vec::Vec};

type QualifiedFunId = QualifiedId<FunId>;

Expand All @@ -20,8 +20,8 @@ fn identify_function_types_with_functions_in_args(func_types: Vec<Type>) -> Vec<
func_types
.into_iter()
.filter_map(|ty| {
if let Type::Fun(argt, _) = &ty {
if argt.deref().has_function() {
if let Type::Fun(args, _, _) = &ty {
if args.as_ref().has_function() {
Some(ty)
} else {
None
Expand All @@ -41,8 +41,8 @@ fn identify_function_typed_params_with_functions_in_rets(
func_types
.iter()
.filter_map(|param| {
if let Type::Fun(_argt, rest) = &param.1 {
let rest_unboxed = rest.deref();
if let Type::Fun(_args, result, _) = &param.1 {
let rest_unboxed = result.as_ref();
if rest_unboxed.has_function() {
Some((*param, rest_unboxed))
} else {
Expand Down Expand Up @@ -270,7 +270,7 @@ fn check_privileged_operations_on_structs(env: &GlobalEnv, fun_env: &FunctionEnv
},
ExpData::Assign(_, pat, _)
| ExpData::Block(_, pat, _, _)
| ExpData::Lambda(_, pat, _) => {
| ExpData::Lambda(_, pat, _, _, _) => {
pat.visit_pre_post(&mut |_, pat| {
if let Pattern::Struct(id, str, _, _) = pat {
let module_id = str.module_id;
Expand Down Expand Up @@ -344,7 +344,7 @@ pub fn check_access_and_use(env: &mut GlobalEnv, before_inlining: bool) {

// Check that functions being called are accessible.
if let Some(def) = caller_func.get_def() {
let callees_with_sites = def.called_funs_with_callsites();
let callees_with_sites = def.used_funs_with_uses();
for (callee, sites) in &callees_with_sites {
let callee_func = env.get_function(*callee);
// Check visibility.
Expand Down
4 changes: 2 additions & 2 deletions third_party/move/move-compiler-v2/src/env_pipeline/inliner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub fn run_inlining(env: &mut GlobalEnv, scope: RewritingScope, keep_inline_func
let mut visited_targets = BTreeSet::new();
while let Some(target) = todo.pop_first() {
if visited_targets.insert(target.clone()) {
let callees_with_sites = target.called_funs_with_call_sites(env);
let callees_with_sites = target.used_funs_with_uses(env);
for (callee, sites) in callees_with_sites {
todo.insert(RewriteTarget::MoveFun(callee));
targets.entry(RewriteTarget::MoveFun(callee));
Expand Down Expand Up @@ -1161,7 +1161,7 @@ impl<'env, 'rewriter> ExpRewriterFunctions for InlinedRewriter<'env, 'rewriter>
};
let call_loc = self.env.get_node_loc(id);
if let Some(lambda_target) = optional_lambda_target {
if let ExpData::Lambda(_, pat, body) = lambda_target.as_ref() {
if let ExpData::Lambda(_, pat, body, _, _) = lambda_target.as_ref() {
let args_vec: Vec<Exp> = args.to_vec();
Some(InlinedRewriter::construct_inlined_call_expression(
self.env,
Expand Down
Loading
Loading