From 5285928bc0190efb54495d8126b68778873884b9 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 8 Aug 2022 10:09:05 -0400 Subject: [PATCH] Fix ICE when checking the HIR ty of closure args. --- clippy_lints/src/dereference.rs | 67 +++++++++++++--------- tests/ui/explicit_auto_deref.fixed | 11 ++++ tests/ui/explicit_auto_deref.rs | 11 ++++ tests/ui/explicit_auto_deref.stderr | 86 ++++++++++++++++------------- 4 files changed, 112 insertions(+), 63 deletions(-) diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 05dbc7434855..821528d7ab94 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -15,7 +15,7 @@ use rustc_hir::{ use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; -use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitable, TypeckResults}; +use rustc_middle::ty::{self, Binder, BoundVariableKind, List, Ty, TyCtxt, TypeVisitable, TypeckResults}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP}; use rustc_trait_selection::infer::InferCtxtExt; @@ -651,7 +651,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, & } match parent { Node::Local(Local { ty: Some(ty), span, .. }) if span.ctxt() == ctxt => { - Some(binding_ty_auto_deref_stability(cx, ty, precedence)) + Some(binding_ty_auto_deref_stability(cx, ty, precedence, List::empty())) }, Node::Item(&Item { kind: ItemKind::Static(..) | ItemKind::Const(..), @@ -703,13 +703,23 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, & ExprKind::Ret(_) => { let owner_id = cx.tcx.hir().body_owner(cx.enclosing_body.unwrap()); Some( - if let Node::Expr(Expr { - kind: ExprKind::Closure(&Closure { fn_decl, .. }), - .. - }) = cx.tcx.hir().get(owner_id) + if let Node::Expr( + closure @ Expr { + kind: ExprKind::Closure(&Closure { fn_decl, .. }), + .. + }, + ) = cx.tcx.hir().get(owner_id) { match fn_decl.output { - FnRetTy::Return(ty) => binding_ty_auto_deref_stability(cx, ty, precedence), + FnRetTy::Return(ty) => { + if let Some(sig) = expr_sig(cx, closure) + && let Some(output) = sig.output() + { + binding_ty_auto_deref_stability(cx, ty, precedence, output.bound_vars()) + } else { + Position::Other(precedence) + } + }, FnRetTy::DefaultReturn(_) => Position::Other(precedence), } } else { @@ -731,7 +741,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, & .map(|(hir_ty, ty)| match hir_ty { // Type inference for closures can depend on how they're called. Only go by the explicit // types here. - Some(ty) => binding_ty_auto_deref_stability(cx, ty, precedence), + Some(hir_ty) => binding_ty_auto_deref_stability(cx, hir_ty, precedence, ty.bound_vars()), None => ty_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence) .position_for_arg(), }), @@ -824,7 +834,12 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, & // // Here `y1` and `y2` would resolve to different types, so the type `&Box<_>` is not stable when // switching to auto-dereferencing. -fn binding_ty_auto_deref_stability(cx: &LateContext<'_>, ty: &hir::Ty<'_>, precedence: i8) -> Position { +fn binding_ty_auto_deref_stability<'tcx>( + cx: &LateContext<'tcx>, + ty: &'tcx hir::Ty<'_>, + precedence: i8, + binder_args: &'tcx List, +) -> Position { let TyKind::Rptr(_, ty) = &ty.kind else { return Position::Other(precedence); }; @@ -856,31 +871,31 @@ fn binding_ty_auto_deref_stability(cx: &LateContext<'_>, ty: &hir::Ty<'_>, prece } else { Position::DerefStable( precedence, - cx - .typeck_results() - .node_type(ty.ty.hir_id) + cx.tcx + .erase_late_bound_regions(Binder::bind_with_vars( + cx.typeck_results().node_type(ty.ty.hir_id), + binder_args, + )) .is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()), ) } }, - TyKind::Slice(_) - | TyKind::Array(..) - | TyKind::BareFn(_) - | TyKind::Never + TyKind::Slice(_) => Position::DerefStable(precedence, false), + TyKind::Array(..) | TyKind::Ptr(_) | TyKind::BareFn(_) => Position::DerefStable(precedence, true), + TyKind::Never | TyKind::Tup(_) - | TyKind::Ptr(_) | TyKind::Path(_) => Position::DerefStable( precedence, - cx - .typeck_results() - .node_type(ty.ty.hir_id) - .is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()), + cx.tcx + .erase_late_bound_regions(Binder::bind_with_vars( + cx.typeck_results().node_type(ty.ty.hir_id), + binder_args, + )) + .is_sized(cx.tcx.at(DUMMY_SP), cx.param_env.without_caller_bounds()), ), - TyKind::OpaqueDef(..) - | TyKind::Infer - | TyKind::Typeof(..) - | TyKind::TraitObject(..) - | TyKind::Err => Position::ReborrowStable(precedence), + TyKind::OpaqueDef(..) | TyKind::Infer | TyKind::Typeof(..) | TyKind::TraitObject(..) | TyKind::Err => { + Position::ReborrowStable(precedence) + }, }; } } diff --git a/tests/ui/explicit_auto_deref.fixed b/tests/ui/explicit_auto_deref.fixed index 1e868af87cb8..27bc7fbfae3c 100644 --- a/tests/ui/explicit_auto_deref.fixed +++ b/tests/ui/explicit_auto_deref.fixed @@ -1,5 +1,6 @@ // run-rustfix +#![feature(closure_lifetime_binder)] #![warn(clippy::explicit_auto_deref)] #![allow( dead_code, @@ -255,4 +256,14 @@ fn main() { } let x = S7([0]); let _: &[u32] = &*x; + + let c1 = |x: &Vec<&u32>| {}; + let x = &&vec![&1u32]; + c1(x); + let _ = for<'a, 'b> |x: &'a &'a Vec<&'b u32>, b: bool| -> &'a Vec<&'b u32> { + if b { + return x; + } + *x + }; } diff --git a/tests/ui/explicit_auto_deref.rs b/tests/ui/explicit_auto_deref.rs index a2157d9b4f05..64aea9f464e4 100644 --- a/tests/ui/explicit_auto_deref.rs +++ b/tests/ui/explicit_auto_deref.rs @@ -1,5 +1,6 @@ // run-rustfix +#![feature(closure_lifetime_binder)] #![warn(clippy::explicit_auto_deref)] #![allow( dead_code, @@ -255,4 +256,14 @@ fn main() { } let x = S7([0]); let _: &[u32] = &*x; + + let c1 = |x: &Vec<&u32>| {}; + let x = &&vec![&1u32]; + c1(*x); + let _ = for<'a, 'b> |x: &'a &'a Vec<&'b u32>, b: bool| -> &'a Vec<&'b u32> { + if b { + return *x; + } + *x + }; } diff --git a/tests/ui/explicit_auto_deref.stderr b/tests/ui/explicit_auto_deref.stderr index f2933390fb95..12db0c6f87fc 100644 --- a/tests/ui/explicit_auto_deref.stderr +++ b/tests/ui/explicit_auto_deref.stderr @@ -1,5 +1,5 @@ error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:69:19 + --> $DIR/explicit_auto_deref.rs:70:19 | LL | let _: &str = &*s; | ^^^ help: try this: `&s` @@ -7,214 +7,226 @@ LL | let _: &str = &*s; = note: `-D clippy::explicit-auto-deref` implied by `-D warnings` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:70:19 + --> $DIR/explicit_auto_deref.rs:71:19 | LL | let _: &str = &*{ String::new() }; | ^^^^^^^^^^^^^^^^^^^ help: try this: `&{ String::new() }` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:71:19 + --> $DIR/explicit_auto_deref.rs:72:19 | LL | let _: &str = &mut *{ String::new() }; | ^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut { String::new() }` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:75:11 + --> $DIR/explicit_auto_deref.rs:76:11 | LL | f_str(&*s); | ^^^ help: try this: `&s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:79:13 + --> $DIR/explicit_auto_deref.rs:80:13 | LL | f_str_t(&*s, &*s); // Don't lint second param. | ^^^ help: try this: `&s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:82:24 + --> $DIR/explicit_auto_deref.rs:83:24 | LL | let _: &Box = &**b; | ^^^^ help: try this: `&b` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:88:7 + --> $DIR/explicit_auto_deref.rs:89:7 | LL | c(&*s); | ^^^ help: try this: `&s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:94:9 + --> $DIR/explicit_auto_deref.rs:95:9 | LL | &**x | ^^^^ help: try this: `x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:98:11 + --> $DIR/explicit_auto_deref.rs:99:11 | LL | { &**x } | ^^^^ help: try this: `x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:102:9 + --> $DIR/explicit_auto_deref.rs:103:9 | LL | &**{ x } | ^^^^^^^^ help: try this: `{ x }` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:106:9 + --> $DIR/explicit_auto_deref.rs:107:9 | LL | &***x | ^^^^^ help: try this: `x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:123:12 + --> $DIR/explicit_auto_deref.rs:124:12 | LL | f1(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:124:12 + --> $DIR/explicit_auto_deref.rs:125:12 | LL | f2(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:125:12 + --> $DIR/explicit_auto_deref.rs:126:12 | LL | f3(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:126:27 + --> $DIR/explicit_auto_deref.rs:127:27 | LL | f4.callable_str()(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:127:12 + --> $DIR/explicit_auto_deref.rs:128:12 | LL | f5(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:128:12 + --> $DIR/explicit_auto_deref.rs:129:12 | LL | f6(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:129:27 + --> $DIR/explicit_auto_deref.rs:130:27 | LL | f7.callable_str()(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:130:25 + --> $DIR/explicit_auto_deref.rs:131:25 | LL | f8.callable_t()(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:131:12 + --> $DIR/explicit_auto_deref.rs:132:12 | LL | f9(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:132:13 + --> $DIR/explicit_auto_deref.rs:133:13 | LL | f10(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:133:26 + --> $DIR/explicit_auto_deref.rs:134:26 | LL | f11.callable_t()(&*x); | ^^^ help: try this: `&x` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:137:16 + --> $DIR/explicit_auto_deref.rs:138:16 | LL | let _ = S1(&*s); | ^^^ help: try this: `&s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:142:21 + --> $DIR/explicit_auto_deref.rs:143:21 | LL | let _ = S2 { s: &*s }; | ^^^ help: try this: `&s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:158:30 + --> $DIR/explicit_auto_deref.rs:159:30 | LL | let _ = Self::S1(&**s); | ^^^^ help: try this: `s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:159:35 + --> $DIR/explicit_auto_deref.rs:160:35 | LL | let _ = Self::S2 { s: &**s }; | ^^^^ help: try this: `s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:162:20 + --> $DIR/explicit_auto_deref.rs:163:20 | LL | let _ = E1::S1(&*s); | ^^^ help: try this: `&s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:163:25 + --> $DIR/explicit_auto_deref.rs:164:25 | LL | let _ = E1::S2 { s: &*s }; | ^^^ help: try this: `&s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:181:13 + --> $DIR/explicit_auto_deref.rs:182:13 | LL | let _ = (*b).foo; | ^^^^ help: try this: `b` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:182:13 + --> $DIR/explicit_auto_deref.rs:183:13 | LL | let _ = (**b).foo; | ^^^^^ help: try this: `b` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:197:19 + --> $DIR/explicit_auto_deref.rs:198:19 | LL | let _ = f_str(*ref_str); | ^^^^^^^^ help: try this: `ref_str` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:199:19 + --> $DIR/explicit_auto_deref.rs:200:19 | LL | let _ = f_str(**ref_ref_str); | ^^^^^^^^^^^^^ help: try this: `ref_ref_str` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:209:13 + --> $DIR/explicit_auto_deref.rs:210:13 | LL | f_str(&&*ref_str); // `needless_borrow` will suggest removing both references | ^^^^^^^^ help: try this: `ref_str` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:210:12 + --> $DIR/explicit_auto_deref.rs:211:12 | LL | f_str(&&**ref_str); // `needless_borrow` will suggest removing only one reference | ^^^^^^^^^^ help: try this: `ref_str` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:219:41 + --> $DIR/explicit_auto_deref.rs:220:41 | LL | let _ = || -> &'static str { return *s }; | ^^ help: try this: `s` error: deref which would be done by auto-deref - --> $DIR/explicit_auto_deref.rs:238:9 + --> $DIR/explicit_auto_deref.rs:239:9 | LL | &**x | ^^^^ help: try this: `x` -error: aborting due to 36 previous errors +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:262:8 + | +LL | c1(*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:265:20 + | +LL | return *x; + | ^^ help: try this: `x` + +error: aborting due to 38 previous errors