Skip to content

Commit

Permalink
[move-2024] Slighlty improve type mismatch errors in syntax index err…
Browse files Browse the repository at this point in the history 
…ors (MystenLabs#16429)

## Description 

This does a bit more work in syntax index function checking to produce
nicer errors when types disagree.

## Test Plan 

Updated expectation files

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
@cgswords
cgswords authored Feb 29, 2024
1 parent 1c018cd commit d54ac2f
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 56 deletions.
72 changes: 51 additions & 21 deletions external-crates/move/crates/move-compiler/src/typing/syntax_methods.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@
//! trait-like constraints around their definitions. We process them here, using typing machinery
//! to ensure the are.

use std::collections::{BTreeMap, BTreeSet};

use crate::{
diag,
diagnostics::Diagnostic,
expansion::ast::ModuleIdent,
ice,
naming::ast::{self as N, IndexSyntaxMethods, SyntaxMethod},
typing::core::{self, Context},
};
use move_ir_types::location::*;
use std::collections::{BTreeMap, BTreeSet};

//-------------------------------------------------------------------------------------------------
// Validation
Expand Down Expand Up @@ -172,12 +172,9 @@ fn validate_index_syntax_methods(
Some(mut_tparam_types),
);

let mut param_tys = index_ty
.params
.iter()
.map(|(_, t1)| t1)
.zip(mut_finfo.signature.parameters.iter().map(|(_, _, ty)| ty))
.enumerate();
let index_params = index_ty.params.iter().map(|(_, t1)| t1);
let mut_params = mut_finfo.signature.parameters.iter().map(|(_, _, ty)| ty);
let mut param_tys = index_params.zip(mut_params).enumerate();

let mut subst = std::mem::replace(&mut context.subst, core::Subst::empty());

Expand All @@ -197,14 +194,26 @@ fn validate_index_syntax_methods(
"This index function subject has type {}",
ty_str(index_type)
);
let N::Type_::Ref(false, inner) =
core::ready_tvars(&subst, subject_ref_type.clone()).value
else {
context.env.add_diag(ice!((
index_finfo.signature.return_type.loc,
"This index function got to type verification with an invalid type"
)));
return false;
};
let expected_type = sp(mut_type.loc, N::Type_::Ref(true, inner.clone()));
let mut_msg = format!(
"This mutable index function subject has type {}",
ty_str(mut_type)
"Expected this mutable index function subject to have type {}",
ty_str(&expected_type)
);
let mut_msg_2 = format!("It has type {}", ty_str(mut_type));
let mut diag = diag!(
TypeSafety::IncompatibleSyntaxMethods,
(index_type.loc, index_msg),
(mut_type.loc, mut_msg)
(mut_type.loc, mut_msg),
(mut_type.loc, mut_msg_2)
);
add_type_param_info(
&mut diag,
Expand All @@ -230,15 +239,17 @@ fn validate_index_syntax_methods(
} else {
let (_, _, index_type) = &index_finfo.signature.parameters[ndx];
let (_, _, mut_type) = &mut_finfo.signature.parameters[ndx];
let index_msg = format!("This index function expects type {}", ty_str(index_type));
let index_msg = format!("This parameter has type {}", ty_str(index_type));
let mut_msg = format!(
"This mutable index function expects type {}",
ty_str(mut_type)
"Expected this parameter to have type {}",
ty_str(&core::ready_tvars(&subst, ptype.clone()))
);
let mut_msg_2 = format!("It has type {}", ty_str(mut_type));
let mut diag = diag!(
TypeSafety::IncompatibleSyntaxMethods,
(index_type.loc, index_msg),
(mut_type.loc, mut_msg)
(mut_type.loc, mut_msg),
(mut_type.loc, mut_msg_2)
);
add_type_param_info(
&mut diag,
Expand All @@ -256,18 +267,35 @@ fn validate_index_syntax_methods(
// Similar to the subject type, we ensure the return types are the same. We already checked
// that they are appropriately-shaped references, and now we ensure they refer to the same type
// under the reference.
if core::subtype(subst, &mut_finfo.signature.return_type, &index_ty.return_).is_err() {
if core::subtype(
subst.clone(),
&mut_finfo.signature.return_type,
&index_ty.return_,
)
.is_err()
{
let index_type = &index_finfo.signature.return_type;
let mut_type = &mut_finfo.signature.return_type;
let index_msg = format!("This index function returns type {}", ty_str(index_type));
let N::Type_::Ref(false, inner) = core::ready_tvars(&subst, index_ty.return_.clone()).value
else {
context.env.add_diag(ice!((
index_finfo.signature.return_type.loc,
"This index function got to type verification with an invalid type"
)));
return false;
};
let expected_type = sp(mut_type.loc, N::Type_::Ref(true, inner.clone()));
let mut_msg = format!(
"This mutable index function returns type {}",
ty_str(mut_type)
"Expected this mutable index function to return type {}",
ty_str(&expected_type)
);
let mut_msg_2 = format!("It returns type {}", ty_str(mut_type));
let mut diag = diag!(
TypeSafety::IncompatibleSyntaxMethods,
(index_type.loc, index_msg),
(mut_type.loc, mut_msg)
(mut_type.loc, mut_msg),
(mut_type.loc, mut_msg_2)
);
add_type_param_info(
&mut diag,
Expand Down Expand Up @@ -339,8 +367,10 @@ fn type_param_positions(
.into_iter()
.filter_map(|tparam| {
if let Some(posn) = tparams.iter().position(|t| t == &tparam) {
let declared_loc = tparams[posn].user_specified_name.loc;
Some((tparam.user_specified_name, (posn, declared_loc)))
Some((
tparam.user_specified_name,
(posn, tparam.user_specified_name.loc),
))
} else {
None
}
Expand Down
5 changes: 4 additions & 1 deletion ...move/crates/move-compiler/tests/move_2024/naming/index_syntax_methods_return_mismatch.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ error[E04034]: 'syntax' method types differ
│ ^^^^ This index function returns type '&u64'
·
62 │ public fun borrow_q_mut(s: &mut S, i: u64): &mut u32 {
│ -------- This mutable index function returns type '&mut u32'
│ --------
│ │
│ Expected this mutable index function to return type '&mut u64'
│ It returns type '&mut u32'
│
= These functions must return the same type, differing only by mutability

Expand Down
7 changes: 5 additions & 2 deletions ...nal-crates/move/crates/move-compiler/tests/move_2024/typing/primitive_index_invalid_2.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/primitive_index_invalid_2.move:5:64
│
5 │ native public fun vborrow<Element>(v: &vector<Element>, i: u64): &Element;
│ ^^^ This index function expects type 'u64'
│ ^^^ This parameter has type 'u64'
6 │ #[syntax(index)]
7 │ native public fun vborrow_mut<Element>(v: &mut vector<Element>, i: u32): &mut Element;
│ --- This mutable index function expects type 'u32'
│ ---
│ │
│ Expected this parameter to have type 'u64'
│ It has type 'u32'
│
= Index operation non-subject parameter types must match exactly

63 changes: 45 additions & 18 deletions ...l-crates/move/crates/move-compiler/tests/move_2024/typing/syntax_methods_args_invalid.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_invalid.move:7:39
│
7 │ public fun borrow<T>(s: &S<T>, i: u64): &u64 { abort 0 }
│ ^^^ This index function expects type 'u64'
│ ^^^ This parameter has type 'u64'
·
11 │ public fun borrow_mut<T>(s: &mut S<T>, i: T): &mut u64 { abort 0 }
│ - This mutable index function expects type 'T'
│ -
│ │
│ Expected this parameter to have type 'u64'
│ It has type 'T'
│
= Index operation non-subject parameter types must match exactly

Expand All @@ -24,10 +27,13 @@ error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_invalid.move:34:39
│
34 │ public fun borrow<T>(s: &S<T>, i: u64, j: T): &T { abort 0 }
│ ^^^ This index function expects type 'u64'
│ ^^^ This parameter has type 'u64'
·
38 │ public fun borrow_mut<T>(s: &mut S<T>, i: u32, j: T): &mut T { abort 0 }
│ --- This mutable index function expects type 'u32'
│ ---
│ │
│ Expected this parameter to have type 'u64'
│ It has type 'u32'
│
= Index operation non-subject parameter types must match exactly

Expand All @@ -46,76 +52,97 @@ error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_invalid.move:62:41
│
62 │ public fun borrow<T,Q>(s: &S<T>, i: u64, j: Q): &T { abort 0 }
│ ^^^ This index function expects type 'u64'
│ ^^^ This parameter has type 'u64'
·
66 │ public fun borrow_mut<T,Q>(s: &mut S<T>, i: u32, j: T): &mut T { abort 0 }
│ --- This mutable index function expects type 'u32'
│ ---
│ │
│ Expected this parameter to have type 'u64'
│ It has type 'u32'
│
= Index operation non-subject parameter types must match exactly

error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_invalid.move:62:49
│
62 │ public fun borrow<T,Q>(s: &S<T>, i: u64, j: Q): &T { abort 0 }
│ ^ This index function expects type 'Q'
│ ^ This parameter has type 'Q'
·
66 │ public fun borrow_mut<T,Q>(s: &mut S<T>, i: u32, j: T): &mut T { abort 0 }
│ - This mutable index function expects type 'T'
│ -
│ │
│ Expected this parameter to have type 'Q'
│ It has type 'T'
│
= Index operation non-subject parameter types must match exactly

error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_invalid.move:76:41
│
76 │ public fun borrow<T,R>(s: &S<T>, i: R, j: T): &T { abort 0 }
│ ^ This index function expects type 'R'
│ ^ This parameter has type 'R'
·
80 │ public fun borrow_mut<T,R>(s: &mut S<T>, i: T, j: T): &mut T { abort 0 }
│ - This mutable index function expects type 'T'
│ -
│ │
│ Expected this parameter to have type 'R'
│ It has type 'T'
│
= Index operation non-subject parameter types must match exactly

error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_invalid.move:90:39
│
90 │ public fun borrow<T>(s: &S<T>, i: u64): &T {
│ ^^^ This index function expects type 'u64'
│ ^^^ This parameter has type 'u64'
·
95 │ public fun borrow_mut<T>(s: &mut S<T>, i: u32): &mut T {
│ --- This mutable index function expects type 'u32'
│ ---
│ │
│ Expected this parameter to have type 'u64'
│ It has type 'u32'
│
= Index operation non-subject parameter types must match exactly

error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_invalid.move:107:39
│
107 │ public fun borrow<T>(s: &S<T>, i: &u64, j: T): &T { abort 0 }
│ ^^^^ This index function expects type '&u64'
│ ^^^^ This parameter has type '&u64'
·
111 │ public fun borrow_mut<T>(s: &mut S<T>, i: &mut u64, j: T): &mut T { abort 0 }
│ -------- This mutable index function expects type '&mut u64'
│ --------
│ │
│ Expected this parameter to have type '&u64'
│ It has type '&mut u64'
│
= Index operation non-subject parameter types must match exactly

error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_invalid.move:121:48
│
121 │ public fun borrow<T>(s: &S<T>, i: &u64, j: &T): &T { abort 0 }
│ ^^ This index function expects type '&T'
│ ^^ This parameter has type '&T'
·
125 │ public fun borrow_mut<T>(s: &mut S<T>, i: &u64, j: &mut T): &mut T { abort 0 }
│ ------ This mutable index function expects type '&mut T'
│ ------
│ │
│ Expected this parameter to have type '&T'
│ It has type '&mut T'
│
= Index operation non-subject parameter types must match exactly

error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_invalid.move:135:48
│
135 │ public fun borrow<T>(s: &S<T>, i: &u64, j: &mut T): &T { abort 0 }
│ ^^^^^^ This index function expects type '&mut T'
│ ^^^^^^ This parameter has type '&mut T'
·
139 │ public fun borrow_mut<T>(s: &mut S<T>, i: &u64, j: &T): &mut T { abort 0 }
│ -- This mutable index function expects type '&T'
│ --
│ │
│ Expected this parameter to have type '&mut T'
│ It has type '&T'
│
= Index operation non-subject parameter types must match exactly

21 changes: 14 additions & 7 deletions ...e/crates/move-compiler/tests/move_2024/typing/syntax_methods_args_polymorphic_invalid.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_polymorphic_invalid.move:6:43
│
6 │ public fun borrow<T,Q>(_s: &S<T>, _j: Q): &T { abort 0 }
│ ^ This index function expects type 'Q'
│ ^ This parameter has type 'Q'
·
9 │ public fun borrow_mut<T,Q>(_s: &mut S<T>, _j: T): &mut T { abort 0 }
│ - This mutable index function expects type 'T'
│ -
│ │
│ Expected this parameter to have type 'Q'
│ It has type 'T'
│
= Index operation non-subject parameter types must match exactly

Expand All @@ -18,8 +21,10 @@ error[E04034]: 'syntax' method types differ
│ Type parameter T appears in position 2 here
15 │ #[syntax(index)]
16 │ public fun borrow_muAt<T,Q>(_s: &mut A<T>, _j: T): &mut T { abort 0 }
│ - --------- This mutable index function subject has type '&mut a::invalid::A<T>'
│ │
│ - ---------
│ │ │
│ │ Expected this mutable index function subject to have type '&mut a::invalid::A<Q>'
│ │ It has type '&mut a::invalid::A<T>'
│ Type parameter T appears in position 1 here
│
= Type parameters must be used the same by position, not name
Expand All @@ -29,13 +34,15 @@ error[E04034]: 'syntax' method types differ
┌─ tests/move_2024/typing/syntax_methods_args_polymorphic_invalid.move:14:44
│
14 │ public fun borrowA<Q,T>(_s: &A<T>, _j: T): &Q { abort 0 }
│ - ^ This index function expects type 'T'
│ - ^ This parameter has type 'T'
│ │
│ Type parameter T appears in position 2 here
15 │ #[syntax(index)]
16 │ public fun borrow_muAt<T,Q>(_s: &mut A<T>, _j: T): &mut T { abort 0 }
│ - - This mutable index function expects type 'T'
│ │
│ - -
│ │ │
│ │ Expected this parameter to have type 'Q'
│ │ It has type 'T'
│ Type parameter T appears in position 1 here
│
= Type parameters must be used the same by position, not name
Expand Down
Loading

0 comments on commit d54ac2f

Please sign in to comment.