|
1 | 1 | use std::iter::repeat; |
2 | 2 | use std::ops::ControlFlow; |
3 | 3 |
|
4 | | -use hir::intravisit::Visitor; |
| 4 | +use hir::intravisit::{self, Visitor}; |
5 | 5 | use rustc_ast::Recovered; |
6 | 6 | use rustc_errors::{ |
7 | 7 | Applicability, Diag, EmissionGuarantee, SubdiagMessageOp, Subdiagnostic, SuggestionStyle, |
8 | 8 | }; |
9 | 9 | use rustc_hir::{self as hir, HirIdSet}; |
10 | 10 | use rustc_macros::LintDiagnostic; |
11 | 11 | use rustc_middle::ty::TyCtxt; |
| 12 | +use rustc_middle::ty::adjustment::Adjust; |
12 | 13 | use rustc_session::lint::{FutureIncompatibilityReason, LintId}; |
13 | 14 | use rustc_session::{declare_lint, impl_lint_pass}; |
14 | 15 | use rustc_span::Span; |
@@ -160,7 +161,7 @@ impl IfLetRescope { |
160 | 161 | let lifetime_end = source_map.end_point(conseq.span); |
161 | 162 |
|
162 | 163 | if let ControlFlow::Break(significant_dropper) = |
163 | | - (FindSignificantDropper { cx }).visit_expr(init) |
| 164 | + (FindSignificantDropper { cx }).check_if_let_scrutinee(init) |
164 | 165 | { |
165 | 166 | first_if_to_lint = first_if_to_lint.or_else(|| Some((span, expr.hir_id))); |
166 | 167 | significant_droppers.push(significant_dropper); |
@@ -363,96 +364,97 @@ enum SingleArmMatchBegin { |
363 | 364 | WithoutOpenBracket(Span), |
364 | 365 | } |
365 | 366 |
|
366 | | -struct FindSignificantDropper<'tcx, 'a> { |
| 367 | +struct FindSignificantDropper<'a, 'tcx> { |
367 | 368 | cx: &'a LateContext<'tcx>, |
368 | 369 | } |
369 | 370 |
|
370 | | -impl<'tcx, 'a> Visitor<'tcx> for FindSignificantDropper<'tcx, 'a> { |
371 | | - type Result = ControlFlow<Span>; |
| 371 | +impl<'tcx> FindSignificantDropper<'_, 'tcx> { |
| 372 | + /// Check the scrutinee of an `if let` to see if it promotes any temporary values |
| 373 | + /// that would change drop order in edition 2024. Specifically, it checks the value |
| 374 | + /// of the scrutinee itself, and also recurses into the expression to find any ref |
| 375 | + /// exprs (or autoref) which would promote temporaries that would be scoped to the |
| 376 | + /// end of this `if`. |
| 377 | + fn check_if_let_scrutinee(&mut self, init: &'tcx hir::Expr<'tcx>) -> ControlFlow<Span> { |
| 378 | + self.check_promoted_temp_with_drop(init)?; |
| 379 | + self.visit_expr(init) |
| 380 | + } |
372 | 381 |
|
373 | | - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) -> Self::Result { |
374 | | - if self |
| 382 | + /// Check that an expression is not a promoted temporary with a significant |
| 383 | + /// drop impl. |
| 384 | + /// |
| 385 | + /// An expression is a promoted temporary if it has an addr taken (i.e. `&expr` or autoref) |
| 386 | + /// or is the scrutinee of the `if let`, *and* the expression is not a place |
| 387 | + /// expr, and it has a significant drop. |
| 388 | + fn check_promoted_temp_with_drop(&self, expr: &'tcx hir::Expr<'tcx>) -> ControlFlow<Span> { |
| 389 | + if !expr.is_place_expr(|base| { |
| 390 | + self.cx |
| 391 | + .typeck_results() |
| 392 | + .adjustments() |
| 393 | + .get(base.hir_id) |
| 394 | + .is_some_and(|x| x.iter().any(|adj| matches!(adj.kind, Adjust::Deref(_)))) |
| 395 | + }) && self |
375 | 396 | .cx |
376 | 397 | .typeck_results() |
377 | 398 | .expr_ty(expr) |
378 | 399 | .has_significant_drop(self.cx.tcx, self.cx.typing_env()) |
379 | 400 | { |
380 | | - return ControlFlow::Break(expr.span); |
| 401 | + ControlFlow::Break(expr.span) |
| 402 | + } else { |
| 403 | + ControlFlow::Continue(()) |
381 | 404 | } |
382 | | - match expr.kind { |
383 | | - hir::ExprKind::ConstBlock(_) |
384 | | - | hir::ExprKind::Lit(_) |
385 | | - | hir::ExprKind::Path(_) |
386 | | - | hir::ExprKind::Assign(_, _, _) |
387 | | - | hir::ExprKind::AssignOp(_, _, _) |
388 | | - | hir::ExprKind::Break(_, _) |
389 | | - | hir::ExprKind::Continue(_) |
390 | | - | hir::ExprKind::Ret(_) |
391 | | - | hir::ExprKind::Become(_) |
392 | | - | hir::ExprKind::InlineAsm(_) |
393 | | - | hir::ExprKind::OffsetOf(_, _) |
394 | | - | hir::ExprKind::Repeat(_, _) |
395 | | - | hir::ExprKind::Err(_) |
396 | | - | hir::ExprKind::Struct(_, _, _) |
397 | | - | hir::ExprKind::Closure(_) |
398 | | - | hir::ExprKind::Block(_, _) |
399 | | - | hir::ExprKind::DropTemps(_) |
400 | | - | hir::ExprKind::Loop(_, _, _, _) => ControlFlow::Continue(()), |
| 405 | + } |
| 406 | +} |
401 | 407 |
|
402 | | - hir::ExprKind::Tup(exprs) | hir::ExprKind::Array(exprs) => { |
403 | | - for expr in exprs { |
404 | | - self.visit_expr(expr)?; |
405 | | - } |
406 | | - ControlFlow::Continue(()) |
407 | | - } |
408 | | - hir::ExprKind::Call(callee, args) => { |
409 | | - self.visit_expr(callee)?; |
410 | | - for expr in args { |
411 | | - self.visit_expr(expr)?; |
412 | | - } |
413 | | - ControlFlow::Continue(()) |
414 | | - } |
415 | | - hir::ExprKind::MethodCall(_, receiver, args, _) => { |
416 | | - self.visit_expr(receiver)?; |
417 | | - for expr in args { |
418 | | - self.visit_expr(expr)?; |
| 408 | +impl<'tcx> Visitor<'tcx> for FindSignificantDropper<'_, 'tcx> { |
| 409 | + type Result = ControlFlow<Span>; |
| 410 | + |
| 411 | + fn visit_block(&mut self, b: &'tcx hir::Block<'tcx>) -> Self::Result { |
| 412 | + // Blocks introduce temporary terminating scope for all of its |
| 413 | + // statements, so just visit the tail expr, skipping over any |
| 414 | + // statements. This prevents false positives like `{ let x = &Drop; }`. |
| 415 | + if let Some(expr) = b.expr { self.visit_expr(expr) } else { ControlFlow::Continue(()) } |
| 416 | + } |
| 417 | + |
| 418 | + fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) -> Self::Result { |
| 419 | + // Check for promoted temporaries from autoref, e.g. |
| 420 | + // `if let None = TypeWithDrop.as_ref() {} else {}` |
| 421 | + // where `fn as_ref(&self) -> Option<...>`. |
| 422 | + for adj in self.cx.typeck_results().expr_adjustments(expr) { |
| 423 | + match adj.kind { |
| 424 | + // Skip when we hit the first deref expr. |
| 425 | + Adjust::Deref(_) => break, |
| 426 | + Adjust::Borrow(_) => { |
| 427 | + self.check_promoted_temp_with_drop(expr)?; |
419 | 428 | } |
420 | | - ControlFlow::Continue(()) |
421 | | - } |
422 | | - hir::ExprKind::Index(left, right, _) | hir::ExprKind::Binary(_, left, right) => { |
423 | | - self.visit_expr(left)?; |
424 | | - self.visit_expr(right) |
| 429 | + _ => {} |
425 | 430 | } |
426 | | - hir::ExprKind::Unary(_, expr) |
427 | | - | hir::ExprKind::Cast(expr, _) |
428 | | - | hir::ExprKind::Type(expr, _) |
429 | | - | hir::ExprKind::UnsafeBinderCast(_, expr, _) |
430 | | - | hir::ExprKind::Yield(expr, _) |
431 | | - | hir::ExprKind::AddrOf(_, _, expr) |
432 | | - | hir::ExprKind::Match(expr, _, _) |
433 | | - | hir::ExprKind::Field(expr, _) |
434 | | - | hir::ExprKind::Let(&hir::LetExpr { |
435 | | - init: expr, |
436 | | - span: _, |
437 | | - pat: _, |
438 | | - ty: _, |
439 | | - recovered: Recovered::No, |
440 | | - }) => self.visit_expr(expr), |
441 | | - hir::ExprKind::Let(_) => ControlFlow::Continue(()), |
| 431 | + } |
442 | 432 |
|
443 | | - hir::ExprKind::If(cond, _, _) => { |
444 | | - if let hir::ExprKind::Let(hir::LetExpr { |
445 | | - init, |
446 | | - span: _, |
447 | | - pat: _, |
448 | | - ty: _, |
449 | | - recovered: Recovered::No, |
450 | | - }) = cond.kind |
451 | | - { |
452 | | - self.visit_expr(init)?; |
453 | | - } |
454 | | - ControlFlow::Continue(()) |
| 433 | + match expr.kind { |
| 434 | + // Account for cases like `if let None = Some(&Drop) {} else {}`. |
| 435 | + hir::ExprKind::AddrOf(_, _, expr) => { |
| 436 | + self.check_promoted_temp_with_drop(expr)?; |
| 437 | + intravisit::walk_expr(self, expr) |
| 438 | + } |
| 439 | + // `(Drop, ()).1` introduces a temporary and then moves out of |
| 440 | + // part of it, therefore we should check it for temporaries. |
| 441 | + // FIXME: This may have false positives if we move the part |
| 442 | + // that actually has drop, but oh well. |
| 443 | + hir::ExprKind::Index(expr, _, _) | hir::ExprKind::Field(expr, _) => { |
| 444 | + self.check_promoted_temp_with_drop(expr)?; |
| 445 | + intravisit::walk_expr(self, expr) |
455 | 446 | } |
| 447 | + // If always introduces a temporary terminating scope for its cond and arms, |
| 448 | + // so don't visit them. |
| 449 | + hir::ExprKind::If(..) => ControlFlow::Continue(()), |
| 450 | + // Match introduces temporary terminating scopes for arms, so don't visit |
| 451 | + // them, and only visit the scrutinee to account for cases like: |
| 452 | + // `if let None = match &Drop { _ => Some(1) } {} else {}`. |
| 453 | + hir::ExprKind::Match(scrut, _, _) => self.visit_expr(scrut), |
| 454 | + // Self explanatory. |
| 455 | + hir::ExprKind::DropTemps(_) => ControlFlow::Continue(()), |
| 456 | + // Otherwise, walk into the expr's parts. |
| 457 | + _ => intravisit::walk_expr(self, expr), |
456 | 458 | } |
457 | 459 | } |
458 | 460 | } |
0 commit comments