Skip to content

Commit

Permalink
Remove Unused detection due to inconsistency
Browse files Browse the repository at this point in the history
  • Loading branch information
xFrednet committed Apr 29, 2024
1 parent 94006ac commit b1c79e5
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 89 deletions.
18 changes: 1 addition & 17 deletions clippy_lints/src/borrow_pats/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ pub struct OwnedAnalysis<'a, 'tcx> {
local_info: &'a VarInfo,
/// This should be a `BTreeSet` to have it ordered and consistent.
pats: BTreeSet<OwnedPat>,
/// Counts how many times a value was used. This starts at 1 for arguments otherwise 0.
use_count: u32,
}

impl<'a, 'tcx> OwnedAnalysis<'a, 'tcx> {
Expand All @@ -31,9 +29,6 @@ impl<'a, 'tcx> OwnedAnalysis<'a, 'tcx> {
};
let name = local_kind.name().unwrap();

// Arguments are assigned outside and therefore have at least a use of 1
let use_count = u32::from(local_kind.is_arg());

let bbs_ctn = info.body.basic_blocks.len();
let mut states = IndexVec::with_capacity(bbs_ctn);
for bb in 0..bbs_ctn {
Expand All @@ -48,18 +43,13 @@ impl<'a, 'tcx> OwnedAnalysis<'a, 'tcx> {
local_kind,
local_info,
pats: Default::default(),
use_count,
}
}

pub fn run(info: &'a AnalysisInfo<'tcx>, local: Local) -> BTreeSet<OwnedPat> {
let mut anly = Self::new(info, local);
visit_body_with_state(&mut anly, info);

if anly.use_count == 1 {
anly.pats.insert(OwnedPat::Unused);
}

anly.pats
}

Expand All @@ -83,6 +73,7 @@ pub enum OwnedPat {
#[expect(unused, reason = "Either this needs to be detected consistency or not at all")]
Returned,
/// The value is only assigned once and never read afterwards.
#[expect(unused, reason = "This can't be reliably detected with MIR")]
Unused,
/// The value is dynamically dropped, meaning if it's still valid at a given location.
/// See RFC: #320
Expand Down Expand Up @@ -266,13 +257,6 @@ impl<'a, 'tcx> Visitor<'tcx> for OwnedAnalysis<'a, 'tcx> {
self.visit_terminator_for_anons(term, loc.block);
self.super_terminator(term, loc);
}

fn visit_local(&mut self, local: Local, _context: mir::visit::PlaceContext, _loc: Location) {
// TODO: Check that this isn't called for StorageLife and StorageDead
if local == self.local {
self.use_count += 1;
}
}
}

impl<'a, 'tcx> OwnedAnalysis<'a, 'tcx> {
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/thesis/owned_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ fn assign_drop_field() {
ex.owned_1 = String::new();
}

#[forbid(clippy::borrow_pats)]
#[warn(clippy::borrow_pats)]
fn move_drop_field() {
let ex = Example::default();
// TODO: Chnage state to partially valid
let _hey = ex.owned_1;
}

// #[forbid(clippy::borrow_pats)]
#[warn(clippy::borrow_pats)]
fn copy_field() {
let ex = Example::default();
let _hey = ex.copy_1;
Expand Down
32 changes: 5 additions & 27 deletions tests/ui/thesis/owned_field.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,12 @@
- Body: Output(Unit) , NotConst , Safe , Sync , Free {NoArguments}

# "move_drop_field"
=====
Body for: DefId(0:7 ~ owned_field[df29]::move_drop_field)
bb0:
StorageLive(_1)
_1 = <Example as std::default::Default>::default() -> [return: bb1, unwind continue]

bb1:
StorageLive(_2)
_2 = move (_1.0: std::string::String)
_0 = const ()
drop(_2) -> [return: bb2, unwind: bb5]

bb2:
StorageDead(_2)
drop((_1.1: std::string::String)) -> [return: bb4, unwind: bb3]

bb3:
resume

bb4:
StorageDead(_1)
return

bb5:
drop((_1.1: std::string::String)) -> [return: bb3, unwind terminate(cleanup)]

=====
- ex : (Immutable, Owned , Local , NonCopy, PartDrop, UserDef ) {PartMovedToVar}
- _hey : (Immutable, Owned , Local , NonCopy, PartDrop, UserDef ) {}
- Body: Output(Unit) , NotConst , Safe , Sync , Free {NoArguments}

# "copy_field"
- ex : (Immutable, Owned , Local , NonCopy, PartDrop, UserDef ) {CopiedToVar}
- _hey : (Immutable, Owned , Local , Copy , NonDrop , Primitive) {}
- Body: Output(Unit) , NotConst , Safe , Sync , Free {NoArguments}

8 changes: 4 additions & 4 deletions tests/ui/thesis/pass/owned.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
//@rustc-env: CLIPPY_PETS_PRINT=1

struct Dropper {
duck: u32
duck: u32,
}

impl Drop for Dropper {
fn drop(&mut self) { }
fn drop(&mut self) {}
}

struct Animal {
Expand Down Expand Up @@ -79,13 +79,13 @@ fn conditional_overwrite(mut animal: String, cond: bool) {
if cond {
animal = "Ducks".to_string();
}

magic()
}

fn magic() {}

#[warn(clippy::borrow_pats)]
fn main() {
let dropper = Dropper {duck : 17};
let dropper = Dropper { duck: 17 };
}
6 changes: 3 additions & 3 deletions tests/ui/thesis/pass/owned.stdout
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# "take_one"
- _animal : (Immutable, Owned , Argument, NonCopy, NonDrop , UserDef ) {Unused}
- _animal : (Immutable, Owned , Argument, NonCopy, NonDrop , UserDef ) {}
- Body: Output(Unit) , NotConst , Safe , Sync , Free {}

# "take_two"
- _animal_1 : (Immutable, Owned , Argument, NonCopy, NonDrop , UserDef ) {Unused}
- _animal_2 : (Immutable, Owned , Argument, NonCopy, NonDrop , UserDef ) {Unused}
- _animal_1 : (Immutable, Owned , Argument, NonCopy, NonDrop , UserDef ) {}
- _animal_2 : (Immutable, Owned , Argument, NonCopy, NonDrop , UserDef ) {}
- Body: Output(Unit) , NotConst , Safe , Sync , Free {}

# "pat_return_owned_arg"
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/thesis/pass/owned_mut_to_unmut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn mut_copy_to_immut_and_use_after() {
let _ = snapshot + counter;
}

#[forbid(clippy::borrow_pats)]
#[warn(clippy::borrow_pats)]
fn main() {
let mut s = String::new();
s += "Hey";
Expand Down
35 changes: 0 additions & 35 deletions tests/ui/thesis/pass/owned_mut_to_unmut.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -24,41 +24,6 @@
- Body: Output(Unit) , NotConst , Safe , Sync , Free {NoArguments}

# "main"
=====
Body for: DefId(0:8 ~ owned_mut_to_unmut[4ffe]::main)
bb0:
StorageLive(_1)
_1 = std::string::String::new() -> [return: bb1, unwind continue]

bb1:
StorageLive(_2)
StorageLive(_3)
_3 = &mut _1
StorageLive(_4)
StorageLive(_5)
_5 = const "Hey"
_4 = &(*_5)
_2 = <std::string::String as std::ops::AddAssign<&str>>::add_assign(move _3, move _4) -> [return: bb2, unwind: bb4]

bb2:
StorageDead(_4)
StorageDead(_3)
StorageDead(_5)
StorageDead(_2)
_0 = const ()
drop(_1) -> [return: bb3, unwind: bb5]

bb3:
StorageDead(_1)
return

bb4:
drop(_1) -> [return: bb5, unwind terminate(cleanup)]

bb5:
resume

=====
- s : (Mutable , Owned , Local , NonCopy, PartDrop, UserDef ) {TempBorrowMut}
- Body: Output(Unit) , NotConst , Safe , Sync , Free {NoArguments, HasTempBorrowMut}

0 comments on commit b1c79e5

Please sign in to comment.