Detecting conditional moves and drops

clippy_lints/src/borrow_pats/owned.rs

@@ -191,8 +191,10 @@ pub enum OwnedPat {
     /// A loan of this value was assigned to a named place
     NamedBorrow,
     NamedBorrowMut,
-    ConditionalOverwride,
     ConditionalInit,
+    ConditionalOverwride,
+    ConditionalMove,
+    ConditionalDrop,
 }
 
 impl<'a, 'tcx> MyVisitor<'tcx> for OwnedAnalysis<'a, 'tcx> {

clippy_lints/src/borrow_pats/owned/state.rs

@@ -311,10 +311,10 @@ impl<'a, 'tcx> MyStateInfo<super::OwnedAnalysis<'a, 'tcx>> for StateInfo<'tcx> {
         let self_state = self.state.last().copied().unwrap();
         let other_state = other.state.last().copied().unwrap();
         if self.state.len() != other.state.len() || self_state != other_state {
-            println!("- Merge:");
-            println!("    - {:?}", self.state);
-            println!("    - {:?}", other.state);
-            inspect_deviation(
+            // println!("- Merge:");
+            // println!("    - {:?}", self.state);
+            // println!("    - {:?}", other.state);
+            let other_events = inspect_deviation(
                 &self.state,
                 &other.state,
                 &mut state_owner.pats,
@@ -327,6 +327,16 @@ impl<'a, 'tcx> MyStateInfo<super::OwnedAnalysis<'a, 'tcx>> for StateInfo<'tcx> {
                         if has_fill {
                             pats.insert(OwnedPat::ConditionalOverwride);
                         }
+
+                        let has_drop = deviation.iter().any(|(state, _)| matches!(state, State::Dropped));
+                        if has_drop {
+                            pats.insert(OwnedPat::ConditionalDrop);
+                        }
+
+                        let has_move = deviation.iter().any(|(state, _)| matches!(state, State::Moved));
+                        if has_move {
+                            pats.insert(OwnedPat::ConditionalMove);
+                        }
                     }
                 },
                 |(base, _), a, b, pats| {
@@ -338,12 +348,13 @@ impl<'a, 'tcx> MyStateInfo<super::OwnedAnalysis<'a, 'tcx>> for StateInfo<'tcx> {
                         let a_fill = a.iter().any(|(state, _)| matches!(state, State::Filled));
                         let b_fill = b.iter().any(|(state, _)| matches!(state, State::Filled));
 
-                        if a_fill && b_fill {
+                        if a_fill || b_fill {
                             pats.insert(OwnedPat::ConditionalInit);
                         }
                     }
                 },
             );
+            self.state.extend(other_events.iter().copied());
 
             // TODO: Proper merging here
             let new_state = match (self.validity(), other.validity()) {
@@ -380,9 +391,11 @@ impl<'a, 'tcx> MyStateInfo<super::OwnedAnalysis<'a, 'tcx>> for StateInfo<'tcx> {
 ///     \ |            | /           \   /     //
 ///       x            x               x       //
 /// ```
-fn inspect_deviation(
+/// This returns the deviation of the additional events from the b branch to be
+/// added to the a collection for the next iteration.
+fn inspect_deviation<'b>(
     a: &[(State, BasicBlock)],
-    b: &[(State, BasicBlock)],
+    b: &'b [(State, BasicBlock)],
     pats: &mut BTreeSet<OwnedPat>,
     mut single_devitation: impl FnMut((State, BasicBlock), &[(State, BasicBlock)], &mut BTreeSet<OwnedPat>),
     mut split_devitation: impl FnMut(
@@ -391,22 +404,22 @@ fn inspect_deviation(
         &[(State, BasicBlock)],
         &mut BTreeSet<OwnedPat>,
     ),
-) {
+) -> &'b [(State, BasicBlock)] {
     let a_state = a.last().copied().unwrap();
     let b_state = b.last().copied().unwrap();
 
     // Case 1
     if let Some(idx) = a.iter().rposition(|state| *state == b_state) {
         let base = a[idx];
         single_devitation(base, &a[(idx + 1)..], pats);
-        return;
+        return &[];
     }
 
     // Case 2
     if let Some(idx) = b.iter().rposition(|state| *state == a_state) {
         let base = b[idx];
         single_devitation(base, &b[(idx + 1)..], pats);
-        return;
+        return &b[(idx + 1)..];
     }
 
     let mut b_set = BitSet::new_empty(a_state.1.as_usize().max(b_state.1.as_usize()) + 1);
@@ -419,6 +432,8 @@ fn inspect_deviation(
         && let Some(b_idx) = b.iter().rposition(|state| *state == base)
     {
         split_devitation(base, &a[(a_idx + 1)..], &b[(b_idx + 1)..], pats);
-        return;
+        return &b[(b_idx + 1)..];
     }
+
+    unreachable!()
 }

tests/ui/thesis/owned.rs

@@ -37,7 +37,7 @@ fn pat_maybe_return_owned_arg_1_test(a: u32) -> u32 {
     19
 }
 
-#[forbid(clippy::borrow_pats)]
+#[warn(clippy::borrow_pats)]
 /// FIXME: The argument return is not yet detected both in `a`
 fn pat_maybe_return_owned_arg_2(a: String) -> String {
     let ret;
@@ -50,6 +50,7 @@ fn pat_maybe_return_owned_arg_2(a: String) -> String {
     ret
 }
 
+#[warn(clippy::borrow_pats)]
 fn pat_maybe_return_owned_arg_3(a: String) -> String {
     let ret = if !a.is_empty() { a } else { "hey".to_string() };
     ret
@@ -69,6 +70,15 @@ fn pub_dynamic_drop_1(animal: String, cond: bool) {
     magic()
 }
 
+#[warn(clippy::borrow_pats)]
+fn conditional_overwrite(mut animal: String, cond: bool) {
+    if cond {
+        animal = "Ducks".to_string();
+    }
+
+    magic()
+}
+
 fn magic() {}
 
 fn main() {}

tests/ui/thesis/owned.stdout

@@ -12,9 +12,6 @@
 - Body: Output(UserDef)  , NotConst , Safe  , Sync , Free       {}
 
 # "pat_maybe_return_owned_arg_1"
-- Merge:
-    - [(Filled, bb0), (Moved, bb3)]
-    - [(Filled, bb0), (Dropped, bb4)]
 - a              : (Immutable, Owned    , Argument, NonCopy, Drop   , UserDef  ) {TempBorrow}
 - Body: Output(UserDef)  , NotConst , Safe  , Sync , Free       {HasTempBorrow}
 
@@ -23,177 +20,22 @@
 - Body: Output(Primitive), NotConst , Safe  , Sync , Free       {}
 
 # "pat_maybe_return_owned_arg_2"
-=====
-Body for: DefId(0:12 ~ owned[50f7]::pat_maybe_return_owned_arg_2)
-bb0:
-    _34 = const false
-    _35 = const false
-    _34 = const true
-    StorageLive(_2)
-    StorageLive(_3)
-    StorageLive(_4)
-    StorageLive(_5)
-    _5 = &_1
-    _4 = std::string::String::is_empty(move _5) -> [return: bb1, unwind: bb16]
-
-bb1:
-    switchInt(move _4) -> [0: bb3, otherwise: bb2]
-
-bb2:
-    StorageDead(_5)
-    StorageLive(_7)
-    StorageLive(_8)
-    StorageLive(_9)
-    _9 = const "hey"
-    _8 = &(*_9)
-    _7 = <str as std::string::ToString>::to_string(move _8) -> [return: bb4, unwind: bb16]
-
-bb3:
-    StorageDead(_5)
-    StorageLive(_6)
-    _34 = const false
-    _6 = move _1
-    _35 = const true
-    _2 = move _6
-    StorageDead(_6)
-    _3 = const ()
-    goto -> bb10
-
-bb4:
-    StorageDead(_8)
-    _35 = const true
-    _2 = move _7
-    StorageDead(_7)
-    StorageDead(_9)
-    StorageLive(_10)
-    StorageLive(_11)
-    StorageLive(_12)
-    StorageLive(_13)
-    StorageLive(_14)
-    StorageLive(_15)
-    _33 = const _
-    _15 = &(*_33)
-    _14 = &(*_15)
-    _13 = move _14 as &[&str] (PointerCoercion(Unsize))
-    StorageDead(_14)
-    StorageLive(_17)
-    StorageLive(_18)
-    StorageLive(_19)
-    StorageLive(_20)
-    StorageLive(_21)
-    StorageLive(_22)
-    StorageLive(_23)
-    _23 = &_1
-    _22 = &(*_23)
-    _21 = core::fmt::rt::Argument::<'_>::new_debug::<std::string::String>(move _22) -> [return: bb5, unwind: bb16]
-
-bb5:
-    StorageDead(_22)
-    _20 = [move _21]
-    StorageDead(_21)
-    _19 = &_20
-    _18 = &(*_19)
-    _17 = move _18 as &[core::fmt::rt::Argument<'_>] (PointerCoercion(Unsize))
-    StorageDead(_18)
-    StorageLive(_24)
-    StorageLive(_25)
-    StorageLive(_26)
-    StorageLive(_27)
-    StorageLive(_28)
-    StorageLive(_29)
-    _29 = core::fmt::rt::Alignment::Unknown
-    StorageLive(_30)
-    _30 = core::fmt::rt::Count::Implied
-    StorageLive(_31)
-    _31 = core::fmt::rt::Count::Implied
-    _28 = core::fmt::rt::Placeholder::new(const 0_usize, const ' ', move _29, const 4_u32, move _30, move _31) -> [return: bb6, unwind: bb16]
-
-bb6:
-    StorageDead(_31)
-    StorageDead(_30)
-    StorageDead(_29)
-    _27 = [move _28]
-    StorageDead(_28)
-    _26 = &_27
-    _25 = &(*_26)
-    _24 = move _25 as &[core::fmt::rt::Placeholder] (PointerCoercion(Unsize))
-    StorageDead(_25)
-    StorageLive(_32)
-    _32 = core::fmt::rt::UnsafeArg::new() -> [return: bb7, unwind: bb16]
-
-bb7:
-    _12 = std::fmt::Arguments::<'_>::new_v1_formatted(move _13, move _17, move _24, move _32) -> [return: bb8, unwind: bb16]
-
-bb8:
-    StorageDead(_32)
-    StorageDead(_24)
-    StorageDead(_17)
-    StorageDead(_13)
-    _11 = std::io::_print(move _12) -> [return: bb9, unwind: bb16]
-
-bb9:
-    StorageDead(_12)
-    StorageDead(_27)
-    StorageDead(_26)
-    StorageDead(_23)
-    StorageDead(_20)
-    StorageDead(_19)
-    StorageDead(_15)
-    StorageDead(_11)
-    _10 = const ()
-    StorageDead(_10)
-    _3 = const ()
-    goto -> bb10
-
-bb10:
-    StorageDead(_4)
-    StorageDead(_3)
-    _35 = const false
-    _0 = move _2
-    _35 = const false
-    StorageDead(_2)
-    switchInt(_34) -> [0: bb11, otherwise: bb14]
-
-bb11:
-    return
-
-bb12:
-    drop(_1) -> [return: bb13, unwind terminate(cleanup)]
-
-bb13:
-    resume
-
-bb14:
-    drop(_1) -> [return: bb11, unwind: bb13]
-
-bb15:
-    drop(_2) -> [return: bb12, unwind terminate(cleanup)]
-
-bb16:
-    switchInt(_35) -> [0: bb12, otherwise: bb15]
+- a              : (Immutable, Owned    , Argument, NonCopy, Drop   , UserDef  ) {DynamicDrop, MovedToVar, TempBorrow, TempBorrowExtended, ConditionalMove}
+- ret            : (Immutable, Owned    , Local   , NonCopy, Drop   , UserDef  ) {ConditionalInit}
+- Body: Output(UserDef)  , NotConst , Safe  , Sync , Free       {HasTempBorrow}
 
-=====
-- Merge:
-    - [(Filled, bb0), (Moved, bb3)]
-    - [(Filled, bb0)]
-- Merge:
-    - [(Filled, bb0), (Moved, bb3), (MaybeFilled, bb10)]
-    - [(Filled, bb0), (Moved, bb3), (MaybeFilled, bb10), (Dropped, bb14)]
-- a              : (Immutable, Owned    , Argument, NonCopy, Drop   , UserDef  ) {DynamicDrop, MovedToVar, TempBorrow, TempBorrowExtended}
-- Merge:
-    - [(Empty, bb0), (Filled, bb3)]
-    - [(Empty, bb0), (Filled, bb4)]
+# "pat_maybe_return_owned_arg_3"
+- a              : (Immutable, Owned    , Argument, NonCopy, Drop   , UserDef  ) {DynamicDrop, MovedToVar, TempBorrow, ConditionalMove}
 - ret            : (Immutable, Owned    , Local   , NonCopy, Drop   , UserDef  ) {ConditionalInit}
 - Body: Output(UserDef)  , NotConst , Safe  , Sync , Free       {HasTempBorrow}
 
 # "pub_dynamic_drop_1"
-- Merge:
-    - [(Filled, bb0), (Moved, bb1)]
-    - [(Filled, bb0)]
-- Merge:
-    - [(Filled, bb0), (Moved, bb1), (MaybeFilled, bb4), (Dropped, bb7)]
-    - [(Filled, bb0), (Moved, bb1), (MaybeFilled, bb4)]
-- animal         : (Immutable, Owned    , Argument, NonCopy, Drop   , UserDef  ) {DynamicDrop, MovedToFn, ManualDrop}
+- animal         : (Immutable, Owned    , Argument, NonCopy, Drop   , UserDef  ) {DynamicDrop, MovedToFn, ManualDrop, ConditionalMove}
+- cond           : (Immutable, Owned    , Argument, Copy   , NonDrop, Primitive) {}
+- Body: Output(Unit)     , NotConst , Safe  , Sync , Free       {}
+
+# "conditional_overwrite"
+- animal         : (Mutable  , Owned    , Argument, NonCopy, Drop   , UserDef  ) {Overwrite, ConditionalOverwride, ConditionalDrop}
 - cond           : (Immutable, Owned    , Argument, Copy   , NonDrop, Primitive) {}
 - Body: Output(Unit)     , NotConst , Safe  , Sync , Free       {}