Skip to content

Commit

Permalink
[move] Fix issue where varcalls could persist into typing without an …
Browse files Browse the repository at this point in the history
…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:
  • Loading branch information
cgswords authored May 30, 2024
1 parent ba8e10c commit 0d19e90
Show file tree
Hide file tree
Showing 19 changed files with 207 additions and 6 deletions.
3 changes: 3 additions & 0 deletions external-crates/move/crates/move-compiler/src/editions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub enum FeatureGate {
CleverAssertions,
NoParensCast,
TypeHoles,
Lambda,
}

#[derive(PartialEq, Eq, Clone, Copy, Debug, PartialOrd, Ord, Default)]
Expand Down Expand Up @@ -152,6 +153,7 @@ const E2024_BETA_FEATURES: &[FeatureGate] = &[
FeatureGate::MacroFuns,
FeatureGate::TypeHoles,
FeatureGate::CleverAssertions,
FeatureGate::Lambda,
];

const DEVELOPMENT_FEATURES: &[FeatureGate] = &[];
Expand Down Expand Up @@ -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",
}
}
}
Expand Down
45 changes: 42 additions & 3 deletions external-crates/move/crates/move-compiler/src/naming/translate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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() {
Expand All @@ -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,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1643,7 +1643,7 @@ fn exp(context: &mut Context, ne: Box<N::Exp>) -> Box<T::Exp> {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Original file line number Diff line number Diff line change
Expand Up @@ -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')

Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module a::test_panic {
public fun test_panic() {
let $var = 0;
$var();
}
}
Original file line number Diff line number Diff line change
@@ -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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module a::test_panic {
public fun test_panic() {
let var = 0;
var();
}
}
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Original file line number Diff line number Diff line change
@@ -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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module a::m {
public macro fun do($f: || ()) { $f() }
public fun q() { }
public fun t() { do!(|| q() ) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Original file line number Diff line number Diff line change
@@ -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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module a::test_panic {
public fun test_panic() {
let var = 0;
var();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.

0 comments on commit 0d19e90

Please sign in to comment.