addressed nits

Author: basil-cow

chalk-ir/src/lib.rs

@@ -1158,13 +1158,10 @@ impl<T> Binders<T> {
 
     /// Skips the binder and returns the "bound" value. This is a
     /// risky thing to do because it's easy to get confused about
-    /// De Bruijn indices and the like. It is usually better to
-    /// discharge the binder using `no_bound_vars` or something
-    /// like that. `skip_binder` is only valid when you are either
-    /// extracting data that has nothing to do with bound vars, you
-    /// are doing some sort of test that does not involve bound
-    /// regions, or you are being very careful about your depth
-    /// accounting.
+    /// De Bruijn indices and the like. `skip_binder` is only valid
+    /// when you are either extracting data that has nothing to
+    /// do with bound vars, or you are being very careful about
+    /// your depth accounting.
     ///
     /// Some examples where `skip_binder` is reasonable:
     ///
@@ -1174,6 +1171,9 @@ impl<T> Binders<T> {
         &self.value
     }
 
+    /// Converts `&Binders<T>` to `Binders<&T>`. Produces new `Binders`
+    /// with cloned quantifiers containing a reference to the original
+    /// value, leaving the original in place.
     pub fn as_ref(&self) -> Binders<&T> {
         Binders {
             binders: self.binders.clone(),
@@ -1219,27 +1219,6 @@ impl<T> Binders<T> {
         }
     }
 
-    /// Unwraps and returns the value within, but only if it contains
-    /// no vars bound by this binder. (In other words, if this binder --
-    /// and indeed any enclosing binder -- doesn't bind anything at
-    /// all.) Otherwise, returns `None`.
-    ///
-    /// (One could imagine having a method that just unwraps a single
-    /// binder, but permits late-bound vars bound by enclosing
-    /// binders, but that would require adjusting the debruijn
-    /// indices, and given the shallow binding structure we often use,
-    /// would not be that useful.)
-    pub fn no_bound_vars<I: Interner>(&self, interner: &I) -> Option<&T>
-    where
-        T: Visit<I>,
-    {
-        if self.skip_binders().has_free_vars(interner) {
-            None
-        } else {
-            Some(self.skip_binders())
-        }
-    }
-
     pub fn len(&self) -> usize {
         self.binders.len()
     }

chalk-solve/src/coherence/solve.rs

@@ -89,8 +89,16 @@ impl<I: Interner> CoherenceSolver<'_, I> {
         let (rhs_binders, rhs_bound) = rhs.binders.as_ref().into();
 
         // Upshift the rhs variables in params to account for the joined binders
-        let lhs_params = params(interner, lhs_bound).iter().cloned();
-        let rhs_params = params(interner, rhs_bound)
+        let lhs_params = lhs_bound
+            .trait_ref
+            .substitution
+            .parameters(interner)
+            .iter()
+            .cloned();
+        let rhs_params = rhs_bound
+            .trait_ref
+            .substitution
+            .parameters(interner)
             .iter()
             .map(|param| param.shifted_in(interner));
 
@@ -260,11 +268,3 @@ impl<I: Interner> CoherenceSolver<'_, I> {
         result
     }
 }
-
-fn params<'a, I: Interner>(
-    interner: &I,
-    impl_datum_bound: &'a ImplDatumBound<I>,
-) -> &'a [Parameter<I>] {
-    // We can skip binders here because the caller is handling correct binders handling
-    impl_datum_bound.trait_ref.substitution.parameters(interner)
-}

chalk-solve/src/solve/slg/resolvent.rs

@@ -485,7 +485,7 @@ impl<'i, I: Interner> Zipper<'i, I> for AnswerSubstitutor<'i, I> {
         T: Zip<I> + Fold<I, Result = T>,
     {
         self.outer_binder.shift_in();
-        Zip::zip_with(self, &answer.skip_binders(), &pending.skip_binders())?;
+        Zip::zip_with(self, answer.skip_binders(), pending.skip_binders())?;
         self.outer_binder.shift_out();
         Ok(())
     }