[move-compiler-v2] fix #14633: fix error messages for lambdas, identi…

…fy change points for more complete lambda implementation (#14742)

Catch all errors in move-compiler-v2/tests/checking/typing/lambda.move in Compiler V2,
after splitting and gradually commenting out code as lambda[2-5].move,
to avoid shadowing by other errors. Lay groundwork for support of lambdas in various capacities.

Fixes #14633.

- add checking for function-typed function results in function_checker, and functions in parameters and results of function types in both args and results to functions there.
- to properly show "error" rather than "bug" if lambdas are mistakenly used as values today.
- tag some code to show where changes are needed to support lambda: // TODO(LAMBDA) fix unused_params_checker and recursive_struct_checker to tolerate lambda types
- add experiments to control enabling of various aspects of lambda support:
- add a result_type_loc field to move-model's FunctionData (and model-builder's FunEntry) to more precisely locate errors in function result types.
- Fix GlobalEnv::internal_dump_env to use function- and struct-specific type contexts so that types are shown with correct type parameter names.
- Check that struct fields are not functions.

third_party/move/move-bytecode-verifier/transactional-tests/tests/type_safety/gerbens_test.exp

@@ -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
 
 

third_party/move/move-compiler-v2/src/bytecode_generator.rs

@@ -406,9 +406,12 @@ impl<'env> Generator<'env> {
                         ),
                     );
                 }
-                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) => {
@@ -479,9 +482,16 @@ impl<'env> Generator<'env> {
                     .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",
+            ),
             ExpData::Quant(id, _, _, _, _, _) => {
                 self.internal_error(*id, "unsupported specification construct")
             },
@@ -806,7 +816,11 @@ impl<'env> Generator<'env> {
 
             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",
+            ),
 
             // Non-supported specification related operations
             Operation::Exists(Some(_))

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

@@ -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)
+                } else {
+                    None
+                }
+            } else {
+                None
+            }
+        })
+        .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
+            }
+        })
+        .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)),
+                            )
+                        }
+                        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)),
+                                )
+                            }
+                        }
+                    }
+
+                    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,
+                                );
+                            }
+                        }
                     }
-                }
+                };
             }
         }
     }

third_party/move/move-compiler-v2/src/env_pipeline/lambda_lifter.rs

@@ -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())
                     ),
                 );
@@ -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())
                     ),
                 );

third_party/move/move-compiler-v2/src/env_pipeline/recursive_struct_checker.rs

@@ -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();
@@ -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),
         }
     }

third_party/move/move-compiler-v2/src/env_pipeline/unused_params_checker.rs

@@ -55,12 +55,16 @@ fn used_type_parameters_in_fields(struct_env: &StructEnv) -> BTreeSet<u16> {
 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]
+            .iter()
+            .flat_map(|t| used_type_parameters_in_ty(t))
+            .collect(),
         Type::Reference(..)
-        | Type::Fun(..)
-        | Type::Tuple(..)
         | Type::TypeDomain(..)
         | Type::ResourceDomain(..)
         | Type::Error

third_party/move/move-compiler-v2/src/experiments.rs

@@ -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"
@@ -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";

third_party/move/move-compiler-v2/src/file_format_generator/module_generator.rs

@@ -365,7 +365,18 @@ impl ModuleGenerator {
                     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
+            },
+            TypeDomain(_) | ResourceDomain(_, _, _) | Error | Var(_) => {
                 ctx.internal_error(
                     loc,
                     format!(

third_party/move/move-compiler-v2/tests/bytecode-generator/assign.exp

@@ -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;