From 0d19e9062b5510272887bbb7ded37441efd6c918 Mon Sep 17 00:00:00 2001 From: Cam Swords Date: Thu, 30 May 2024 14:09:39 -0700 Subject: [PATCH] [move] Fix issue where varcalls could persist into typing without an error. (#17958) ## Description This fixes a bug in how var calls were being resolved across naming into typing, when the variable was not a valid macro call target. This also adds a `lambda` feature gate to make error reporting a bit nicer in these cases. ## Test plan New tests cover the crash/bug and ensure it doesn't occur now. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- .../crates/move-compiler/src/editions/mod.rs | 3 ++ .../move-compiler/src/naming/translate.rs | 45 +++++++++++++++++-- .../move-compiler/src/typing/translate.rs | 2 +- .../expansion/macro_identifier_missing.exp | 8 ++++ .../naming/macro_parameter_assignment.exp | 24 ++++++++++ .../naming/macro_var_as_fun_invalid.exp | 21 +++++++++ .../naming/macro_var_as_fun_invalid.move | 6 +++ .../move_2024/naming/var_as_fun_invalid.exp | 8 ++++ .../move_2024/naming/var_as_fun_invalid.move | 6 +++ .../move_2024/naming/var_as_fun_macro.exp | 9 ++++ .../move_2024/naming/var_as_fun_macro.move | 10 +++++ .../use_lambda_outside_call_invalid.exp | 8 ++++ .../feature_gate/macro_definition.exp | 8 ++++ .../macro_definition_with_usage.exp | 24 ++++++++++ .../macro_definition_with_usage.move | 5 +++ .../move_check/feature_gate/macro_lambda.exp | 2 +- .../move_check/naming/var_as_fun_invalid.exp | 16 +++++++ .../move_check/naming/var_as_fun_invalid.move | 6 +++ .../parser/spec_parsing_lambda_fail.exp | 2 +- 19 files changed, 207 insertions(+), 6 deletions(-) create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_var_as_fun_invalid.exp create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_var_as_fun_invalid.move create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_invalid.exp create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_invalid.move create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_macro.exp create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_macro.move create mode 100644 external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition_with_usage.exp create mode 100644 external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition_with_usage.move create mode 100644 external-crates/move/crates/move-compiler/tests/move_check/naming/var_as_fun_invalid.exp create mode 100644 external-crates/move/crates/move-compiler/tests/move_check/naming/var_as_fun_invalid.move diff --git a/external-crates/move/crates/move-compiler/src/editions/mod.rs b/external-crates/move/crates/move-compiler/src/editions/mod.rs index e4e5047a9f0b7..7afbce382a20a 100644 --- a/external-crates/move/crates/move-compiler/src/editions/mod.rs +++ b/external-crates/move/crates/move-compiler/src/editions/mod.rs @@ -50,6 +50,7 @@ pub enum FeatureGate { CleverAssertions, NoParensCast, TypeHoles, + Lambda, } #[derive(PartialEq, Eq, Clone, Copy, Debug, PartialOrd, Ord, Default)] @@ -152,6 +153,7 @@ const E2024_BETA_FEATURES: &[FeatureGate] = &[ FeatureGate::MacroFuns, FeatureGate::TypeHoles, FeatureGate::CleverAssertions, + FeatureGate::Lambda, ]; const DEVELOPMENT_FEATURES: &[FeatureGate] = &[]; @@ -276,6 +278,7 @@ impl FeatureGate { FeatureGate::CleverAssertions => "Clever `assert!`, `abort`, and `#[error]` are", FeatureGate::NoParensCast => "'as' without parentheses is", FeatureGate::TypeHoles => "'_' placeholders for type inference are", + FeatureGate::Lambda => "lambda expressions are", } } } diff --git a/external-crates/move/crates/move-compiler/src/naming/translate.rs b/external-crates/move/crates/move-compiler/src/naming/translate.rs index 38a68e913601a..de1b8a6a1079d 100644 --- a/external-crates/move/crates/move-compiler/src/naming/translate.rs +++ b/external-crates/move/crates/move-compiler/src/naming/translate.rs @@ -444,7 +444,12 @@ pub fn build_member_map( tyarg_arity, field_info, }; - assert!(members.insert(name.value(), M::Datatype(ResolvedDatatype::Struct(Box::new(struct_def)))).is_none()) + assert!(members + .insert( + name.value(), + M::Datatype(ResolvedDatatype::Struct(Box::new(struct_def))) + ) + .is_none()) } for (enum_name, edef) in mdef.enums.key_cloned_iter() { let tyarg_arity = edef.type_parameters.len(); @@ -743,7 +748,10 @@ impl<'env> Context<'env> { Some(e @ ResolvedModuleMember::Datatype(ResolvedDatatype::Enum(_))) => { let mut diag = make_invalid_module_member_kind_error(self, &EK::Function, mloc, &e); - diag.add_note("Enums cannot be instantiated directly. Instead, you must instantiate a variant."); + diag.add_note( + "Enums cannot be instantiated directly. \ + Instead, you must instantiate a variant.", + ); self.env.add_diag(diag); ResolvedCallSubject::Unbound } @@ -3949,6 +3957,10 @@ fn resolve_call( } } ResolvedCallSubject::Var(var) => { + context + .env + .check_feature(context.current_package, FeatureGate::Lambda, call_loc); + check_is_not_macro(context, is_macro, &var.value.name); let tyargs_opt = types_opt(context, TypeAnnotation::Expression, in_tyargs_opt); if tyargs_opt.is_some() { @@ -3960,7 +3972,34 @@ fn resolve_call( ), )); } - N::Exp_::VarCall(sp(subject_loc, var.value), args) + // If this variable refers to a local (num > 0) or it isn't syntax, error. + if !var.value.is_syntax_identifier() { + let name = var.value.name; + let msg = format!( + "Unexpected invocation of parameter or local '{name}'. \ + Non-syntax variables cannot be invoked as functions", + ); + let note = format!( + "Only macro syntax variables, e.g. '${name}', \ + may be invoked as functions." + ); + let mut diag = diag!(TypeSafety::InvalidCallTarget, (var.loc, msg)); + diag.add_note(note); + context.env.add_diag(diag); + N::Exp_::UnresolvedError + } else if var.value.id != 0 { + let msg = format!( + "Unexpected invocation of non-parameter variable '{}'. \ + Only lambda-typed syntax parameters may be invoked", + var.value.name + ); + context + .env + .add_diag(diag!(TypeSafety::InvalidCallTarget, (var.loc, msg))); + N::Exp_::UnresolvedError + } else { + N::Exp_::VarCall(sp(subject_loc, var.value), args) + } } ResolvedCallSubject::Unbound => N::Exp_::UnresolvedError, } diff --git a/external-crates/move/crates/move-compiler/src/typing/translate.rs b/external-crates/move/crates/move-compiler/src/typing/translate.rs index b680d8c9a98ba..20e9ed613d0bb 100644 --- a/external-crates/move/crates/move-compiler/src/typing/translate.rs +++ b/external-crates/move/crates/move-compiler/src/typing/translate.rs @@ -1643,7 +1643,7 @@ fn exp(context: &mut Context, ne: Box) -> Box { NE::Lambda(_) => { if context .env - .check_feature(context.current_package, FeatureGate::MacroFuns, eloc) + .check_feature(context.current_package, FeatureGate::Lambda, eloc) { let msg = "Lambdas can only be used directly as arguments to 'macro' functions"; context diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/expansion/macro_identifier_missing.exp b/external-crates/move/crates/move-compiler/tests/move_2024/expansion/macro_identifier_missing.exp index f19763d95e51e..f3ab5732351c1 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/expansion/macro_identifier_missing.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/expansion/macro_identifier_missing.exp @@ -28,3 +28,11 @@ error[E02010]: invalid name │ = 'macro' parameters start with '$' to indicate that their arguments are not evaluated before the macro is expanded, meaning the entire expression is substituted. This is different from regular function parameters that are evaluated before the function is called. +error[E04029]: invalid function call + ┌─ tests/move_2024/expansion/macro_identifier_missing.move:3:9 + │ +3 │ f(x) + │ ^ Unexpected invocation of parameter or local 'f'. Non-syntax variables cannot be invoked as functions + │ + = Only macro syntax variables, e.g. '$f', may be invoked as functions. + diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_parameter_assignment.exp b/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_parameter_assignment.exp index d021c94c93432..0b3194257b6b7 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_parameter_assignment.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_parameter_assignment.exp @@ -18,3 +18,27 @@ error[E02010]: invalid name │ = 'macro' parameters start with '$' to indicate that their arguments are not evaluated before the macro is expanded, meaning the entire expression is substituted. This is different from regular function parameters that are evaluated before the function is called. +error[E04029]: invalid function call + ┌─ tests/move_2024/naming/macro_parameter_assignment.move:5:9 + │ +5 │ f(x) + │ ^ Unexpected invocation of parameter or local 'f'. Non-syntax variables cannot be invoked as functions + │ + = Only macro syntax variables, e.g. '$f', may be invoked as functions. + +warning[W09005]: dead or unreachable code + ┌─ tests/move_2024/naming/macro_parameter_assignment.move:10:15 + │ +10 │ call!(|_| 1, 0) + 1; + │ ^^^^^ Unused macro argument. Its expression will not be type checked and it will not evaluated + │ + = This warning can be suppressed with '#[allow(dead_code)]' applied to the 'module' or module member ('const', 'fun', or 'struct') + +warning[W09005]: dead or unreachable code + ┌─ tests/move_2024/naming/macro_parameter_assignment.move:10:22 + │ +10 │ call!(|_| 1, 0) + 1; + │ ^ Unused macro argument. Its expression will not be type checked and it will not evaluated + │ + = This warning can be suppressed with '#[allow(dead_code)]' applied to the 'module' or module member ('const', 'fun', or 'struct') + diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_var_as_fun_invalid.exp b/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_var_as_fun_invalid.exp new file mode 100644 index 0000000000000..0bc6590ad7ca8 --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_var_as_fun_invalid.exp @@ -0,0 +1,21 @@ +error[E03004]: unbound type + ┌─ tests/move_2024/naming/macro_var_as_fun_invalid.move:3:13 + │ +3 │ let $var = 0; + │ ^^^^ Unbound type '$var' in current scope + +error[E01002]: unexpected token + ┌─ tests/move_2024/naming/macro_var_as_fun_invalid.move:3:18 + │ +3 │ let $var = 0; + │ ^ + │ │ + │ Unexpected '=' + │ Expected '{' + +error[E03005]: unbound unscoped name + ┌─ tests/move_2024/naming/macro_var_as_fun_invalid.move:4:9 + │ +4 │ $var(); + │ ^^^^ Unbound function '$var' in current scope + diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_var_as_fun_invalid.move b/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_var_as_fun_invalid.move new file mode 100644 index 0000000000000..5ccf4694cec6a --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/macro_var_as_fun_invalid.move @@ -0,0 +1,6 @@ +module a::test_panic { + public fun test_panic() { + let $var = 0; + $var(); + } +} diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_invalid.exp b/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_invalid.exp new file mode 100644 index 0000000000000..e2b263a8d63fe --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_invalid.exp @@ -0,0 +1,8 @@ +error[E04029]: invalid function call + ┌─ tests/move_2024/naming/var_as_fun_invalid.move:4:9 + │ +4 │ var(); + │ ^^^ Unexpected invocation of parameter or local 'var'. Non-syntax variables cannot be invoked as functions + │ + = Only macro syntax variables, e.g. '$var', may be invoked as functions. + diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_invalid.move b/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_invalid.move new file mode 100644 index 0000000000000..7d7e5fb38870f --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_invalid.move @@ -0,0 +1,6 @@ +module a::test_panic { + public fun test_panic() { + let var = 0; + var(); + } +} diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_macro.exp b/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_macro.exp new file mode 100644 index 0000000000000..c6235075cc25d --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_macro.exp @@ -0,0 +1,9 @@ +error[E04032]: unable to expand macro function + ┌─ tests/move_2024/naming/var_as_fun_macro.move:3:9 + │ +3 │ $var(); + │ ^^^^^^ Cannot call non-lambda argument + · +8 │ test_panic!(0) + │ - Expected a lambda argument + diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_macro.move b/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_macro.move new file mode 100644 index 0000000000000..501c8b2293d80 --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/naming/var_as_fun_macro.move @@ -0,0 +1,10 @@ +module a::test_panic { + public macro fun test_panic($var: u64): u64 { + $var(); + 0 + } + + public fun t(): u64 { + test_panic!(0) + } +} diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/typing/use_lambda_outside_call_invalid.exp b/external-crates/move/crates/move-compiler/tests/move_2024/typing/use_lambda_outside_call_invalid.exp index 3eb67c1438236..7f7bb1031686e 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/typing/use_lambda_outside_call_invalid.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/typing/use_lambda_outside_call_invalid.exp @@ -4,6 +4,14 @@ error[E04031]: invalid usage of lambda 3 │ let x = $f; │ ^^ Lambdas can only be used directly as arguments to 'macro' functions +error[E04029]: invalid function call + ┌─ tests/move_2024/typing/use_lambda_outside_call_invalid.move:4:9 + │ +4 │ x(); + │ ^ Unexpected invocation of parameter or local 'x'. Non-syntax variables cannot be invoked as functions + │ + = Only macro syntax variables, e.g. '$x', may be invoked as functions. + error[E02010]: invalid name ┌─ tests/move_2024/typing/use_lambda_outside_call_invalid.move:7:23 │ diff --git a/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition.exp b/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition.exp index c38434a529e10..707e8546f53c4 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition.exp @@ -6,3 +6,11 @@ error[E13001]: feature is not supported in specified edition │ = You can update the edition in the 'Move.toml', or via command line flag if invoking the compiler directly. +error[E13001]: feature is not supported in specified edition + ┌─ tests/move_check/feature_gate/macro_definition.move:2:38 + │ +2 │ public macro fun do($f: || ()) { $f() } + │ ^^^^ lambda expressions are not supported by current edition 'legacy', only '2024.alpha' and '2024.beta' support this feature + │ + = You can update the edition in the 'Move.toml', or via command line flag if invoking the compiler directly. + diff --git a/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition_with_usage.exp b/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition_with_usage.exp new file mode 100644 index 0000000000000..3a965f8c7d34f --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition_with_usage.exp @@ -0,0 +1,24 @@ +error[E13001]: feature is not supported in specified edition + ┌─ tests/move_check/feature_gate/macro_definition_with_usage.move:2:12 + │ +2 │ public macro fun do($f: || ()) { $f() } + │ ^^^^^ 'macro' functions are not supported by current edition 'legacy', only '2024.alpha' and '2024.beta' support this feature + │ + = You can update the edition in the 'Move.toml', or via command line flag if invoking the compiler directly. + +error[E13001]: feature is not supported in specified edition + ┌─ tests/move_check/feature_gate/macro_definition_with_usage.move:2:38 + │ +2 │ public macro fun do($f: || ()) { $f() } + │ ^^^^ lambda expressions are not supported by current edition 'legacy', only '2024.alpha' and '2024.beta' support this feature + │ + = You can update the edition in the 'Move.toml', or via command line flag if invoking the compiler directly. + +error[E13001]: feature is not supported in specified edition + ┌─ tests/move_check/feature_gate/macro_definition_with_usage.move:4:24 + │ +4 │ public fun t() { do!(|| q() ) } + │ ^ 'macro' functions are not supported by current edition 'legacy', only '2024.alpha' and '2024.beta' support this feature + │ + = You can update the edition in the 'Move.toml', or via command line flag if invoking the compiler directly. + diff --git a/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition_with_usage.move b/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition_with_usage.move new file mode 100644 index 0000000000000..09b3873256d90 --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_definition_with_usage.move @@ -0,0 +1,5 @@ +module a::m { + public macro fun do($f: || ()) { $f() } + public fun q() { } + public fun t() { do!(|| q() ) } +} diff --git a/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_lambda.exp b/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_lambda.exp index c736658aa624e..13484c9d8ba42 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_lambda.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/feature_gate/macro_lambda.exp @@ -10,7 +10,7 @@ error[E13001]: feature is not supported in specified edition ┌─ tests/move_check/feature_gate/macro_lambda.move:4:17 │ 4 │ let _ = |x| x; - │ ^^^^^ 'macro' functions are not supported by current edition 'legacy', only '2024.alpha' and '2024.beta' support this feature + │ ^^^^^ lambda expressions are not supported by current edition 'legacy', only '2024.alpha' and '2024.beta' support this feature │ = You can update the edition in the 'Move.toml', or via command line flag if invoking the compiler directly. diff --git a/external-crates/move/crates/move-compiler/tests/move_check/naming/var_as_fun_invalid.exp b/external-crates/move/crates/move-compiler/tests/move_check/naming/var_as_fun_invalid.exp new file mode 100644 index 0000000000000..1c201d0119535 --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_check/naming/var_as_fun_invalid.exp @@ -0,0 +1,16 @@ +error[E04029]: invalid function call + ┌─ tests/move_check/naming/var_as_fun_invalid.move:4:9 + │ +4 │ var(); + │ ^^^ Unexpected invocation of parameter or local 'var'. Non-syntax variables cannot be invoked as functions + │ + = Only macro syntax variables, e.g. '$var', may be invoked as functions. + +error[E13001]: feature is not supported in specified edition + ┌─ tests/move_check/naming/var_as_fun_invalid.move:4:9 + │ +4 │ var(); + │ ^^^^^ lambda expressions are not supported by current edition 'legacy', only '2024.alpha' and '2024.beta' support this feature + │ + = You can update the edition in the 'Move.toml', or via command line flag if invoking the compiler directly. + diff --git a/external-crates/move/crates/move-compiler/tests/move_check/naming/var_as_fun_invalid.move b/external-crates/move/crates/move-compiler/tests/move_check/naming/var_as_fun_invalid.move new file mode 100644 index 0000000000000..7d7e5fb38870f --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_check/naming/var_as_fun_invalid.move @@ -0,0 +1,6 @@ +module a::test_panic { + public fun test_panic() { + let var = 0; + var(); + } +} diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/spec_parsing_lambda_fail.exp b/external-crates/move/crates/move-compiler/tests/move_check/parser/spec_parsing_lambda_fail.exp index 35bf181f58faa..5b29108bca0f2 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/parser/spec_parsing_lambda_fail.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/spec_parsing_lambda_fail.exp @@ -2,7 +2,7 @@ error[E13001]: feature is not supported in specified edition ┌─ tests/move_check/parser/spec_parsing_lambda_fail.move:3:15 │ 3 │ let _ = |y| x + y; - │ ^^^^^^^^^ 'macro' functions are not supported by current edition 'legacy', only '2024.alpha' and '2024.beta' support this feature + │ ^^^^^^^^^ lambda expressions are not supported by current edition 'legacy', only '2024.alpha' and '2024.beta' support this feature │ = You can update the edition in the 'Move.toml', or via command line flag if invoking the compiler directly.