Skip to content

Commit

Permalink
Correct owned value stat tracking
Browse files Browse the repository at this point in the history
  • Loading branch information
xFrednet committed Apr 27, 2024
1 parent 9541343 commit 3251b8f
Show file tree
Hide file tree
Showing 14 changed files with 701 additions and 251 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/borrow_pats/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#![warn(unused)]

use super::prelude::*;
use super::{calc_fn_arg_relations, has_mut_ref, visit_body};
use super::{calc_fn_arg_relations, has_mut_ref, visit_body, BodyStats};

use clippy_utils::ty::for_each_region;

Expand Down
61 changes: 0 additions & 61 deletions clippy_lints/src/borrow_pats/body/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,64 +75,3 @@ pub enum BodyContext {
TraitDef,
}

/// Most of these statistics need to be filled by the individual analysis passed.
/// Every value should document which pass might modify/fill it.
///
/// Without more context and tracking the data flow, it's impossible to know what
/// certain instructions are.
///
/// For example, a named borrow can have different shapes. Assuming `_1` is the
/// owned value and `_2` is the named references, they could have the following
/// shapes:
///
/// ```
/// // Direct
/// _2 = &_1
///
/// // Indirect
/// _3 = &_1
/// _2 = &(*_3)
///
/// // Indirect + Copy
/// _3 = &_1
/// _4 = &(*_3)
/// _2 = move _4
/// ```
#[derive(Debug, Clone, Default)]
pub struct BodyStats {
/// Number of relations between the arguments and the return value accoring
/// to the function signature
///
/// Filled by `BodyAnalysis`
pub return_relations_signature: usize,
/// Number of relations between the arguments and the return value that have
/// been found inside the body
///
/// Filled by `BodyAnalysis`
pub return_relations_found: usize,
/// Number of relations between arguments according to the signature
///
/// Filled by `BodyAnalysis`
pub arg_relations_signature: usize,
/// Number of relations between arguments that have been found in the body
///
/// Filled by `BodyAnalysis`
pub arg_relations_found: usize,

/// The number of borrows into anonymous values.
///
/// These are collected by the BodyAnalysis
pub anon_borrow_count: usize,
pub anon_borrow_count_mut: usize,
/// The number of borrows into named values.
///
/// These are collected by the BodyAnalysis
pub named_borrow_count: usize,
pub named_borrow_count_mut: usize,
/// These are collected by the OnwedAnalysis and LoanAnalysis respectivly
///
/// Note:
/// - This only counts the confirmed two phased borrows.
/// - The borrows that produce the two phased borrow are also counted above.
pub two_phase_borrows: usize,
}
2 changes: 1 addition & 1 deletion clippy_lints/src/borrow_pats/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_middle::mir::{BasicBlock, Local, Place};
use rustc_middle::ty::TyCtxt;
use rustc_span::Symbol;

use super::body::BodyStats;
use super::BodyStats;
use super::PlaceMagic;

use {rustc_borrowck as borrowck, rustc_hir as hir};
Expand Down
6 changes: 5 additions & 1 deletion clippy_lints/src/borrow_pats/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
//! - MIR can violate rust semantics:
//! - `(_1 = &(*_2))`
//! - Two Phased Borrows
//! - Analysis only tracks named values. Anonymous values are generated by Rustc but might change
//! and are also MIR version specific.
//!
//! # Optional and good todos:
//! - Investigate the `explicit_outlives_requirements` lint
Expand Down Expand Up @@ -62,13 +64,15 @@ use {rustc_borrowck as borrowck, rustc_hir as hir, rustc_middle as mid};
mod body;
mod owned;

mod stats;
mod info;
mod prelude;
mod rustc_extention;
mod util;
pub use info::*;
pub use rustc_extention::*;
pub use util::*;
pub use stats::*;

const RETURN: Local = Local::from_u32(0);

Expand Down Expand Up @@ -166,7 +170,7 @@ impl BorrowPats {

if self.print_call_relations {
println!("# Relations for {body_name:?}");
println!("- Self relations: {:#?}", info.stats.borrow());
println!("- Incompltete Stats: {:#?}", info.stats.borrow());
println!("- Called function relations: {:#?}", info.terms);
println!("- Body: {} {:?}", body_info, body_pats);
println!();
Expand Down
9 changes: 8 additions & 1 deletion clippy_lints/src/borrow_pats/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ pub enum OwnedPat {
/// as the value is otherwise still accessible.
ModMutShadowUnmut,
/// A loan of this value was assigned to a named place
NamedLoan,
NamedBorrow,
NamedBorrowMut,
}

impl<'a, 'tcx> MyVisitor<'tcx> for OwnedAnalysis<'a, 'tcx> {
Expand Down Expand Up @@ -521,17 +522,23 @@ impl<'a, 'tcx> OwnedAnalysis<'a, 'tcx> {
immut_bros.push(place);

if matches!(bro_kind, BroKind::Anon) {
let stats = &mut self.info.stats.borrow_mut().owned;
stats.temp_borrow_count += 1;
self.pats.insert(OwnedPat::TempBorrow);
if loan_extended {
stats.temp_borrow_extended_count += 1;
self.pats.insert(OwnedPat::TempBorrowExtended);
}
}
},
Mutability::Mut => {
mut_bro_ctn += 1;
if matches!(bro_kind, BroKind::Anon) {
let stats = &mut self.info.stats.borrow_mut().owned;
stats.temp_borrow_mut_count += 1;
self.pats.insert(OwnedPat::TempBorrowMut);
if loan_extended {
stats.temp_borrow_mut_extended_count += 1;
self.pats.insert(OwnedPat::TempBorrowMutExtended);
}
}
Expand Down
22 changes: 17 additions & 5 deletions clippy_lints/src/borrow_pats/owned/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,18 @@ impl<'tcx> StateInfo<'tcx> {
.any(|(_loc, phase_place)| info.places_conflict(*phase_place, broker))
{
pats.insert(OwnedPat::TwoPhasedBorrow);
info.stats.borrow_mut().two_phase_borrows += 1;
info.stats.borrow_mut().owned.two_phased_borrows += 1;
}

let is_named = matches!(info.locals[&borrow.local].kind, LocalKind::UserVar(..));
if is_named {
pats.insert(OwnedPat::NamedLoan);
if matches!(kind, BorrowKind::Shared) {
info.stats.borrow_mut().owned.named_borrow_count += 1;
pats.insert(OwnedPat::NamedBorrow);
} else {
info.stats.borrow_mut().owned.named_borrow_mut_count += 1;
pats.insert(OwnedPat::NamedBorrowMut);
}
}

let bro_kind = if let Some(bro_kind) = bro_kind {
Expand Down Expand Up @@ -217,8 +223,14 @@ impl<'tcx> StateInfo<'tcx> {
BroKind::Anon => {
let is_named = matches!(info.locals[&dst.local].kind, LocalKind::UserVar(..));
if is_named {
pats.insert(OwnedPat::NamedLoan);
};
if matches!(new_muta, Mutability::Mut) {
info.stats.borrow_mut().owned.named_borrow_mut_count += 1;
pats.insert(OwnedPat::NamedBorrowMut);
} else {
info.stats.borrow_mut().owned.named_borrow_count += 1;
pats.insert(OwnedPat::NamedBorrow);
}
}

let new_bro_kind = if is_named { BroKind::Named } else { BroKind::Anon };

Expand Down
81 changes: 81 additions & 0 deletions clippy_lints/src/borrow_pats/stats.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@

/// Most of these statistics need to be filled by the individual analysis passed.
/// Every value should document which pass might modify/fill it.
///
/// Without more context and tracking the data flow, it's impossible to know what
/// certain instructions are.
///
/// For example, a named borrow can have different shapes. Assuming `_1` is the
/// owned value and `_2` is the named references, they could have the following
/// shapes:
///
/// ```
/// // Direct
/// _2 = &_1
///
/// // Indirect
/// _3 = &_1
/// _2 = &(*_3)
///
/// // Indirect + Copy
/// _3 = &_1
/// _4 = &(*_3)
/// _2 = move _4
/// ```
#[derive(Debug, Clone, Default)]
pub struct BodyStats {
/// Number of relations between the arguments and the return value accoring
/// to the function signature
///
/// Filled by `BodyAnalysis`
pub return_relations_signature: usize,
/// Number of relations between the arguments and the return value that have
/// been found inside the body
///
/// Filled by `BodyAnalysis`
pub return_relations_found: usize,
/// Number of relations between arguments according to the signature
///
/// Filled by `BodyAnalysis`
pub arg_relations_signature: usize,
/// Number of relations between arguments that have been found in the body
///
/// Filled by `BodyAnalysis`
pub arg_relations_found: usize,

/// Stats about named owned values
pub owned: OwnedStats,
}

/// Stats for owned variables
///
/// All of these are collected by the `OwnedAnalysis`
#[derive(Debug, Clone, Default)]
pub struct OwnedStats {
/// Temp borrows are used for function calls.
///
/// The MIR commonly looks like this:
/// ```
/// _3 = &_1
/// _4 = &(*_3)
/// _2 = function(move _4)
/// ```
pub temp_borrow_count: usize,
pub temp_borrow_mut_count: usize,
/// Temporary borrows might be extended if the returned value depends on the input.
///
/// The temporary borrows are also added to the trackers above.
pub temp_borrow_extended_count: usize,
pub temp_borrow_mut_extended_count: usize,
/// A loan was created and stored to a named place.
///
/// See comment of [`BodyStats`] for ways this might be expressed in MIR.
pub named_borrow_count: usize,
pub named_borrow_mut_count: usize,
/// These are collected by the `OwnedAnalysis`
///
/// Note:
/// - This only counts the confirmed two phased borrows.
/// - The borrows that produce the two phased borrow are also counted above.
pub two_phased_borrows: usize
}
18 changes: 11 additions & 7 deletions tests/ui/thesis/nilqam_patterns.stdout
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
# "mut_named_ref_non_kill"
- x : (Mutable , Owned , Local , Copy , NonDrop, Primitive) {NamedLoan}
- y : (Mutable , Owned , Local , Copy , NonDrop, Primitive) {NamedLoan}
- x : (Mutable , Owned , Local , Copy , NonDrop, Primitive) {NamedBorrow}
- y : (Mutable , Owned , Local , Copy , NonDrop, Primitive) {NamedBorrow}
- Body: Output(Unit) , NotConst , Safe , Sync , Free {NoArguments, HasAnonBorrow}
- Stats: BodyStats {
return_relations_signature: 0,
return_relations_found: 0,
arg_relations_signature: 0,
arg_relations_found: 0,
anon_borrow_count: 2,
anon_borrow_count_mut: 0,
named_borrow_count: 0,
named_borrow_count_mut: 0,
two_phase_borrows: 0,
owned: OwnedStats {
temp_borrow_count: 0,
temp_borrow_mut_count: 0,
temp_borrow_extended_count: 0,
temp_borrow_mut_extended_count: 0,
named_borrow_count: 2,
named_borrow_mut_count: 0,
two_phased_borrows: 0,
},
}

Loading

0 comments on commit 3251b8f

Please sign in to comment.