Skip to content

Commit

Permalink
generalize opt_self_var to be able to catch recursion on multiple var…
Browse files Browse the repository at this point in the history
…s. check for `*error*` in type for instantiation type error message, avoid showing an error message which is actually redundant
  • Loading branch information
brmataptos committed Oct 17, 2024
1 parent 550db1e commit 0f45016
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,3 @@ error: undeclared `0x1::ordered_map::OrderedMao`
39 │ public fun borrow<K, V>(self: &OrderedMao<K, V>, key: &K): &V {
│ ^^^^^^^^^^

error: unable to infer instantiation of type `fun self.iter_borrow(Self,&*error*):&V` (consider providing type arguments or annotating the type)
┌─ tests/checking/receiver/bad_receiver.move:40:9
40 │ self.find(key).iter_borrow(self)
│ ^^^^^^^^^^^^^^
18 changes: 11 additions & 7 deletions third_party/move/move-model/src/builder/exp_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,17 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo
};
ty.visit(&mut visitor);
if incomplete {
self.error(
&loc,
&format!(
"unable to infer instantiation of type `{}` (consider providing type arguments or annotating the type)",
ty.display(&self.type_display_context())
),
);
let displayed_ty = format!("{}", ty.display(&self.type_display_context()));
// Skip displaying the error message if there is already an error in the type; we must have another message about it already.
if !displayed_ty.contains("*error*") {
self.error(
&loc,
&format!(
"unable to infer instantiation of type `{}` (consider providing type arguments or annotating the type)",
displayed_ty
),
);
}
}
ty
}
Expand Down
12 changes: 6 additions & 6 deletions third_party/move/move-model/src/builder/model_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ impl<'env> ModelBuilder<'env> {
display_type_vars: false,
used_modules: BTreeSet::new(),
use_module_qualification: false,
opt_self_var: None,
recursive_vars: None,
}
}

Expand Down Expand Up @@ -400,7 +400,7 @@ impl<'env> ModelBuilder<'env> {
&entry.name_loc,
&format!(
"parameter name `{}` indicates a receiver function but \
the type `{}` {}. Consider using a different name.",
the type `{}` {}. Consider using a different name.",
well_known::RECEIVER_PARAM_NAME,
base_type.display(&type_ctx()),
reason
Expand All @@ -414,7 +414,7 @@ impl<'env> ModelBuilder<'env> {
if !matches!(ty, Type::TypeParameter(_)) {
diag(&format!(
"must only use type parameters \
but instead uses `{}`",
but instead uses `{}`",
ty.display(&type_ctx())
))
} else if !seen.insert(ty) {
Expand All @@ -434,7 +434,7 @@ impl<'env> ModelBuilder<'env> {
if &entry.module_id != mid {
diag(
"is declared outside of this module \
and new receiver functions cannot be added",
and new receiver functions cannot be added",
)
} else {
// The instantiation must be fully generic.
Expand All @@ -457,7 +457,7 @@ impl<'env> ModelBuilder<'env> {
{
diag(
"is associated with the standard vector module \
and new receiver functions cannot be added",
and new receiver functions cannot be added",
)
} else {
// See above for structs
Expand All @@ -471,7 +471,7 @@ impl<'env> ModelBuilder<'env> {
},
_ => diag(
"is not suitable for receiver functions. \
Only structs and vectors can have receiver functions",
Only structs and vectors can have receiver functions",
),
}
}
Expand Down
22 changes: 14 additions & 8 deletions third_party/move/move-model/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3311,8 +3311,8 @@ pub struct TypeDisplayContext<'a> {
pub used_modules: BTreeSet<ModuleId>,
/// Whether to use `m::T` for representing types, for stable output in docgen
pub use_module_qualification: bool,
/// Var type that should appear as Self in display
pub opt_self_var: Option<u32>,
/// Var types that are recursive and should appear as `..` in display
pub recursive_vars: Option<BTreeSet<u32>>,
}

impl<'a> TypeDisplayContext<'a> {
Expand All @@ -3326,7 +3326,7 @@ impl<'a> TypeDisplayContext<'a> {
display_type_vars: false,
used_modules: BTreeSet::new(),
use_module_qualification: false,
opt_self_var: None,
recursive_vars: None,
}
}

Expand All @@ -3350,7 +3350,7 @@ impl<'a> TypeDisplayContext<'a> {
display_type_vars: false,
used_modules: BTreeSet::new(),
use_module_qualification: false,
opt_self_var: None,
recursive_vars: None,

Check warning on line 3353 in third_party/move/move-model/src/ty.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-model/src/ty.rs#L3353

Added line #L3353 was not covered by tests
}
}

Expand All @@ -3363,7 +3363,13 @@ impl<'a> TypeDisplayContext<'a> {

pub fn map_var_to_self(&self, idx: u32) -> Self {
Self {
opt_self_var: Some(idx),
recursive_vars: if let Some(existing_set) = &self.recursive_vars {
let mut new_set = existing_set.clone();
new_set.insert(idx);
Some(new_set)

Check warning on line 3369 in third_party/move/move-model/src/ty.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-model/src/ty.rs#L3367-L3369

Added lines #L3367 - L3369 were not covered by tests
} else {
Some(BTreeSet::from([idx]))
},
..self.clone()
}
}
Expand Down Expand Up @@ -3458,9 +3464,9 @@ impl<'a> fmt::Display for TypeDisplay<'a> {
}
},
Var(idx) => {
if let Some(idx2) = self.context.opt_self_var {
if *idx == idx2 {
return f.write_str("Self");
if let Some(recursive_vars) = &self.context.recursive_vars {
if recursive_vars.contains(idx) {
return f.write_str("..");
}

Check warning on line 3470 in third_party/move/move-model/src/ty.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-model/src/ty.rs#L3470

Added line #L3470 was not covered by tests
}
if let Some(ty) = self.context.subs_opt.and_then(|s| s.subs.get(idx)) {
Expand Down

0 comments on commit 0f45016

Please sign in to comment.