[compiler-v2] Avoid infinite recursion showing types with receiver fu…

…nctions in presence of errors (#14922)

- add test showing Issue #14913 and a simple fix, tracking currently printed vars to prevent recursions, replacing them by .. if they appear recursively (e.g., in constraints)
- check for `*error*` in type for instantiation type error message, avoid showing an error message which will be redundant

third_party/move/move-compiler-v2/tests/checking/receiver/bad_receiver.exp

@@ -0,0 +1,25 @@
+
+Diagnostics:
+error: undeclared receiver function `borrow` for type `vector<Entry<K, V>>`
+   ┌─ tests/checking/receiver/bad_receiver.move:25:10
+   │
+25 │         &map.entries.borrow(self.index).key
+   │          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: undeclared receiver function `lower_bound` for type `OrderedMap<K, V>`
+   ┌─ tests/checking/receiver/bad_receiver.move:29:27
+   │
+29 │         let lower_bound = self.lower_bound(key);
+   │                           ^^^^^^^^^^^^^^^^^^^^^
+
+error: type cannot have type arguments
+   ┌─ tests/checking/receiver/bad_receiver.move:39:36
+   │
+39 │     public fun borrow<K, V>(self: &OrderedMao<K, V>, key: &K): &V {
+   │                                    ^^^^^^^^^^
+
+error: undeclared `0x1::ordered_map::OrderedMao`
+   ┌─ tests/checking/receiver/bad_receiver.move:39:36
+   │
+39 │     public fun borrow<K, V>(self: &OrderedMao<K, V>, key: &K): &V {
+   │                                    ^^^^^^^^^^

third_party/move/move-compiler-v2/tests/checking/receiver/bad_receiver.move

@@ -0,0 +1,50 @@
+module aptos_std::ordered_map {
+    const EITER_OUT_OF_BOUNDS: u64 = 3;
+
+    struct Entry<K, V> has drop, copy, store {
+        key: K,
+        value: V,
+    }
+
+    enum OrderedMap<K, V> has drop, copy, store {
+        SortedVectorMap {
+            entries: vector<Entry<K, V>>,
+        }
+    }
+
+    enum Iterator has copy, drop {
+        End,
+        Position {
+            index: u64,
+        },
+    }
+
+    public fun iter_borrow_key<K, V>(self: &Iterator, map: &OrderedMap<K, V>): &K {
+        assert!(!(self is Iterator::End), EITER_OUT_OF_BOUNDS);
+
+        &map.entries.borrow(self.index).key
+    }
+
+    public fun find<K, V>(self: &OrderedMap<K, V>, key: &K): Iterator {
+        let lower_bound = self.lower_bound(key);
+        if (lower_bound.iter_is_end(self)) {
+            lower_bound
+        } else if (lower_bound.iter_borrow_key(self) == key) {
+            lower_bound
+        } else {
+            self.new_end_iter()
+        }
+    }
+
+    public fun borrow<K, V>(self: &OrderedMao<K, V>, key: &K): &V {
+        self.find(key).iter_borrow(self)
+    }
+
+    public fun new_end_iter<K, V>(self: &OrderedMap<K, V>): Iterator {
+        Iterator::End
+    }
+
+    public fun iter_is_end<K, V>(self: &Iterator, _map: &OrderedMap<K, V>): bool {
+        self is Iterator::End
+    }
+}

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
     }

third_party/move/move-model/src/builder/model_builder.rs

@@ -271,6 +271,7 @@ impl<'env> ModelBuilder<'env> {
             display_type_vars: false,
             used_modules: BTreeSet::new(),
             use_module_qualification: false,
+            recursive_vars: None,
         }
     }
 
@@ -399,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
@@ -413,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) {
@@ -433,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.
@@ -456,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
@@ -465,9 +466,12 @@ impl<'env> ModelBuilder<'env> {
                                 .insert(name.symbol, name.clone());
                         }
                     },
+                    Type::Error => {
+                        // Ignore this, there will be a message where the error type is generated.
+                    },
                     _ => diag(
                         "is not suitable for receiver functions. \
-                    Only structs and vectors can have receiver functions",
+                         Only structs and vectors can have receiver functions",
                     ),
                 }
             }

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

@@ -3311,6 +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 types that are recursive and should appear as `..` in display
+    pub recursive_vars: Option<BTreeSet<u32>>,
 }
 
 impl<'a> TypeDisplayContext<'a> {
@@ -3324,6 +3326,7 @@ impl<'a> TypeDisplayContext<'a> {
             display_type_vars: false,
             used_modules: BTreeSet::new(),
             use_module_qualification: false,
+            recursive_vars: None,
         }
     }
 
@@ -3347,6 +3350,7 @@ impl<'a> TypeDisplayContext<'a> {
             display_type_vars: false,
             used_modules: BTreeSet::new(),
             use_module_qualification: false,
+            recursive_vars: None,
         }
     }
 
@@ -3356,6 +3360,19 @@ impl<'a> TypeDisplayContext<'a> {
             ..self
         }
     }
+
+    pub fn map_var_to_self(&self, idx: u32) -> Self {
+        Self {
+            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()
+        }
+    }
 }
 
 /// Helper for type displays.
@@ -3447,6 +3464,11 @@ impl<'a> fmt::Display for TypeDisplay<'a> {
                 }
             },
             Var(idx) => {
+                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)) {
                     write!(f, "{}", ty.display(self.context))
                 } else if let Some(ctrs) =
@@ -3456,9 +3478,10 @@ impl<'a> fmt::Display for TypeDisplay<'a> {
                     if ctrs.is_empty() {
                         f.write_str(&self.type_var_str(*idx))
                     } else {
+                        let recursive_context = self.context.map_var_to_self(*idx);
                         let out = ctrs
                             .iter()
-                            .map(|(_, _, c)| c.display(self.context).to_string())
+                            .map(|(_, _, c)| c.display(&recursive_context).to_string())
                             .join(" + ");
                         f.write_str(&out)
                     }