From 2797936f6dcd29777f236a5d966d487a7f4cbbb3 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 22 Feb 2025 22:13:23 +0000 Subject: [PATCH] More comments --- compiler/rustc_lint/src/if_let_rescope.rs | 24 +++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_lint/src/if_let_rescope.rs b/compiler/rustc_lint/src/if_let_rescope.rs index 7a6d656d00d..55d900e0319 100644 --- a/compiler/rustc_lint/src/if_let_rescope.rs +++ b/compiler/rustc_lint/src/if_let_rescope.rs @@ -369,17 +369,22 @@ struct FindSignificantDropper<'a, 'tcx> { } impl<'tcx> FindSignificantDropper<'_, 'tcx> { + /// Check the scrutinee of an `if let` to see if it promotes any temporary values + /// that would change drop order in edition 2024. Specifically, it checks the value + /// of the scrutinee itself, and also recurses into the expression to find any ref + /// exprs (or autoref) which would promote temporaries that would be scoped to the + /// end of this `if`. fn check_if_let_scrutinee(&mut self, init: &'tcx hir::Expr<'tcx>) -> ControlFlow { self.check_promoted_temp_with_drop(init)?; self.visit_expr(init) } - // Check that an expression is not a promoted temporary with a significant - // drop impl. - // - // An expression is a promoted temporary if it has an addr taken (i.e. `&expr`) - // or is the scrutinee of the `if let`, *and* the expression is not a place - // expr, and it has a significant drop. + /// Check that an expression is not a promoted temporary with a significant + /// drop impl. + /// + /// An expression is a promoted temporary if it has an addr taken (i.e. `&expr` or autoref) + /// or is the scrutinee of the `if let`, *and* the expression is not a place + /// expr, and it has a significant drop. fn check_promoted_temp_with_drop(&self, expr: &'tcx hir::Expr<'tcx>) -> ControlFlow { if !expr.is_place_expr(|base| { self.cx @@ -405,7 +410,8 @@ impl<'tcx> Visitor<'tcx> for FindSignificantDropper<'_, 'tcx> { fn visit_block(&mut self, b: &'tcx hir::Block<'tcx>) -> Self::Result { // Blocks introduce temporary terminating scope for all of its - // statements, so just visit the tail expr. + // statements, so just visit the tail expr, skipping over any + // statements. This prevents false positives like `{ let x = &Drop; }`. if let Some(expr) = b.expr { self.visit_expr(expr) } else { ControlFlow::Continue(()) } } @@ -415,6 +421,7 @@ impl<'tcx> Visitor<'tcx> for FindSignificantDropper<'_, 'tcx> { // where `fn as_ref(&self) -> Option<...>`. for adj in self.cx.typeck_results().expr_adjustments(expr) { match adj.kind { + // Skip when we hit the first deref expr. Adjust::Deref(_) => break, Adjust::Borrow(_) => { self.check_promoted_temp_with_drop(expr)?; @@ -424,7 +431,7 @@ impl<'tcx> Visitor<'tcx> for FindSignificantDropper<'_, 'tcx> { } match expr.kind { - // Check for cases like `if let None = Some(&Drop) {} else {}`. + // Account for cases like `if let None = Some(&Drop) {} else {}`. hir::ExprKind::AddrOf(_, _, expr) => { self.check_promoted_temp_with_drop(expr)?; intravisit::walk_expr(self, expr) @@ -438,6 +445,7 @@ impl<'tcx> Visitor<'tcx> for FindSignificantDropper<'_, 'tcx> { hir::ExprKind::Match(scrut, _, _) => self.visit_expr(scrut), // Self explanatory. hir::ExprKind::DropTemps(_) => ControlFlow::Continue(()), + // Otherwise, walk into the expr's parts. _ => intravisit::walk_expr(self, expr), } }