Skip to content

Commit

Permalink
[compiler v1] Better compiler error messages (#9222) (#10505)
Browse files Browse the repository at this point in the history
Improve the compiler error messages when type inference fails by
- removing duplicated error messages for the same uninferred type variable (see `uninferred_type_unpack_assign.exp`).
- telling why we cannot infer; e.g., which generic needs to be provided.
  • Loading branch information
fEst1ck authored and gedigi committed Nov 21, 2023
1 parent 9acb48d commit 70af8fa
Show file tree
Hide file tree
Showing 15 changed files with 133 additions and 114 deletions.
4 changes: 2 additions & 2 deletions third_party/move/move-compiler/src/typing/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ impl<'env> Context<'env> {
.expect("ICE should have failed in naming")
}

fn struct_definition(&self, m: &ModuleIdent, n: &StructName) -> &StructDefinition {
pub fn struct_definition(&self, m: &ModuleIdent, n: &StructName) -> &StructDefinition {
let minfo = self.module_info(m);
minfo
.structs
Expand Down Expand Up @@ -1574,7 +1574,7 @@ fn join_tvar(
}
}

fn forward_tvar(subst: &Subst, id: TVar) -> TVar {
pub fn forward_tvar(subst: &Subst, id: TVar) -> TVar {
let mut cur = id;
loop {
match subst.get(cur) {
Expand Down
119 changes: 96 additions & 23 deletions third_party/move/move-compiler/src/typing/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

use super::core::{self, Context};
use super::core::{self, forward_tvar, Context, Subst};
use crate::{
diag,
expansion::ast::Value_,
naming::ast::{BuiltinTypeName_, FunctionSignature, Type, TypeName_, Type_},
parser::ast::Ability_,
expansion::ast::{ModuleIdent, Value_},
naming::ast::{BuiltinTypeName_, FunctionSignature, TVar, Type, TypeName_, Type_},
parser::ast::{Ability_, FunctionName},
typing::ast as T,
};
use move_core_types::u256::U256;
Expand Down Expand Up @@ -57,43 +57,100 @@ fn types(context: &mut Context, ss: &mut Vec<Type>) {
}
}

pub fn type_(context: &mut Context, ty: &mut Type) {
// type_ for expanding the `i`-th type param of struct `ty_name`
fn type_struct_ty_param(context: &mut Context, ty: &mut Type, i: usize, ty_name: &TypeName_) {
if let TypeName_::ModuleType(mod_id, struct_name) = ty_name {
let param_name = &context
.struct_definition(mod_id, struct_name)
.type_parameters[i]
.param
.user_specified_name
.value;
let msg = format!("Cannot infer the type parameter `{param_name}` for generic struct `{ty_name}`. Try providing a type parameter.");
type_with_context_msg(context, ty, &msg);
} else {
type_(context, ty)
}
}

fn type_fun_ty_param(
context: &mut Context,
ty: &mut Type,
i: usize,
m: &ModuleIdent,
n: &FunctionName,
) {
let param_name = context.function_info(m, n).signature.type_parameters[i].user_specified_name;
type_with_context_msg(context, ty, &format!("Cannot infer the type parameter `{param_name}` for generic function `{m}::{n}`. Try providing a type parameter."));
}

// Unfold the type vars. If the type cannot be inferred, return the last tvar.
fn unfold_type_or_last_var(subst: &Subst, sp!(loc, t_): Type) -> Result<Type, Spanned<TVar>> {
match t_ {
Type_::Var(i) => {
let last_tvar = forward_tvar(subst, i);
match subst.get(last_tvar) {
Some(sp!(_, Type_::Var(_))) => panic!("ICE forward_tvar returned a type variable"),
Some(inner) => Ok(inner.clone()),
None => Err(sp(loc, last_tvar)),
}
},
_ => Ok(sp(loc, t_)),
}
}

// Try to expand the type of ty, warning with msg_uninferred if type cannot be inferred.
fn type_with_context_msg(context: &mut Context, ty: &mut Type, msg_uninferred: &str) {
use Type_::*;
match &mut ty.value {
Anything | UnresolvedError | Param(_) | Unit => (),
Ref(_, b) => type_(context, b),
Var(tvar) => {
let ty_tvar = sp(ty.loc, Var(*tvar));
let replacement = core::unfold_type(&context.subst, ty_tvar);
let replacement = unfold_type_or_last_var(&context.subst, ty_tvar);
let replacement = match replacement {
sp!(_, Var(_)) => panic!("ICE unfold_type_base failed to expand"),
sp!(loc, Anything) => {
let msg = "Could not infer this type. Try adding an annotation";
Err(sp!(loc, last_tvar)) => {
// this is to avoid duplicate error messages for uninferred type variables
// in the first time they are resolved to Err(_), and to Anything for all following queries
context.subst.insert(last_tvar, sp(ty.loc, Type_::Anything));
context
.env
.add_diag(diag!(TypeSafety::UninferredType, (ty.loc, msg)));
.add_diag(diag!(TypeSafety::UninferredType, (loc, msg_uninferred)));
sp(loc, UnresolvedError)
},
t => t,
Ok(sp!(_, Var(_))) => panic!("ICE unfold_type_base failed to expand"),
Ok(t) => t,
};
*ty = replacement;
type_(context, ty);
},
Apply(Some(_), sp!(_, TypeName_::Builtin(_)), tys) => types(context, tys),
Apply(Some(_), _, _) => panic!("ICE expanding pre expanded type"),
Apply(None, _, _) => {
Apply(None, ty_name, _) => {
let ty_name = ty_name.clone();
let abilities = core::infer_abilities(context, &context.subst, ty.clone());
match &mut ty.value {
Apply(abilities_opt, _, tys) => {
*abilities_opt = Some(abilities);
types(context, tys);
for (i, ty) in tys.iter_mut().enumerate() {
type_struct_ty_param(context, ty, i, &ty_name.value)
}
},
_ => panic!("ICE impossible. tapply switched to nontapply"),
}
},
}
}

// Try to expand the type of ty, warning with msg_uninferred if type cannot be inferred.
pub fn type_(context: &mut Context, ty: &mut Type) {
type_with_context_msg(
context,
ty,
"Could not infer this type. Try adding an annotation",
)
}

//**************************************************************************************************
// Expressions
//**************************************************************************************************
Expand Down Expand Up @@ -184,7 +241,7 @@ pub fn exp(context: &mut Context, e: &mut T::Exp) {
};
let new_exp = if v > max {
let msg = format!(
"Expected a literal of type '{}', but the value is too large.",
"Expected a literal of type `{}`, but the value is too large.",
bt
);
let fix_bt = if v > u128_max {
Expand All @@ -201,7 +258,7 @@ pub fn exp(context: &mut Context, e: &mut T::Exp) {
};

let fix = format!(
"Annotating the literal might help inference: '{value}{type}'",
"Annotating the literal might help inference: `{value}{type}`",
value=v,
type=fix_bt,
);
Expand Down Expand Up @@ -295,10 +352,16 @@ pub fn exp(context: &mut Context, e: &mut T::Exp) {
type_(context, operand_ty);
},

E::Pack(_, _, bs, fields) => {
types(context, bs);
for (_, _, (_, (bt, fe))) in fields.iter_mut() {
type_(context, bt);
E::Pack(mod_id, struct_name, bs, fields) => {
for (i, b) in bs.iter_mut().enumerate() {
type_struct_ty_param(context, b, i, &TypeName_::ModuleType(*mod_id, *struct_name));
}
for (_, s, (_, (bt, fe))) in fields.iter_mut() {
type_with_context_msg(
context,
bt,
&format!("Could not infer the type of field {s}"),
);
exp(context, fe)
}
},
Expand All @@ -324,8 +387,11 @@ fn lvalue(context: &mut Context, b: &mut T::LValue) {
type_(context, ty);
core::check_non_fun(context, ty.as_ref())
},
L::BorrowUnpack(_, _, _, bts, fields) | L::Unpack(_, _, bts, fields) => {
types(context, bts);
L::BorrowUnpack(_, mod_id, struct_name, bts, fields)
| L::Unpack(mod_id, struct_name, bts, fields) => {
for (i, b) in bts.iter_mut().enumerate() {
type_struct_ty_param(context, b, i, &TypeName_::ModuleType(*mod_id, *struct_name))
}
for (_, _, (_, (bt, innerb))) in fields.iter_mut() {
type_(context, bt);
lvalue(context, innerb)
Expand All @@ -335,20 +401,27 @@ fn lvalue(context: &mut Context, b: &mut T::LValue) {
}

fn module_call(context: &mut Context, call: &mut T::ModuleCall) {
types(context, &mut call.type_arguments);
for (i, t) in call.type_arguments.iter_mut().enumerate() {
type_fun_ty_param(context, t, i, &call.module, &call.name);
}
exp(context, &mut call.arguments);
types(context, &mut call.parameter_types)
}

fn builtin_function(context: &mut Context, b: &mut T::BuiltinFunction) {
use T::BuiltinFunction_ as B;
let f_name = b.value.display_name();
match &mut b.value {
B::MoveTo(bt)
| B::MoveFrom(bt)
| B::BorrowGlobal(_, bt)
| B::Exists(bt)
| B::Freeze(bt) => {
type_(context, bt);
type_with_context_msg(
context,
bt,
&format!("Cannot infer a type parameter for built-in function `{f_name}`"),
);
},
B::Assert(_) => (),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,7 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/eq_invalid.move:28:9
28 │ G2{} == G2{};
│ ^^^^ Could not infer this type. Try adding an annotation

error[E04010]: cannot infer type
┌─ tests/move_check/typing/eq_invalid.move:28:17
28 │ G2{} == G2{};
│ ^^^^ Could not infer this type. Try adding an annotation
│ ^^^^ Cannot infer the type parameter `T` for generic struct `0x8675309::M::G2`. Try providing a type parameter.

error[E05001]: ability constraint not satisfied
┌─ tests/move_check/typing/eq_invalid.move:29:9
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/exp_list_nested.move:6:19
6 │ (0, (S{}, R{}))
│ ^^^ Could not infer this type. Try adding an annotation
│ ^^^ Cannot infer the type parameter `T` for generic struct `0x8675309::M::R`. Try providing a type parameter.

Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/exp_list_resource_drop.move:9:19
9 │ (0, S{ }, Box { f: abort 0 });
│ ^^^^^^^^^^^^^^^^^^ Could not infer this type. Try adding an annotation
│ ^^^^^^^^^^^^^^^^^^ Cannot infer the type parameter `T` for generic struct `0x8675309::M::Box`. Try providing a type parameter.

Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/ignore_inferred_resource.move:4:9
4 │ S{};
│ ^^^ Could not infer this type. Try adding an annotation
│ ^^^ Cannot infer the type parameter `T` for generic struct `0x8675309::M::S`. Try providing a type parameter.

Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,7 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/invalid_type_acquire.move:30:9
30 │ destroy(account, move_from(a));
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Could not infer this type. Try adding an annotation

error[E04010]: cannot infer type
┌─ tests/move_check/typing/invalid_type_acquire.move:30:26
30 │ destroy(account, move_from(a));
│ ^^^^^^^^^^^^ Could not infer this type. Try adding an annotation
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Cannot infer the type parameter `T` for generic function `0x2::M::destroy`. Try providing a type parameter.

error[E04009]: expected specific type
┌─ tests/move_check/typing/invalid_type_acquire.move:31:26
Expand Down Expand Up @@ -173,7 +167,7 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/invalid_type_acquire.move:48:9
48 │ exists(a);
│ ^^^^^^^^^ Could not infer this type. Try adding an annotation
│ ^^^^^^^^^ Cannot infer a type parameter for built-in function `exists`

error[E04009]: expected specific type
┌─ tests/move_check/typing/invalid_type_acquire.move:49:9
Expand Down Expand Up @@ -218,13 +212,7 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/invalid_type_acquire.move:54:9
54 │ move_to(account, any());
│ ^^^^^^^^^^^^^^^^^^^^^^^ Could not infer this type. Try adding an annotation

error[E04010]: cannot infer type
┌─ tests/move_check/typing/invalid_type_acquire.move:54:26
54 │ move_to(account, any());
│ ^^^^^ Could not infer this type. Try adding an annotation
│ ^^^^^^^^^^^^^^^^^^^^^^^ Cannot infer a type parameter for built-in function `move_to`

error[E04009]: expected specific type
┌─ tests/move_check/typing/invalid_type_acquire.move:55:9
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,7 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/neq_invalid.move:26:9
26 │ G0{} != G0{};
│ ^^^^ Could not infer this type. Try adding an annotation

error[E04010]: cannot infer type
┌─ tests/move_check/typing/neq_invalid.move:26:17
26 │ G0{} != G0{};
│ ^^^^ Could not infer this type. Try adding an annotation
│ ^^^^ Cannot infer the type parameter `T` for generic struct `0x8675309::M::G0`. Try providing a type parameter.

error[E05001]: ability constraint not satisfied
┌─ tests/move_check/typing/neq_invalid.move:27:9
Expand All @@ -130,7 +124,7 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/neq_invalid.move:27:9
27 │ G1{} != G1{};
│ ^^^^ Could not infer this type. Try adding an annotation
│ ^^^^ Cannot infer the type parameter `T` for generic struct `0x8675309::M::G1`. Try providing a type parameter.

error[E05001]: ability constraint not satisfied
┌─ tests/move_check/typing/neq_invalid.move:27:17
Expand All @@ -144,12 +138,6 @@ error[E05001]: ability constraint not satisfied
│ '!=' requires the 'drop' ability as the value is consumed. Try borrowing the values with '&' first.'
│ The type '0x8675309::M::G1<_>' does not have the ability 'drop'

error[E04010]: cannot infer type
┌─ tests/move_check/typing/neq_invalid.move:27:17
27 │ G1{} != G1{};
│ ^^^^ Could not infer this type. Try adding an annotation

error[E05001]: ability constraint not satisfied
┌─ tests/move_check/typing/neq_invalid.move:28:9
Expand All @@ -166,7 +154,7 @@ error[E04010]: cannot infer type
┌─ tests/move_check/typing/neq_invalid.move:28:9
28 │ G2{} != G2{};
│ ^^^^ Could not infer this type. Try adding an annotation
│ ^^^^ Cannot infer the type parameter `T` for generic struct `0x8675309::M::G2`. Try providing a type parameter.

error[E05001]: ability constraint not satisfied
┌─ tests/move_check/typing/neq_invalid.move:28:17
Expand All @@ -180,12 +168,6 @@ error[E05001]: ability constraint not satisfied
│ '!=' requires the 'drop' ability as the value is consumed. Try borrowing the values with '&' first.'
│ The type '0x8675309::M::G2<_>' does not have the ability 'drop'

error[E04010]: cannot infer type
┌─ tests/move_check/typing/neq_invalid.move:28:17
28 │ G2{} != G2{};
│ ^^^^ Could not infer this type. Try adding an annotation

error[E04005]: expected a single type
┌─ tests/move_check/typing/neq_invalid.move:32:9
Expand Down
Loading

0 comments on commit 70af8fa

Please sign in to comment.