From 0f45016afc1ffad726f0d635f7868899b5f79059 Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Thu, 10 Oct 2024 13:37:28 -0700 Subject: [PATCH] generalize opt_self_var to be able to catch recursion on multiple vars. check for `*error*` in type for instantiation type error message, avoid showing an error message which is actually redundant --- .../tests/checking/receiver/bad_receiver.exp | 6 ----- .../move-model/src/builder/exp_builder.rs | 18 +++++++++------ .../move-model/src/builder/model_builder.rs | 12 +++++----- third_party/move/move-model/src/ty.rs | 22 ++++++++++++------- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/third_party/move/move-compiler-v2/tests/checking/receiver/bad_receiver.exp b/third_party/move/move-compiler-v2/tests/checking/receiver/bad_receiver.exp index 54d19855b70f6..aad7cbc7821fd 100644 --- a/third_party/move/move-compiler-v2/tests/checking/receiver/bad_receiver.exp +++ b/third_party/move/move-compiler-v2/tests/checking/receiver/bad_receiver.exp @@ -23,9 +23,3 @@ error: undeclared `0x1::ordered_map::OrderedMao` │ 39 │ public fun borrow(self: &OrderedMao, 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) - │ ^^^^^^^^^^^^^^ diff --git a/third_party/move/move-model/src/builder/exp_builder.rs b/third_party/move/move-model/src/builder/exp_builder.rs index 651680c20c8d1..5fca751d2e0a2 100644 --- a/third_party/move/move-model/src/builder/exp_builder.rs +++ b/third_party/move/move-model/src/builder/exp_builder.rs @@ -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 } diff --git a/third_party/move/move-model/src/builder/model_builder.rs b/third_party/move/move-model/src/builder/model_builder.rs index 169370d27ed5c..79586773981db 100644 --- a/third_party/move/move-model/src/builder/model_builder.rs +++ b/third_party/move/move-model/src/builder/model_builder.rs @@ -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, } } @@ -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 @@ -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) { @@ -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. @@ -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 @@ -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", ), } } diff --git a/third_party/move/move-model/src/ty.rs b/third_party/move/move-model/src/ty.rs index 5b105012444eb..e19f9172b3868 100644 --- a/third_party/move/move-model/src/ty.rs +++ b/third_party/move/move-model/src/ty.rs @@ -3311,8 +3311,8 @@ pub struct TypeDisplayContext<'a> { pub used_modules: BTreeSet, /// 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, + /// Var types that are recursive and should appear as `..` in display + pub recursive_vars: Option>, } impl<'a> TypeDisplayContext<'a> { @@ -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, } } @@ -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, } } @@ -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) + } else { + Some(BTreeSet::from([idx])) + }, ..self.clone() } } @@ -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(".."); } } if let Some(ty) = self.context.subs_opt.and_then(|s| s.subs.get(idx)) {