Optimize and eliminate panic

Author: smoelius

clippy_lints/src/dereference.rs

@@ -17,8 +17,8 @@ use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
 use rustc_middle::ty::{
-    self, subst::Subst, EarlyBinder, FnSig, ParamTy, PredicateKind, ProjectionPredicate, TraitPredicate, Ty, TyCtxt,
-    TypeVisitable, TypeckResults,
+    self, subst::Subst, EarlyBinder, FnSig, ParamTy, PredicateKind, ProjectionPredicate, TraitPredicate, TraitRef, Ty,
+    TyCtxt, TypeVisitable, TypeckResults,
 };
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{symbol::sym, Span, Symbol};
@@ -971,18 +971,6 @@ fn needless_borrow_impl_arg_position<'tcx>(
         });
 
     let predicates = cx.tcx.param_env(callee_def_id).caller_bounds();
-    let trait_predicates_for_param_ty = predicates
-        .iter()
-        .filter_map(|predicate| {
-            if let PredicateKind::Trait(trait_predicate) = predicate.kind().skip_binder()
-                && trait_predicate.trait_ref.self_ty() == param_ty.to_ty(cx.tcx)
-            {
-                Some(trait_predicate)
-            } else {
-                None
-            }
-        })
-        .collect::<Vec<_>>();
     let projection_predicates = predicates
         .iter()
         .filter_map(|predicate| {
@@ -994,18 +982,26 @@ fn needless_borrow_impl_arg_position<'tcx>(
         })
         .collect::<Vec<_>>();
 
+    let mut trait_with_ref_mut_self_method = false;
+
     // If no traits were found, or only the `Sized` or `Any` traits were found, return.
-    if trait_predicates_for_param_ty.iter().all(|trait_predicate| {
-        let trait_def_id = trait_predicate.def_id();
-        Some(trait_def_id) == sized_trait_def_id || cx.tcx.is_diagnostic_item(sym::Any, trait_def_id)
+    if predicates.iter().all(|predicate| {
+        if let PredicateKind::Trait(TraitPredicate {
+            trait_ref: TraitRef {
+                def_id: trait_def_id, ..
+            },
+            ..
+        }) = predicate.kind().skip_binder()
+        {
+            trait_with_ref_mut_self_method |= has_ref_mut_self_method(cx, trait_def_id);
+            Some(trait_def_id) == sized_trait_def_id || cx.tcx.is_diagnostic_item(sym::Any, trait_def_id)
+        } else {
+            true
+        }
     }) {
         return Position::Other(precedence);
     }
 
-    let has_ref_mut_self_method = trait_predicates_for_param_ty
-        .iter()
-        .any(|TraitPredicate { trait_ref, .. }| has_ref_mut_self_method(cx, trait_ref.def_id));
-
     // `substs_with_referent_ty` can be constructed outside of `check_referent` because the same
     // elements are modified each time `check_referent` is called.
     let mut substs_with_referent_ty = substs_with_expr_ty.to_vec();
@@ -1018,7 +1014,7 @@ fn needless_borrow_impl_arg_position<'tcx>(
         }
 
         // https://github.com/rust-lang/rust-clippy/pull/9136#pullrequestreview-1037379321
-        if has_ref_mut_self_method && !matches!(referent_ty.kind(), ty::Ref(_, _, Mutability::Mut)) {
+        if trait_with_ref_mut_self_method && !matches!(referent_ty.kind(), ty::Ref(_, _, Mutability::Mut)) {
             return false;
         }
 
@@ -1087,29 +1083,22 @@ fn replace_types<'tcx>(
     substs: &mut [ty::GenericArg<'tcx>],
 ) -> bool {
     let mut replaced = BitSet::new_empty(substs.len());
-
     let mut deque = VecDeque::with_capacity(substs.len());
-    deque.push_back((param_ty, new_ty));
 
-    while let Some((param_ty, new_ty)) = deque.pop_front() {
-        let new_generic_arg = ty::GenericArg::from(new_ty);
-
-        // If `replaced.is_empty()`, then `param_ty` and `new_ty` are those initially passed in.
-        if !(substs[param_ty.index as usize] == new_generic_arg
-            || fn_sig
-                .inputs_and_output
-                .iter()
-                .enumerate()
-                .all(|(i, ty)| (replaced.is_empty() && i == arg_index) || !contains_ty(ty, param_ty.to_ty(cx.tcx))))
-        {
-            return false;
-        }
+    if fn_sig
+        .inputs_and_output
+        .iter()
+        .enumerate()
+        .any(|(i, ty)| i != arg_index && contains_ty(ty, param_ty.to_ty(cx.tcx)))
+    {
+        return false;
+    }
 
-        // The next line should not panic, but panicking seems better than going into an infinite loop.
-        assert!(replaced.insert(param_ty.index));
+    substs[param_ty.index as usize] = ty::GenericArg::from(new_ty);
 
-        substs[param_ty.index as usize] = new_generic_arg;
+    deque.push_back((param_ty, new_ty));
 
+    while let Some((param_ty, new_ty)) = deque.pop_front() {
         for projection_predicate in projection_predicates {
             if projection_predicate.projection_ty.self_ty() == param_ty.to_ty(cx.tcx)
                 && let ty::Term::Ty(term_ty) = projection_predicate.term
@@ -1121,6 +1110,27 @@ fn replace_types<'tcx>(
                     .mk_projection(assoc_item.def_id, cx.tcx.mk_substs_trait(new_ty, &[]));
                 let projected_ty = cx.tcx.normalize_erasing_regions(cx.param_env, projection);
 
+                let inserted = replaced.insert(term_param_ty.index);
+
+                let new_generic_arg = ty::GenericArg::from(projected_ty);
+
+                if substs[term_param_ty.index as usize] == new_generic_arg {
+                    continue;
+                }
+
+                // The next line provides some protection against infinite loops.
+                assert!(inserted);
+
+                if fn_sig
+                    .inputs_and_output
+                    .iter()
+                    .any(|ty| contains_ty(ty, term_param_ty.to_ty(cx.tcx)))
+                {
+                    return false;
+                }
+
+                substs[term_param_ty.index as usize] = new_generic_arg;
+
                 deque.push_back((*term_param_ty, projected_ty));
             }
         }

tests/ui/needless_borrow.fixed

@@ -138,7 +138,7 @@ fn main() {
     let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
     let _ = Box::new(&""); // Don't lint. Type parameter appears in return type
     ref_as_ref_path(&""); // Don't lint. Argument type is not a type parameter
-    multiple_constraints_does_not_normalize_to_same(&[[""]], &[""]); // Don't lint. Projected type appears in arguments
+    multiple_constraints_normalizes_to_different(&[[""]], &[""]); // Don't lint. Projected type appears in arguments
 }
 
 #[allow(clippy::needless_borrowed_reference)]
@@ -237,7 +237,7 @@ where
 {
 }
 
-fn multiple_constraints_does_not_normalize_to_same<T, U, V>(_: T, _: U)
+fn multiple_constraints_normalizes_to_different<T, U, V>(_: T, _: U)
 where
     T: IntoIterator<Item = U>,
     U: IntoIterator<Item = V>,

tests/ui/needless_borrow.rs

@@ -138,7 +138,7 @@ fn main() {
     let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
     let _ = Box::new(&""); // Don't lint. Type parameter appears in return type
     ref_as_ref_path(&""); // Don't lint. Argument type is not a type parameter
-    multiple_constraints_does_not_normalize_to_same(&[[""]], &[""]); // Don't lint. Projected type appears in arguments
+    multiple_constraints_normalizes_to_different(&[[""]], &[""]); // Don't lint. Projected type appears in arguments
 }
 
 #[allow(clippy::needless_borrowed_reference)]
@@ -237,7 +237,7 @@ where
 {
 }
 
-fn multiple_constraints_does_not_normalize_to_same<T, U, V>(_: T, _: U)
+fn multiple_constraints_normalizes_to_different<T, U, V>(_: T, _: U)
 where
     T: IntoIterator<Item = U>,
     U: IntoIterator<Item = V>,