Skip to content

Commit 782b484

Browse files
committed
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 #9041
1 parent 7142a59 commit 782b484

7 files changed

+129
-9
lines changed

clippy_utils/src/sugg.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#![deny(clippy::missing_docs_in_private_items)]
33

44
use crate::source::{snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite};
5+
use crate::ty::expr_sig;
56
use crate::{get_parent_expr_for_hir, higher};
67
use rustc_ast::util::parser::AssocOp;
78
use rustc_ast::{ast, token};
@@ -18,7 +19,6 @@ use rustc_span::source_map::{BytePos, CharPos, Pos, Span, SyntaxContext};
1819
use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
1920
use std::borrow::Cow;
2021
use std::fmt::{Display, Write as _};
21-
use std::iter;
2222
use std::ops::{Add, Neg, Not, Sub};
2323

2424
/// A helper type to build suggestion correctly handling parentheses.
@@ -861,23 +861,37 @@ impl<'tcx> DerefDelegate<'_, 'tcx> {
861861

862862
/// indicates whether the function from `parent_expr` takes its args by double reference
863863
fn func_takes_arg_by_double_ref(&self, parent_expr: &'tcx hir::Expr<'_>, cmt_hir_id: HirId) -> bool {
864-
let (call_args, inputs) = match parent_expr.kind {
864+
let ty = match parent_expr.kind {
865865
ExprKind::MethodCall(_, call_args, _) => {
866-
if let Some(method_did) = self.cx.typeck_results().type_dependent_def_id(parent_expr.hir_id) {
867-
(call_args, self.cx.tcx.fn_sig(method_did).skip_binder().inputs())
866+
if let Some(sig) = self
867+
.cx
868+
.typeck_results()
869+
.type_dependent_def_id(parent_expr.hir_id)
870+
.map(|did| self.cx.tcx.fn_sig(did).skip_binder())
871+
{
872+
call_args
873+
.iter()
874+
.position(|arg| arg.hir_id == cmt_hir_id)
875+
.map(|i| sig.inputs()[i])
868876
} else {
869877
return false;
870878
}
871879
},
872880
ExprKind::Call(func, call_args) => {
873-
let typ = self.cx.typeck_results().expr_ty(func);
874-
(call_args, typ.fn_sig(self.cx.tcx).skip_binder().inputs())
881+
if let Some(sig) = expr_sig(self.cx, func) {
882+
call_args
883+
.iter()
884+
.position(|arg| arg.hir_id == cmt_hir_id)
885+
.and_then(|i| sig.input(i))
886+
.map(ty::Binder::skip_binder)
887+
} else {
888+
return false;
889+
}
875890
},
876891
_ => return false,
877892
};
878893

879-
iter::zip(call_args, inputs)
880-
.any(|(arg, ty)| arg.hir_id == cmt_hir_id && matches!(ty.kind(), ty::Ref(_, inner, _) if inner.is_ref()))
894+
ty.map_or(false, |ty| matches!(ty.kind(), ty::Ref(_, inner, _) if inner.is_ref()))
881895
}
882896
}
883897

clippy_utils/src/ty.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,9 @@ pub fn expr_sig<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<ExprFnS
565565
}
566566

567567
fn ty_sig<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<ExprFnSig<'tcx>> {
568+
if ty.is_box() {
569+
return ty_sig(cx, ty.boxed_ty());
570+
}
568571
match *ty.kind() {
569572
ty::Closure(id, subs) => {
570573
let decl = id
@@ -573,6 +576,7 @@ fn ty_sig<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<ExprFnSig<'tcx>>
573576
Some(ExprFnSig::Closure(decl, subs.as_closure().sig()))
574577
},
575578
ty::FnDef(id, subs) => Some(ExprFnSig::Sig(cx.tcx.bound_fn_sig(id).subst(cx.tcx, subs))),
579+
ty::Opaque(id, _) => ty_sig(cx, cx.tcx.type_of(id)),
576580
ty::FnPtr(sig) => Some(ExprFnSig::Sig(sig)),
577581
ty::Dynamic(bounds, _) => {
578582
let lang_items = cx.tcx.lang_items();

tests/ui/crashes/ice-9041.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
pub struct Thing;
2+
3+
pub fn has_thing(things: &[Thing]) -> bool {
4+
let is_thing_ready = |_peer: &Thing| -> bool { todo!() };
5+
things.iter().find(|p| is_thing_ready(p)).is_some()
6+
}
7+
8+
fn main() {}

tests/ui/crashes/ice-9041.stderr

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: called `is_some()` after searching an `Iterator` with `find`
2+
--> $DIR/ice-9041.rs:5:19
3+
|
4+
LL | things.iter().find(|p| is_thing_ready(p)).is_some()
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|p| is_thing_ready(&p))`
6+
|
7+
= note: `-D clippy::search-is-some` implied by `-D warnings`
8+
9+
error: aborting due to previous error
10+

tests/ui/search_is_some_fixable_some.fixed

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,33 @@ mod issue7392 {
216216
let _ = v.iter().any(|fp| test_u32_2(*fp.field));
217217
}
218218
}
219+
220+
mod issue9120 {
221+
fn make_arg_no_deref_impl() -> impl Fn(&&u32) -> bool {
222+
move |x: &&u32| **x == 78
223+
}
224+
225+
fn make_arg_no_deref_dyn() -> Box<dyn Fn(&&u32) -> bool> {
226+
Box::new(move |x: &&u32| **x == 78)
227+
}
228+
229+
fn wrapper<T: Fn(&&u32) -> bool>(v: Vec<u32>, func: T) -> bool {
230+
#[allow(clippy::redundant_closure)]
231+
v.iter().any(|x: &u32| func(&x))
232+
}
233+
234+
fn do_tests() {
235+
let v = vec![3, 2, 1, 0];
236+
let arg_no_deref_impl = make_arg_no_deref_impl();
237+
let arg_no_deref_dyn = make_arg_no_deref_dyn();
238+
239+
#[allow(clippy::redundant_closure)]
240+
let _ = v.iter().any(|x: &u32| arg_no_deref_impl(&x));
241+
242+
#[allow(clippy::redundant_closure)]
243+
let _ = v.iter().any(|x: &u32| arg_no_deref_dyn(&x));
244+
245+
#[allow(clippy::redundant_closure)]
246+
let _ = v.iter().any(|x: &u32| (*arg_no_deref_dyn)(&x));
247+
}
248+
}

tests/ui/search_is_some_fixable_some.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,33 @@ mod issue7392 {
219219
let _ = v.iter().find(|fp| test_u32_2(*fp.field)).is_some();
220220
}
221221
}
222+
223+
mod issue9120 {
224+
fn make_arg_no_deref_impl() -> impl Fn(&&u32) -> bool {
225+
move |x: &&u32| **x == 78
226+
}
227+
228+
fn make_arg_no_deref_dyn() -> Box<dyn Fn(&&u32) -> bool> {
229+
Box::new(move |x: &&u32| **x == 78)
230+
}
231+
232+
fn wrapper<T: Fn(&&u32) -> bool>(v: Vec<u32>, func: T) -> bool {
233+
#[allow(clippy::redundant_closure)]
234+
v.iter().find(|x: &&u32| func(x)).is_some()
235+
}
236+
237+
fn do_tests() {
238+
let v = vec![3, 2, 1, 0];
239+
let arg_no_deref_impl = make_arg_no_deref_impl();
240+
let arg_no_deref_dyn = make_arg_no_deref_dyn();
241+
242+
#[allow(clippy::redundant_closure)]
243+
let _ = v.iter().find(|x: &&u32| arg_no_deref_impl(x)).is_some();
244+
245+
#[allow(clippy::redundant_closure)]
246+
let _ = v.iter().find(|x: &&u32| arg_no_deref_dyn(x)).is_some();
247+
248+
#[allow(clippy::redundant_closure)]
249+
let _ = v.iter().find(|x: &&u32| (*arg_no_deref_dyn)(x)).is_some();
250+
}
251+
}

tests/ui/search_is_some_fixable_some.stderr

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,5 +264,29 @@ error: called `is_some()` after searching an `Iterator` with `find`
264264
LL | let _ = v.iter().find(|fp| test_u32_2(*fp.field)).is_some();
265265
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|fp| test_u32_2(*fp.field))`
266266

267-
error: aborting due to 43 previous errors
267+
error: called `is_some()` after searching an `Iterator` with `find`
268+
--> $DIR/search_is_some_fixable_some.rs:234:18
269+
|
270+
LL | v.iter().find(|x: &&u32| func(x)).is_some()
271+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x: &u32| func(&x))`
272+
273+
error: called `is_some()` after searching an `Iterator` with `find`
274+
--> $DIR/search_is_some_fixable_some.rs:243:26
275+
|
276+
LL | let _ = v.iter().find(|x: &&u32| arg_no_deref_impl(x)).is_some();
277+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x: &u32| arg_no_deref_impl(&x))`
278+
279+
error: called `is_some()` after searching an `Iterator` with `find`
280+
--> $DIR/search_is_some_fixable_some.rs:246:26
281+
|
282+
LL | let _ = v.iter().find(|x: &&u32| arg_no_deref_dyn(x)).is_some();
283+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x: &u32| arg_no_deref_dyn(&x))`
284+
285+
error: called `is_some()` after searching an `Iterator` with `find`
286+
--> $DIR/search_is_some_fixable_some.rs:249:26
287+
|
288+
LL | let _ = v.iter().find(|x: &&u32| (*arg_no_deref_dyn)(x)).is_some();
289+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `any()` instead: `any(|x: &u32| (*arg_no_deref_dyn)(&x))`
290+
291+
error: aborting due to 47 previous errors
268292

0 commit comments

Comments
 (0)