Shorten drop scope of temporaries in conditions

compiler/rustc_ast_lowering/src/expr.rs

@@ -419,14 +419,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
     // Lowers a condition (i.e. `cond` in `if cond` or `while cond`), wrapping it in a terminating scope
     // so that temporaries created in the condition don't live beyond it.
     fn lower_cond(&mut self, cond: &Expr) -> &'hir hir::Expr<'hir> {
-        fn has_let_expr(expr: &Expr) -> bool {
-            match &expr.kind {
-                ExprKind::Binary(_, lhs, rhs) => has_let_expr(lhs) || has_let_expr(rhs),
-                ExprKind::Let(..) => true,
-                _ => false,
-            }
-        }
-
         // We have to take special care for `let` exprs in the condition, e.g. in
         // `if let pat = val` or `if foo && let pat = val`, as we _do_ want `val` to live beyond the
         // condition in this case.
@@ -435,9 +427,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         // we still wrap them in terminating scopes, e.g. `if foo && let pat = val` essentially
         // gets transformed into `if { let _t = foo; _t } && let pat = val`
         match &cond.kind {
-            ExprKind::Binary(op @ Spanned { node: ast::BinOpKind::And, .. }, lhs, rhs)
-                if has_let_expr(cond) =>
-            {
+            ExprKind::Binary(op @ Spanned { node: ast::BinOpKind::And, .. }, lhs, rhs) => {
                 let op = self.lower_binop(*op);
                 let lhs = self.lower_cond(lhs);
                 let rhs = self.lower_cond(rhs);

src/tools/clippy/clippy_lints/src/booleans.rs

@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
 use clippy_utils::eq_expr_value;
+use clippy_utils::peel_droptemps;
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
 use if_chain::if_chain;
@@ -114,6 +115,7 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
     }
 
     fn run(&mut self, e: &'v Expr<'_>) -> Result<Bool, String> {
+        let e = peel_droptemps(e);
         fn negate(bin_op_kind: BinOpKind) -> Option<BinOpKind> {
             match bin_op_kind {
                 BinOpKind::Eq => Some(BinOpKind::Ne),

src/tools/clippy/clippy_lints/src/operators/mod.rs

@@ -23,6 +23,7 @@ mod verbose_bit_mask;
 
 pub(crate) mod arithmetic_side_effects;
 
+use clippy_utils::peel_droptemps;
 use rustc_hir::{Body, Expr, ExprKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
@@ -825,6 +826,8 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
         eq_op::check_assert(cx, e);
         match e.kind {
             ExprKind::Binary(op, lhs, rhs) => {
+                let lhs = peel_droptemps(lhs);
+                let rhs = peel_droptemps(rhs);
                 if !e.span.from_expansion() {
                     absurd_extreme_comparisons::check(cx, e, op.node, lhs, rhs);
                     if !(macro_with_not_op(lhs) || macro_with_not_op(rhs)) {

src/tools/clippy/clippy_lints/src/unwrap.rs

@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::higher;
+use clippy_utils::peel_droptemps;
 use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::{path_to_local, usage::is_potentially_mutated};
 use if_chain::if_chain;
@@ -142,6 +143,7 @@ fn collect_unwrap_info<'tcx>(
         is_type_diagnostic_item(cx, ty, sym::Result) && ["is_ok", "is_err"].contains(&method_name)
     }
 
+    let expr = peel_droptemps(expr);
     if let ExprKind::Binary(op, left, right) = &expr.kind {
         match (invert, op.node) {
             (false, BinOpKind::And | BinOpKind::BitAnd) | (true, BinOpKind::Or | BinOpKind::BitOr) => {

src/tools/clippy/clippy_utils/src/consts.rs

@@ -375,6 +375,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
             },
             ExprKind::Index(arr, index) => self.index(arr, index),
             ExprKind::AddrOf(_, _, inner) => self.expr(inner).map(|r| Constant::Ref(Box::new(r))),
+            ExprKind::DropTemps(e) => self.expr(e),
             // TODO: add other expressions.
             _ => None,
         }

src/tools/clippy/clippy_utils/src/higher.rs

@@ -4,13 +4,13 @@
 
 use crate::consts::{constant_simple, Constant};
 use crate::ty::is_type_diagnostic_item;
-use crate::{is_expn_of, match_def_path, paths};
+use crate::{is_expn_of, match_def_path, paths, peel_droptemps};
 use if_chain::if_chain;
 use rustc_ast::ast;
 use rustc_hir as hir;
-use rustc_hir::{Arm, Block, Expr, ExprKind, HirId, LoopSource, MatchSource, Node, Pat, QPath};
+use rustc_hir::{Arm, BinOpKind, Block, Expr, ExprKind, HirId, LoopSource, MatchSource, Node, Pat, QPath};
 use rustc_lint::LateContext;
-use rustc_span::{sym, symbol, Span};
+use rustc_span::{source_map::Spanned, sym, symbol, Span};
 
 /// The essential nodes of a desugared for loop as well as the entire span:
 /// `for pat in arg { body }` becomes `(pat, arg, body)`. Return `(pat, arg, body, span)`.
@@ -63,20 +63,35 @@ pub struct If<'hir> {
     pub r#else: Option<&'hir Expr<'hir>>,
 }
 
+const fn has_let(expr: &Expr<'_>) -> bool {
+    match expr.kind {
+        ExprKind::DropTemps(..) => false,
+        ExprKind::Let(..) => true,
+        ExprKind::Binary(
+            Spanned {
+                node: BinOpKind::And, ..
+            },
+            lhs,
+            rhs,
+        ) => has_let(lhs) || has_let(rhs),
+        _ => panic!("unexpected"),
+    }
+}
+
 impl<'hir> If<'hir> {
     #[inline]
     /// Parses an `if` expression
     pub const fn hir(expr: &Expr<'hir>) -> Option<Self> {
-        if let ExprKind::If(
-            Expr {
-                kind: ExprKind::DropTemps(cond),
-                ..
-            },
-            then,
-            r#else,
-        ) = expr.kind
-        {
-            Some(Self { cond, then, r#else })
+        if let ExprKind::If(cond, then, r#else) = expr.kind {
+            if !has_let(cond) {
+                Some(Self {
+                    cond: peel_droptemps(cond),
+                    then,
+                    r#else,
+                })
+            } else {
+                None
+            }
         } else {
             None
         }
@@ -323,15 +338,7 @@ impl<'hir> While<'hir> {
             Block {
                 expr:
                     Some(Expr {
-                        kind:
-                            ExprKind::If(
-                                Expr {
-                                    kind: ExprKind::DropTemps(condition),
-                                    ..
-                                },
-                                body,
-                                _,
-                            ),
+                        kind: ExprKind::If(cond, body, _),
                         ..
                     }),
                 ..
@@ -341,9 +348,18 @@ impl<'hir> While<'hir> {
             span,
         ) = expr.kind
         {
-            return Some(Self { condition, body, span });
+            if !has_let(cond) {
+                Some(Self {
+                    condition: peel_droptemps(cond),
+                    body,
+                    span,
+                })
+            } else {
+                None
+            }
+        } else {
+            None
         }
-        None
     }
 }
 

src/tools/clippy/clippy_utils/src/lib.rs

@@ -1388,6 +1388,13 @@ pub fn get_parent_as_impl(tcx: TyCtxt<'_>, id: HirId) -> Option<&Impl<'_>> {
     }
 }
 
+pub const fn peel_droptemps<'a>(expr: &'a Expr<'a>) -> &'a Expr<'a> {
+    match expr.kind {
+        ExprKind::DropTemps(source) => peel_droptemps(source),
+        _ => expr,
+    }
+}
+
 /// Removes blocks around an expression, only if the block contains just one expression
 /// and no statements. Unsafe blocks are not removed.
 ///

tests/mir-opt/building/logical_and_in_conditional.function_from_issue_111583.built.after.mir

@@ -0,0 +1,103 @@
+// MIR for `function_from_issue_111583` after built
+
+fn function_from_issue_111583(_1: f64) -> () {
+    debug z => _1;                       // in scope 0 at $DIR/logical_and_in_conditional.rs:+0:31: +0:32
+    let mut _0: ();                      // return place in scope 0 at $DIR/logical_and_in_conditional.rs:+0:39: +0:39
+    let _2: u64;                         // in scope 0 at $DIR/logical_and_in_conditional.rs:+1:9: +1:13
+    let mut _3: u64;                     // in scope 0 at $DIR/logical_and_in_conditional.rs:+1:16: +1:25
+    let mut _5: bool;                    // in scope 0 at $DIR/logical_and_in_conditional.rs:+3:8: +3:38
+    let mut _6: u64;                     // in scope 0 at $DIR/logical_and_in_conditional.rs:+3:8: +3:33
+    let mut _7: u64;                     // in scope 0 at $DIR/logical_and_in_conditional.rs:+3:8: +3:26
+    let mut _8: u64;                     // in scope 0 at $DIR/logical_and_in_conditional.rs:+3:9: +3:20
+    let mut _9: f64;                     // in scope 0 at $DIR/logical_and_in_conditional.rs:+3:9: +3:10
+    let mut _10: u64;                    // in scope 0 at $DIR/logical_and_in_conditional.rs:+3:29: +3:33
+    let mut _11: bool;                   // in scope 0 at $DIR/logical_and_in_conditional.rs:+3:42: +3:60
+    let mut _12: f64;                    // in scope 0 at $DIR/logical_and_in_conditional.rs:+3:42: +3:52
+    let mut _13: f64;                    // in scope 0 at $DIR/logical_and_in_conditional.rs:+3:42: +3:43
+    scope 1 {
+        debug mask => _2;                // in scope 1 at $DIR/logical_and_in_conditional.rs:+1:9: +1:13
+        let mut _4: i32;                 // in scope 1 at $DIR/logical_and_in_conditional.rs:+2:9: +2:16
+        scope 2 {
+            debug ret => _4;             // in scope 2 at $DIR/logical_and_in_conditional.rs:+2:9: +2:16
+        }
+    }
+
+    bb0: {
+        StorageLive(_2);                 // scope 0 at $DIR/logical_and_in_conditional.rs:+1:9: +1:13
+        StorageLive(_3);                 // scope 0 at $DIR/logical_and_in_conditional.rs:+1:16: +1:25
+        _3 = Shl(const 1_u64, const 38_i32); // scope 0 at $DIR/logical_and_in_conditional.rs:+1:16: +1:25
+        _2 = Sub(move _3, const 1_u64);  // scope 0 at $DIR/logical_and_in_conditional.rs:+1:16: +1:29
+        StorageDead(_3);                 // scope 0 at $DIR/logical_and_in_conditional.rs:+1:28: +1:29
+        FakeRead(ForLet(None), _2);      // scope 0 at $DIR/logical_and_in_conditional.rs:+1:9: +1:13
+        StorageLive(_4);                 // scope 1 at $DIR/logical_and_in_conditional.rs:+2:9: +2:16
+        _4 = const 0_i32;                // scope 1 at $DIR/logical_and_in_conditional.rs:+2:19: +2:20
+        FakeRead(ForLet(None), _4);      // scope 1 at $DIR/logical_and_in_conditional.rs:+2:9: +2:16
+        StorageLive(_5);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+3:8: +3:38
+        StorageLive(_6);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+3:8: +3:33
+        StorageLive(_7);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+3:8: +3:26
+        StorageLive(_8);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+3:9: +3:20
+        StorageLive(_9);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+3:9: +3:10
+        _9 = _1;                         // scope 2 at $DIR/logical_and_in_conditional.rs:+3:9: +3:10
+        _8 = core::f64::<impl f64>::to_bits(move _9) -> [return: bb1, unwind: bb8]; // scope 2 at $DIR/logical_and_in_conditional.rs:+3:9: +3:20
+                                         // mir::Constant
+                                         // + span: $DIR/logical_and_in_conditional.rs:19:11: 19:18
+                                         // + literal: Const { ty: fn(f64) -> u64 {core::f64::<impl f64>::to_bits}, val: Value(<ZST>) }
+    }
+
+    bb1: {
+        StorageDead(_9);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+3:19: +3:20
+        _7 = Shr(move _8, const 8_i32);  // scope 2 at $DIR/logical_and_in_conditional.rs:+3:8: +3:26
+        StorageDead(_8);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+3:25: +3:26
+        StorageLive(_10);                // scope 2 at $DIR/logical_and_in_conditional.rs:+3:29: +3:33
+        _10 = _2;                        // scope 2 at $DIR/logical_and_in_conditional.rs:+3:29: +3:33
+        _6 = BitAnd(move _7, move _10);  // scope 2 at $DIR/logical_and_in_conditional.rs:+3:8: +3:33
+        StorageDead(_10);                // scope 2 at $DIR/logical_and_in_conditional.rs:+3:32: +3:33
+        StorageDead(_7);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+3:32: +3:33
+        _5 = Eq(move _6, const 0_u64);   // scope 2 at $DIR/logical_and_in_conditional.rs:+3:8: +3:38
+        StorageDead(_6);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+3:37: +3:38
+        switchInt(move _5) -> [0: bb3, otherwise: bb2]; // scope 2 at $DIR/logical_and_in_conditional.rs:+3:8: +3:38
+    }
+
+    bb2: {
+        StorageLive(_11);                // scope 2 at $DIR/logical_and_in_conditional.rs:+3:42: +3:60
+        StorageLive(_12);                // scope 2 at $DIR/logical_and_in_conditional.rs:+3:42: +3:52
+        StorageLive(_13);                // scope 2 at $DIR/logical_and_in_conditional.rs:+3:42: +3:43
+        _13 = _1;                        // scope 2 at $DIR/logical_and_in_conditional.rs:+3:42: +3:43
+        _12 = Rem(move _13, const 0.0625f64); // scope 2 at $DIR/logical_and_in_conditional.rs:+3:42: +3:52
+        StorageDead(_13);                // scope 2 at $DIR/logical_and_in_conditional.rs:+3:51: +3:52
+        _11 = Lt(move _12, const 1.0E-13f64); // scope 2 at $DIR/logical_and_in_conditional.rs:+3:42: +3:60
+        StorageDead(_12);                // scope 2 at $DIR/logical_and_in_conditional.rs:+3:59: +3:60
+        switchInt(move _11) -> [0: bb5, otherwise: bb4]; // scope 2 at $DIR/logical_and_in_conditional.rs:+3:42: +3:60
+    }
+
+    bb3: {
+        goto -> bb6;                     // scope 2 at $DIR/logical_and_in_conditional.rs:+3:8: +3:38
+    }
+
+    bb4: {
+        _4 = Add(_4, const 1_i32);       // scope 2 at $DIR/logical_and_in_conditional.rs:+4:9: +4:17
+        _0 = const ();                   // scope 2 at $DIR/logical_and_in_conditional.rs:+3:61: +5:6
+        goto -> bb7;                     // scope 2 at $DIR/logical_and_in_conditional.rs:+3:5: +5:6
+    }
+
+    bb5: {
+        goto -> bb6;                     // scope 2 at $DIR/logical_and_in_conditional.rs:+3:42: +3:60
+    }
+
+    bb6: {
+        _0 = const ();                   // scope 2 at $DIR/logical_and_in_conditional.rs:+5:6: +5:6
+        goto -> bb7;                     // scope 2 at $DIR/logical_and_in_conditional.rs:+3:5: +5:6
+    }
+
+    bb7: {
+        StorageDead(_11);                // scope 2 at $DIR/logical_and_in_conditional.rs:+5:5: +5:6
+        StorageDead(_5);                 // scope 2 at $DIR/logical_and_in_conditional.rs:+5:5: +5:6
+        StorageDead(_4);                 // scope 1 at $DIR/logical_and_in_conditional.rs:+6:1: +6:2
+        StorageDead(_2);                 // scope 0 at $DIR/logical_and_in_conditional.rs:+6:1: +6:2
+        return;                          // scope 0 at $DIR/logical_and_in_conditional.rs:+6:2: +6:2
+    }
+
+    bb8 (cleanup): {
+        resume;                          // scope 0 at $DIR/logical_and_in_conditional.rs:+0:1: +6:2
+    }
+}

tests/mir-opt/building/logical_and_in_conditional.rs

@@ -0,0 +1,33 @@
+struct Droppy(bool);
+
+impl Drop for Droppy {
+    fn drop(&mut self) {
+        println!("drop");
+    }
+}
+
+// EMIT_MIR logical_and_in_conditional.test_and.built.after.mir
+fn test_and(a: i32, b: i32, c: i32) {
+    if Droppy(a == 0).0 && Droppy(b == 0).0 && Droppy(c == 0).0 {}
+}
+
+// https://github.com/rust-lang/rust/issues/111583
+// EMIT_MIR logical_and_in_conditional.function_from_issue_111583.built.after.mir
+fn function_from_issue_111583(z: f64) {
+    let mask = (1 << 38) - 1;
+    let mut ret = 0;
+    if (z.to_bits() >> 8) & mask == 0 && z % 0.0625 < 1e-13 {
+        ret += 1;
+    }
+}
+
+fn main() {
+    function_from_issue_111583(0.0);
+    for a in 0..1 {
+        for b in 0..1 {
+            for c in 0..1 {
+                test_and(a, b, c);
+            }
+        }
+    }
+}