Auto merge of rust-lang#9120 - anall:bugfix/ice9041, r=Jarcho

Fix ICE in sugg::DerefDelegate with (named) closures

rustc comiler internals helpfully tell us how to fix the issue:
```
  to get the signature of a closure, use `substs.as_closure().sig()` not `fn_sig()`
```
Fixes ICE in rust-lang#9041

This also makes this code in `sugg::DerefDelegate` match a different use `typ.fn_sig(…)` I found: in `mixed_read_write_in_expression` -- being strict on the value of `typ.kind()` will hopefully reduce any future possibility of ICE crashes in this area.

---

changelog: none

Author: bors

clippy_utils/src/sugg.rs

@@ -2,6 +2,7 @@
 #![deny(clippy::missing_docs_in_private_items)]
 
 use crate::source::{snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite};
+use crate::ty::expr_sig;
 use crate::{get_parent_expr_for_hir, higher};
 use rustc_ast::util::parser::AssocOp;
 use rustc_ast::{ast, token};
@@ -18,7 +19,6 @@ use rustc_span::source_map::{BytePos, CharPos, Pos, Span, SyntaxContext};
 use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
 use std::borrow::Cow;
 use std::fmt::{Display, Write as _};
-use std::iter;
 use std::ops::{Add, Neg, Not, Sub};
 
 /// A helper type to build suggestion correctly handling parentheses.
@@ -861,23 +861,37 @@ impl<'tcx> DerefDelegate<'_, 'tcx> {
 
     /// indicates whether the function from `parent_expr` takes its args by double reference
     fn func_takes_arg_by_double_ref(&self, parent_expr: &'tcx hir::Expr<'_>, cmt_hir_id: HirId) -> bool {
-        let (call_args, inputs) = match parent_expr.kind {
+        let ty = match parent_expr.kind {
             ExprKind::MethodCall(_, call_args, _) => {
-                if let Some(method_did) = self.cx.typeck_results().type_dependent_def_id(parent_expr.hir_id) {
-                    (call_args, self.cx.tcx.fn_sig(method_did).skip_binder().inputs())
+                if let Some(sig) = self
+                    .cx
+                    .typeck_results()
+                    .type_dependent_def_id(parent_expr.hir_id)
+                    .map(|did| self.cx.tcx.fn_sig(did).skip_binder())
+                {
+                    call_args
+                        .iter()
+                        .position(|arg| arg.hir_id == cmt_hir_id)
+                        .map(|i| sig.inputs()[i])
                 } else {
                     return false;
                 }
             },
             ExprKind::Call(func, call_args) => {
-                let typ = self.cx.typeck_results().expr_ty(func);
-                (call_args, typ.fn_sig(self.cx.tcx).skip_binder().inputs())
+                if let Some(sig) = expr_sig(self.cx, func) {
+                    call_args
+                        .iter()
+                        .position(|arg| arg.hir_id == cmt_hir_id)
+                        .and_then(|i| sig.input(i))
+                        .map(ty::Binder::skip_binder)
+                } else {
+                    return false;
+                }
             },
             _ => return false,
         };
 
-        iter::zip(call_args, inputs)
-            .any(|(arg, ty)| arg.hir_id == cmt_hir_id && matches!(ty.kind(), ty::Ref(_, inner, _) if inner.is_ref()))
+        ty.map_or(false, |ty| matches!(ty.kind(), ty::Ref(_, inner, _) if inner.is_ref()))
     }
 }
 

clippy_utils/src/ty.rs

@@ -565,6 +565,9 @@ pub fn expr_sig<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<ExprFnS
 }
 
 fn ty_sig<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<ExprFnSig<'tcx>> {
+    if ty.is_box() {
+        return ty_sig(cx, ty.boxed_ty());
+    }
     match *ty.kind() {
         ty::Closure(id, subs) => {
             let decl = id
@@ -573,6 +576,7 @@ fn ty_sig<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<ExprFnSig<'tcx>>
             Some(ExprFnSig::Closure(decl, subs.as_closure().sig()))
         },
         ty::FnDef(id, subs) => Some(ExprFnSig::Sig(cx.tcx.bound_fn_sig(id).subst(cx.tcx, subs))),
+        ty::Opaque(id, _) => ty_sig(cx, cx.tcx.type_of(id)),
         ty::FnPtr(sig) => Some(ExprFnSig::Sig(sig)),
         ty::Dynamic(bounds, _) => {
             let lang_items = cx.tcx.lang_items();

tests/ui/crashes/ice-9041.rs

@@ -0,0 +1,8 @@
+pub struct Thing;
+
+pub fn has_thing(things: &[Thing]) -> bool {
+    let is_thing_ready = |_peer: &Thing| -> bool { todo!() };
+    things.iter().find(|p| is_thing_ready(p)).is_some()
+}
+
+fn main() {}

tests/ui/crashes/ice-9041.stderr

@@ -0,0 +1,10 @@
+error: called `is_some()` after searching an `Iterator` with `find`
+  --> $DIR/ice-9041.rs:5:19
+   |
+LL |     things.iter().find(|p| is_thing_ready(p)).is_some()
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|p| is_thing_ready(&p))`
+   |
+   = note: `-D clippy::search-is-some` implied by `-D warnings`
+
+error: aborting due to previous error
+

tests/ui/search_is_some_fixable_some.fixed

@@ -216,3 +216,33 @@ mod issue7392 {
         let _ = v.iter().any(|fp| test_u32_2(*fp.field));
     }
 }
+
+mod issue9120 {
+    fn make_arg_no_deref_impl() -> impl Fn(&&u32) -> bool {
+        move |x: &&u32| **x == 78
+    }
+
+    fn make_arg_no_deref_dyn() -> Box<dyn Fn(&&u32) -> bool> {
+        Box::new(move |x: &&u32| **x == 78)
+    }
+
+    fn wrapper<T: Fn(&&u32) -> bool>(v: Vec<u32>, func: T) -> bool {
+        #[allow(clippy::redundant_closure)]
+        v.iter().any(|x: &u32| func(&x))
+    }
+
+    fn do_tests() {
+        let v = vec![3, 2, 1, 0];
+        let arg_no_deref_impl = make_arg_no_deref_impl();
+        let arg_no_deref_dyn = make_arg_no_deref_dyn();
+
+        #[allow(clippy::redundant_closure)]
+        let _ = v.iter().any(|x: &u32| arg_no_deref_impl(&x));
+
+        #[allow(clippy::redundant_closure)]
+        let _ = v.iter().any(|x: &u32| arg_no_deref_dyn(&x));
+
+        #[allow(clippy::redundant_closure)]
+        let _ = v.iter().any(|x: &u32| (*arg_no_deref_dyn)(&x));
+    }
+}

tests/ui/search_is_some_fixable_some.rs

@@ -219,3 +219,33 @@ mod issue7392 {
         let _ = v.iter().find(|fp| test_u32_2(*fp.field)).is_some();
     }
 }
+
+mod issue9120 {
+    fn make_arg_no_deref_impl() -> impl Fn(&&u32) -> bool {
+        move |x: &&u32| **x == 78
+    }
+
+    fn make_arg_no_deref_dyn() -> Box<dyn Fn(&&u32) -> bool> {
+        Box::new(move |x: &&u32| **x == 78)
+    }
+
+    fn wrapper<T: Fn(&&u32) -> bool>(v: Vec<u32>, func: T) -> bool {
+        #[allow(clippy::redundant_closure)]
+        v.iter().find(|x: &&u32| func(x)).is_some()
+    }
+
+    fn do_tests() {
+        let v = vec![3, 2, 1, 0];
+        let arg_no_deref_impl = make_arg_no_deref_impl();
+        let arg_no_deref_dyn = make_arg_no_deref_dyn();
+
+        #[allow(clippy::redundant_closure)]
+        let _ = v.iter().find(|x: &&u32| arg_no_deref_impl(x)).is_some();
+
+        #[allow(clippy::redundant_closure)]
+        let _ = v.iter().find(|x: &&u32| arg_no_deref_dyn(x)).is_some();
+
+        #[allow(clippy::redundant_closure)]
+        let _ = v.iter().find(|x: &&u32| (*arg_no_deref_dyn)(x)).is_some();
+    }
+}

tests/ui/search_is_some_fixable_some.stderr

@@ -264,5 +264,29 @@ error: called `is_some()` after searching an `Iterator` with `find`
 LL |         let _ = v.iter().find(|fp| test_u32_2(*fp.field)).is_some();
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|fp| test_u32_2(*fp.field))`
 
-error: aborting due to 43 previous errors
+error: called `is_some()` after searching an `Iterator` with `find`
+  --> $DIR/search_is_some_fixable_some.rs:234:18
+   |
+LL |         v.iter().find(|x: &&u32| func(x)).is_some()
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x: &u32| func(&x))`
+
+error: called `is_some()` after searching an `Iterator` with `find`
+  --> $DIR/search_is_some_fixable_some.rs:243:26
+   |
+LL |         let _ = v.iter().find(|x: &&u32| arg_no_deref_impl(x)).is_some();
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x: &u32| arg_no_deref_impl(&x))`
+
+error: called `is_some()` after searching an `Iterator` with `find`
+  --> $DIR/search_is_some_fixable_some.rs:246:26
+   |
+LL |         let _ = v.iter().find(|x: &&u32| arg_no_deref_dyn(x)).is_some();
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x: &u32| arg_no_deref_dyn(&x))`
+
+error: called `is_some()` after searching an `Iterator` with `find`
+  --> $DIR/search_is_some_fixable_some.rs:249:26
+   |
+LL |         let _ = v.iter().find(|x: &&u32| (*arg_no_deref_dyn)(x)).is_some();
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x: &u32| (*arg_no_deref_dyn)(&x))`
+
+error: aborting due to 47 previous errors