diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index ff32329e431..8c255746478 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/compiler/rustc_hir_analysis/src/check/region.rs @@ -241,18 +241,35 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h // scopes, meaning that temporaries cannot outlive them. // This ensures fixed size stacks. hir::ExprKind::Binary( - source_map::Spanned { node: hir::BinOpKind::And, .. }, - _, + source_map::Spanned { node: outer @ hir::BinOpKind::And, .. }, + ref l, ref r, ) | hir::ExprKind::Binary( - source_map::Spanned { node: hir::BinOpKind::Or, .. }, - _, + source_map::Spanned { node: outer @ hir::BinOpKind::Or, .. }, + ref l, ref r, ) => { // For shortcircuiting operators, mark the RHS as a terminating // scope since it only executes conditionally. + // If the LHS is not another binop itself of the same kind as ours, + // we also mark it as terminating, so that in && or || chains, + // the temporaries are dropped in order instead of the very first + // being dropped last. For the Let exception, see below. + let terminate_lhs = match l.kind { + hir::ExprKind::Let(_) => false, + hir::ExprKind::Binary(source_map::Spanned { node, .. }, ..) + if node == outer => + { + false + } + _ => true, + }; + if terminate_lhs { + terminating(l.hir_id.local_id); + } + // `Let` expressions (in a let-chain) shouldn't be terminating, as their temporaries // should live beyond the immediate expression if !matches!(r.kind, hir::ExprKind::Let(_)) { diff --git a/src/test/ui/drop/drop_order.rs b/src/test/ui/drop/drop_order.rs index 42385216ae7..4e5c1205bb9 100644 --- a/src/test/ui/drop/drop_order.rs +++ b/src/test/ui/drop/drop_order.rs @@ -43,7 +43,7 @@ impl DropOrderCollector { } if { - if self.option_loud_drop(7).is_some() && self.option_loud_drop(6).is_some() { + if self.option_loud_drop(6).is_some() && self.option_loud_drop(7).is_some() { self.loud_drop(8); true } else { @@ -118,17 +118,71 @@ impl DropOrderCollector { } } + fn and_chain(&self) { + // issue-103107 + if self.option_loud_drop(1).is_some() // 1 + && self.option_loud_drop(2).is_some() // 2 + && self.option_loud_drop(3).is_some() // 3 + && self.option_loud_drop(4).is_some() // 4 + && self.option_loud_drop(5).is_some() // 5 + { + self.print(6); // 6 + } + + let _ = self.option_loud_drop(7).is_some() // 1 + && self.option_loud_drop(8).is_some() // 2 + && self.option_loud_drop(9).is_some(); // 3 + self.print(10); // 4 + + // Test associativity + if self.option_loud_drop(11).is_some() // 1 + && (self.option_loud_drop(12).is_some() // 2 + && self.option_loud_drop(13).is_some() // 3 + && self.option_loud_drop(14).is_some()) // 4 + && self.option_loud_drop(15).is_some() // 5 + { + self.print(16); // 6 + } + } + + fn or_chain(&self) { + // issue-103107 + if self.option_loud_drop(1).is_none() // 1 + || self.option_loud_drop(2).is_none() // 2 + || self.option_loud_drop(3).is_none() // 3 + || self.option_loud_drop(4).is_none() // 4 + || self.option_loud_drop(5).is_some() // 5 + { + self.print(6); // 6 + } + + let _ = self.option_loud_drop(7).is_none() // 1 + || self.option_loud_drop(8).is_none() // 2 + || self.option_loud_drop(9).is_none(); // 3 + self.print(10); // 4 + + // Test associativity + if self.option_loud_drop(11).is_none() // 1 + || (self.option_loud_drop(12).is_none() // 2 + || self.option_loud_drop(13).is_none() // 3 + || self.option_loud_drop(14).is_none()) // 4 + || self.option_loud_drop(15).is_some() // 5 + { + self.print(16); // 6 + } + } + fn let_chain(&self) { // take the "then" branch - if self.option_loud_drop(2).is_some() // 2 - && self.option_loud_drop(1).is_some() // 1 + if self.option_loud_drop(1).is_some() // 1 + && self.option_loud_drop(2).is_some() // 2 && let Some(_d) = self.option_loud_drop(4) { // 4 self.print(3); // 3 } // take the "else" branch - if self.option_loud_drop(6).is_some() // 2 - && self.option_loud_drop(5).is_some() // 1 + if self.option_loud_drop(5).is_some() // 1 + && self.option_loud_drop(6).is_some() // 2 && let None = self.option_loud_drop(8) { // 4 unreachable!(); } else { @@ -152,8 +206,8 @@ impl DropOrderCollector { } // let exprs last - if self.option_loud_drop(20).is_some() // 2 - && self.option_loud_drop(19).is_some() // 1 + if self.option_loud_drop(19).is_some() // 1 + && self.option_loud_drop(20).is_some() // 2 && let Some(_d) = self.option_loud_drop(23) // 5 && let Some(_e) = self.option_loud_drop(22) { // 4 self.print(21); // 3 @@ -187,6 +241,16 @@ fn main() { collector.if_(); collector.assert_sorted(); + println!("-- and chain --"); + let collector = DropOrderCollector::default(); + collector.and_chain(); + collector.assert_sorted(); + + println!("-- or chain --"); + let collector = DropOrderCollector::default(); + collector.or_chain(); + collector.assert_sorted(); + println!("-- if let --"); let collector = DropOrderCollector::default(); collector.if_let(); diff --git a/src/test/ui/drop/issue-103107.rs b/src/test/ui/drop/issue-103107.rs new file mode 100644 index 00000000000..5f447595662 --- /dev/null +++ b/src/test/ui/drop/issue-103107.rs @@ -0,0 +1,37 @@ +// check-pass +// compile-flags: -Z validate-mir + +struct Foo<'a>(&'a mut u32); + +impl<'a> Drop for Foo<'a> { + fn drop(&mut self) { + *self.0 = 0; + } +} + +fn and() { + let mut foo = 0; + // This used to compile also before the fix + if true && *Foo(&mut foo).0 == 0 && ({ foo = 0; true}) {} + + // This used to fail before the fix + if *Foo(&mut foo).0 == 0 && ({ foo = 0; true}) {} + + println!("{foo}"); +} + +fn or() { + let mut foo = 0; + // This used to compile also before the fix + if false || *Foo(&mut foo).0 == 1 || ({ foo = 0; true}) {} + + // This used to fail before the fix + if *Foo(&mut foo).0 == 1 || ({ foo = 0; true}) {} + + println!("{foo}"); +} + +fn main() { + and(); + or(); +}