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] fix #14633: fix error messages for lambdas, identify change points for more complete lambda implementation #14742

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ Error: error[E04024]: invalid usage of function type
┌─ TEMPFILE:9:62
9 │ public fun for_each_ref<Element>(v: &vector<Element>, f: |&Element|) {
│ ^^^^^^^^^^ function type only allowed for inline function arguments
│ ^^^^^^^^^^ function-typed values only allowed for inline function arguments


28 changes: 21 additions & 7 deletions third_party/move/move-compiler-v2/src/bytecode_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,12 @@
),
);
}
self.emit_call(*id, targets, BytecodeOperation::WriteRef, vec![
lhs_temp, rhs_temp,
])
self.emit_call(
*id,
targets,
BytecodeOperation::WriteRef,
vec![lhs_temp, rhs_temp],
)
},
ExpData::Assign(id, lhs, rhs) => self.gen_assign(*id, lhs, rhs, None),
ExpData::Return(id, exp) => {
Expand Down Expand Up @@ -479,9 +482,16 @@
.rewrite_spec_descent(&SpecBlockTarget::Inline, spec);
self.emit_with(*id, |attr| Bytecode::SpecBlock(attr, spec));
},
ExpData::Invoke(id, _, _) | ExpData::Lambda(id, _, _) => {
self.internal_error(*id, format!("not yet implemented: {:?}", exp))
},
// TODO(LAMBDA)
ExpData::Lambda(id, _, _) => self.error(
*id,
"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",
),

Check warning on line 494 in third_party/move/move-compiler-v2/src/bytecode_generator.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/bytecode_generator.rs#L491-L494

Added lines #L491 - L494 were not covered by tests
ExpData::Quant(id, _, _, _, _, _) => {
self.internal_error(*id, "unsupported specification construct")
},
Expand Down Expand Up @@ -806,7 +816,11 @@

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

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

Check warning on line 823 in third_party/move/move-compiler-v2/src/bytecode_generator.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/bytecode_generator.rs#L820-L823

Added lines #L820 - L823 were not covered by tests

// Non-supported specification related operations
Operation::Exists(Some(_))
Expand Down
189 changes: 157 additions & 32 deletions third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,55 +3,180 @@

//! Do a few checks of functions and function calls.

use crate::Options;
use crate::{experiments::Experiment, Options};
use codespan_reporting::diagnostic::Severity;
use move_binary_format::file_format::Visibility;
use move_model::{
ast::{ExpData, Operation, Pattern},
model::{FunId, FunctionEnv, GlobalEnv, Loc, ModuleEnv, NodeId, QualifiedId},
model::{FunId, FunctionEnv, GlobalEnv, Loc, ModuleEnv, NodeId, Parameter, QualifiedId},
ty::Type,
};
use std::{collections::BTreeSet, iter::Iterator, vec::Vec};
use std::{collections::BTreeSet, iter::Iterator, ops::Deref, vec::Vec};

type QualifiedFunId = QualifiedId<FunId>;

/// check that non-inline function parameters do not have function type.
// Takes a list of function types, returns those which have a function type in their argument type
fn identify_function_types_with_functions_in_args(func_types: Vec<Type>) -> Vec<Type> {
func_types
.into_iter()
.filter_map(|ty| {
if let Type::Fun(argt, _) = &ty {
if argt.deref().has_function() {
Some(ty)

Check warning on line 25 in third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs#L25

Added line #L25 was not covered by tests
} else {
None
}
} else {
None

Check warning on line 30 in third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs#L30

Added line #L30 was not covered by tests
}
})
.collect()
}

// Takes a list of function-typed parameters, along with argument and result type
// Returns a list of any parameters whose result type has a function value, along with that result type.
fn identify_function_typed_params_with_functions_in_rets(
func_types: Vec<&Parameter>,
) -> Vec<(&Parameter, &Type)> {
func_types
.iter()
.filter_map(|param| {
if let Type::Fun(_argt, rest) = &param.1 {
let rest_unboxed = rest.deref();
if rest_unboxed.has_function() {
Some((*param, rest_unboxed))
} else {
None
}
} else {
None

Check warning on line 52 in third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs#L52

Added line #L52 was not covered by tests
}
})
.collect()
}

/// check that function parameters/results do not have function type unless allowed.
/// (1) is there a function type arg at the top level? This is allowed for inline or LAMBDA_IN_PARAMS
/// (2) is there a function type result at the top level? This is allowed only for LAMBDA_IN_RETURNS
/// (3) is there *any* function type with function type in an arg? This is allowed only for LAMBDA_IN_PARAMS
/// (4) is there *any* function type with function type in a result? This is allowed only for LAMBDA_IN_RETURNS
pub fn check_for_function_typed_parameters(env: &mut GlobalEnv) {
let options = env
.get_extension::<Options>()
.expect("Options is available");
let lambda_params_ok = options.experiment_on(Experiment::LAMBDA_IN_PARAMS);
let lambda_return_ok = options.experiment_on(Experiment::LAMBDA_IN_RETURNS);
if lambda_params_ok && lambda_return_ok {
return;
}

for caller_module in env.get_modules() {
if caller_module.is_primary_target() {
for caller_func in caller_module.get_functions() {
// Check that non-inline function parameters don't have function type
if !caller_func.is_inline() {
let parameters = caller_func.get_parameters();
let bad_params: Vec<_> = parameters
if !lambda_params_ok || !lambda_return_ok {
let caller_name = caller_func.get_full_name_str();
let return_type = caller_func.get_result_type();
let func_returns: Vec<_> = return_type
.clone()
.flatten()
.into_iter()
.filter(|t| t.is_function())
.collect();
let type_display_ctx = caller_func.get_type_display_ctx();
if !func_returns.is_empty() {
// (2) is there a function type result at the top level? This is allowed
// only for LAMBDA_IN_RETURNS
if !lambda_return_ok && !func_returns.is_empty() {
env.diag(
Severity::Error,
&caller_func.get_result_type_loc(),
&format!("Functions may not return function-typed values, but function `{}` return type is the function type `{}`:",
&caller_name,
return_type.display(&type_display_ctx)),
)
}

Check warning on line 97 in third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs#L97

Added line #L97 was not covered by tests
if !lambda_params_ok {
// (3) is there *any* function type with function type in an arg? This
// is allowed only for LAMBDA_IN_PARAMS
let bad_returns =
identify_function_types_with_functions_in_args(func_returns);
if !bad_returns.is_empty() {
env.diag(
Severity::Error,
&caller_func.get_result_type_loc(),
&format!("Non-inline functions may not take function-typed parameters, but function `{}` return type is `{}`, which has a function type taking a function parameter:",
&caller_name,
return_type.display(&type_display_ctx)),
)

Check warning on line 110 in third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs#L104-L110

Added lines #L104 - L110 were not covered by tests
}
}

Check warning on line 112 in third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs#L112

Added line #L112 was not covered by tests
}

let parameters = caller_func.get_parameters_ref();
let func_params: Vec<_> = parameters
.iter()
.filter(|param| matches!(param.1, Type::Fun(_, _)))
.filter(|param| matches!(param.1, Type::Fun(..)))
.collect();
if !bad_params.is_empty() {
let caller_name = caller_func.get_full_name_str();
let reasons: Vec<(Loc, String)> = bad_params
.iter()
.map(|param| {
(
param.2.clone(),
format!(
"Parameter `{}` has a function type.",
param.0.display(env.symbol_pool()),
if !func_params.is_empty() {
// (1) is there a function type arg at the top level? This is allowed for
// inline or LAMBDA_IN_PARAMS
if !caller_func.is_inline() && !lambda_params_ok {
let reasons: Vec<(Loc, String)> = func_params
.iter()
.map(|param| {
(
param.2.clone(),
format!(
"Parameter `{}` has function-valued type `{}`.",
param.0.display(env.symbol_pool()),
param.1.display(&type_display_ctx)
),
)
})
.collect();
env.diag_with_labels(
Severity::Error,
&caller_func.get_id_loc(),
&format!("Only inline functions may have function-typed parameters, but non-inline function `{}` has {}:",
caller_name,
if reasons.len() > 1 { "function parameters" } else { "a function parameter" },
),
reasons,
);
}
if !lambda_return_ok {
// (4) is there *any* function type with function type in its result? This is
// allowed only for LAMBDA_IN_RETURNS
let bad_params =
identify_function_typed_params_with_functions_in_rets(func_params);
if !bad_params.is_empty() {
let reasons: Vec<(Loc, String)> = bad_params
.iter()
.map(|(param, ty)| {
(
param.2.clone(),
format!(
"Parameter `{}` has type `{}`, which has function type `{}` as a function result type",
param.0.display(env.symbol_pool()),
param.1.display(&type_display_ctx),
ty.display(&type_display_ctx),
),
)
})
.collect();
env.diag_with_labels(
Severity::Error,
&caller_func.get_id_loc(),
&format!("Functions may not return function-typed values, but function `{}` has {} of function type with function-typed result:",
caller_name,
if reasons.len() > 1 { "parameters" } else { "a parameter" },
),
)
})
.collect();
env.diag_with_labels(
Severity::Error,
&caller_func.get_id_loc(),
&format!("Only inline functions may have function-typed parameters, but non-inline function `{}` has {}:",
caller_name,
if reasons.len() > 1 { "function parameters" } else { "a function parameter" },
),
reasons,
);
reasons,
);
}
}

Check warning on line 177 in third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs#L177

Added line #L177 was not covered by tests
}
}
};

Check warning on line 179 in third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/env_pipeline/function_checker.rs#L179

Added line #L179 was not covered by tests
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ impl<'a> ExpRewriterFunctions for LambdaLifter<'a> {
env.error(
&loc,
&format!(
"captured variable `{}` cannot be modified inside of a lambda",
"captured variable `{}` cannot be modified inside of a lambda", // TODO(LAMBDA)
name.display(env.symbol_pool())
),
);
Expand All @@ -327,7 +327,7 @@ impl<'a> ExpRewriterFunctions for LambdaLifter<'a> {
env.error(
&loc,
&format!(
"captured variable `{}` cannot be modified inside of a lambda",
"captured variable `{}` cannot be modified inside of a lambda", // TODO(LAMBDA)
name.display(env.symbol_pool())
),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl<'a> RecursiveStructChecker<'a> {
self.report_invalid_field(&struct_env, &field_env);
}
},
Type::Primitive(_) | Type::TypeParameter(_) => {},
Type::Primitive(_) | Type::TypeParameter(_) | Type::Fun(..) => {},
_ => unreachable!("invalid field type"),
}
path.pop();
Expand Down Expand Up @@ -195,7 +195,7 @@ impl<'a> RecursiveStructChecker<'a> {
.iter()
.any(|ty| self.ty_contains_struct(path, ty, loc.clone(), struct_id, checked))
},
Type::Primitive(_) | Type::TypeParameter(_) => false,
Type::Primitive(_) | Type::TypeParameter(_) | Type::Fun(..) => false,
_ => panic!("ICE: {:?} used as a type parameter", ty),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,16 @@
fn used_type_parameters_in_ty(ty: &Type) -> BTreeSet<u16> {
match ty {
Type::Primitive(_) => BTreeSet::new(),
Type::Struct(_, _, tys) => tys.iter().flat_map(used_type_parameters_in_ty).collect(),
Type::Tuple(tys) | Type::Struct(_, _, tys) => {
tys.iter().flat_map(used_type_parameters_in_ty).collect()
},
Type::TypeParameter(i) => BTreeSet::from([*i]),
Type::Vector(ty) => used_type_parameters_in_ty(ty),
Type::Fun(t1, t2) => [t1, t2]
vineethk marked this conversation as resolved.
Show resolved Hide resolved
.iter()
.flat_map(|t| used_type_parameters_in_ty(t))
.collect(),

Check warning on line 66 in third_party/move/move-compiler-v2/src/env_pipeline/unused_params_checker.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/env_pipeline/unused_params_checker.rs#L63-L66

Added lines #L63 - L66 were not covered by tests
Type::Reference(..)
| Type::Fun(..)
| Type::Tuple(..)
| Type::TypeDomain(..)
| Type::ResourceDomain(..)
| Type::Error
Expand Down
25 changes: 25 additions & 0 deletions third_party/move/move-compiler-v2/src/experiments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,32 @@ pub static EXPERIMENTS: Lazy<BTreeMap<String, Experiment>> = Lazy::new(|| {
description: "Turns on or off specification rewriting".to_string(),
default: Given(false),
},
Experiment {
name: Experiment::LAMBDA_FIELDS.to_string(),
description: "Turns on or off function values in struct fields".to_string(),
default: Given(false),
},
Experiment {
name: Experiment::LAMBDA_LIFTING.to_string(),
description: "Turns on or off lambda lifting".to_string(),
default: Given(false),
},
Experiment {
name: Experiment::LAMBDA_IN_PARAMS.to_string(),
description: "Turns on or off function values as parameters to non-inline functions"
.to_string(),
default: Given(false),
},
Experiment {
name: Experiment::LAMBDA_IN_RETURNS.to_string(),
description: "Turns on or off function values in function return values".to_string(),
default: Given(false),
},
Experiment {
name: Experiment::LAMBDA_VALUES.to_string(),
description: "Turns on or off first-class function values".to_string(),
default: Given(false),
},
Experiment {
name: Experiment::RECURSIVE_TYPE_CHECK.to_string(),
description: "Turns on or off checking of recursive structs and type instantiations"
Expand Down Expand Up @@ -275,7 +296,11 @@ impl Experiment {
pub const INLINING: &'static str = "inlining";
pub const KEEP_INLINE_FUNS: &'static str = "keep-inline-funs";
pub const KEEP_UNINIT_ANNOTATIONS: &'static str = "keep-uninit-annotations";
pub const LAMBDA_FIELDS: &'static str = "lambda-fields";
pub const LAMBDA_IN_PARAMS: &'static str = "lambda-in-params";
pub const LAMBDA_IN_RETURNS: &'static str = "lambda-in-returns";
pub const LAMBDA_LIFTING: &'static str = "lambda-lifting";
pub const LAMBDA_VALUES: &'static str = "lambda-values";
pub const LINT_CHECKS: &'static str = "lint-checks";
pub const OPTIMIZE: &'static str = "optimize";
pub const OPTIMIZE_EXTRA: &'static str = "optimize-extra";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,18 @@
ReferenceKind::Mutable => FF::SignatureToken::MutableReference(target_ty),
}
},
Fun(_, _) | TypeDomain(_) | ResourceDomain(_, _, _) | Error | Var(_) => {
Fun(_param_ty, _result_ty) => {
// TODO(LAMBDA)
ctx.error(
loc,
format!(
"Unexpected type: {}",
ty.display(&ctx.env.get_type_display_ctx())
),
);
FF::SignatureToken::Bool

Check warning on line 377 in third_party/move/move-compiler-v2/src/file_format_generator/module_generator.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/file_format_generator/module_generator.rs#L368-L377

Added lines #L368 - L377 were not covered by tests
},
TypeDomain(_) | ResourceDomain(_, _, _) | Error | Var(_) => {
ctx.internal_error(
loc,
format!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module 0x42::assign {
}
struct S {
f: u64,
g: 0x42::assign::T,
g: T,
}
private fun assign_field(s: &mut S,f: u64) {
select assign::S.f<&mut S>(s) = f;
Expand Down
Loading
Loading