Skip to content

Commit

Permalink
Thesis: Humble amount of squashed commits
Browse files Browse the repository at this point in the history
More testing or something

Duck

Checking more bbs

Checking more bbs

Some progress IIRC

Handle more teminators

Diving into RValue evaluation

I have no idea what I'm doing

It doesn't crash I guess?

Finding the first pattern

Refactoring: Extract pattern from automata to simplify states

Remove old value usage based pattern detection

Refactoring: Better Automata structure

More automator nodes :D

More states and small progress

Detecting conditional assignments

Event posting refactoring

Adding test output and testing temp borrows

Working on field borrows and cleanup :D

Different errors yay

Use `mir::Place` for events

More patterns and more states, just need some pats :3

Getting the first fricking region

Collect dependent types for return tys

Somehow handle deref calls 😭

Cleanup AnonRef state thing by using an info struct wrapper

General Cleanup

Working the return state machine

Testing borrows in HIR format...

Working on something I guess

Having a direction to run against walls

Control flow information :D

CFG with ranges..

CFG: resonable downgrade

CFG: Start

Caching return Relations

I love refactoring in Rust :D

Detecting first returns

Handling more returns

Tracking assignments better

Collecting local information

Refactor metadata collection for MIR

The first return patterns

Cleanup console output with envs

Fix `LocalInfo` for arguments

Some refactorings again

Check unused for owned values

Complaining about MIR again

Why is there a no-op Drop????

Determine a nice traversal order

Probably progress??

Detecting manual drops and moves into functions

Fix ICE for item names

Remove old automata based implementation

Dogfood my code (52 Errors...)

Detecting temp borrows

Abort on feature use

Handling assignments better

Unify arguments and variables

Remove RC from ownership patterns

Better anon ref tracking

Stash that thing

Better function call relation calculation

Track relations between mut inputs as well :D

Correct `has_projection()` usage

Detecting some patterns in function params

Detecting two phase borrows

Owned `StateInfo` refactoring

Update todos

Generalize visitors for analyzers

Tracking all assignments

Dataflow analysis

Dataflow analysis with `&mut` loans

Dataflow refactoring, this only considers locals

Owned: Detect `ModMutShadowUnmut`

Reorganize UI tests

Include more type information in `VarInfo`

Better track return types

Refactorings for borrow tracking

Allow nested two phase borrows

Identify named borrows for owned values better

Correct owned value stat tracking

Fill body pats based on stats

Pause `Return` pattern for owned values

Modeling the state as a list

Detecting the first patterns with state merging

Detecting conditional moves and drops

Mark more tests as passing

Better detect overwrites for owned values

Improve Drop impl info

Remove `Unused` detection due to inconsistency

Update docs and move test to pass

Refactored `state.borrows`

Handle double refs correctly

Dealing with ref-refs and renaming temp to arg borows

Add `Moved` as a base pattern

Handle part moves

Better separation between borrow and part borrows

Track possibly missed relations

Closures

Add simple closure ty

Snapshot

Ctor things and stuff

Remove lint level gate

Avoiding `rand` crashes 🎉

Aggregate stats for bodies

Jsonfy analysis output

Update todos

Improving traversal construction and running regex :D

Type cleanupg

Feeding the dog
  • Loading branch information
xFrednet committed May 5, 2024
1 parent e485a02 commit 31ff2b0
Show file tree
Hide file tree
Showing 58 changed files with 11,218 additions and 6 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Used by CI to be able to push:
/.github/deploy_key
out
output.txt
rustc-ice*

# Compiled files
*.o
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5066,6 +5066,7 @@ Released 2018-09-13
[`borrow_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr
[`borrow_deref_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_deref_ref
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
[`borrow_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_pats
[`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box
[`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection
[`box_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_default
Expand Down
7 changes: 4 additions & 3 deletions clippy_lints/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ declare_clippy_lint = { path = "../declare_clippy_lint" }
itertools = "0.12"
quine-mc_cluskey = "0.2"
regex-syntax = "0.8"
serde = { version = "1.0", features = ["derive"] }
serde_json = { version = "1.0", optional = true }
serde = { version = "1.0", features = ["derive", "std"] }
serde_json = { version = "1.0" }
tempfile = { version = "3.3.0", optional = true }
toml = "0.7.3"
regex = { version = "1.5", optional = true }
Expand All @@ -27,14 +27,15 @@ unicode-script = { version = "0.5", default-features = false }
semver = "1.0"
rustc-semver = "1.1"
url = "2.2"
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }

[dev-dependencies]
walkdir = "2.3"

[features]
deny-warnings = ["clippy_config/deny-warnings", "clippy_utils/deny-warnings"]
# build clippy with internal lints enabled, off by default
internal = ["serde_json", "tempfile", "regex"]
internal = ["tempfile", "regex"]

[package.metadata.rust-analyzer]
# This crate uses #[feature(rustc_private)]
Expand Down
294 changes: 294 additions & 0 deletions clippy_lints/src/borrow_pats/body.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,294 @@
//! This module analyzes the relationship between the function signature and
//! the internal dataflow. Specifically, it checks for the following things:
//!
//! - Might an owned argument be returned
//! - Are arguments stored in `&mut` loans
//! - Are dependent loans returned
//! - Might a returned loan be `'static`
//! - Are all returned values const
//! - Is the unit type returned
//! - How often do `&mut self` refs need to be `&mut`
//! - Are all the dependencies from the function signature used.
#![warn(unused)]

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

use clippy_utils::ty::for_each_region;

mod pattern;
pub use pattern::*;
mod flow;
use flow::DfWalker;
use rustc_middle::ty::Region;

#[derive(Debug)]
pub struct BodyAnalysis<'a, 'tcx> {
info: &'a AnalysisInfo<'tcx>,
pats: BTreeSet<BodyPat>,
data_flow: IndexVec<Local, SmallVec<[MutInfo; 2]>>,
stats: BodyStats,
}

/// This indicates an assignment to `to`. In most cases, there is also a `from`.
#[derive(Debug, Clone)]
enum MutInfo {
/// A different place was copied or moved into this one
Place(Local),
Const,
Arg,
Calc,
/// This is typical for loans and function calls.
Dep(Vec<Local>),
/// A value was constructed from this data
Ctor(Vec<Local>),
/// This is not an assignment, but the notification that a mut borrow was created
Loan(Local),
MutRef(Local),
}

impl<'a, 'tcx> BodyAnalysis<'a, 'tcx> {
fn new(info: &'a AnalysisInfo<'tcx>, arg_ctn: usize) -> Self {
let mut data_flow: IndexVec<Local, SmallVec<[MutInfo; 2]>> = IndexVec::default();
data_flow.resize(info.locals.len(), SmallVec::default());

(0..arg_ctn).for_each(|idx| data_flow[Local::from_usize(idx + 1)].push(MutInfo::Arg));

let mut pats = BTreeSet::default();
if arg_ctn == 0 {
pats.insert(BodyPat::NoArguments);
}

Self {
info,
pats,
data_flow,
stats: Default::default(),
}
}

pub fn run(
info: &'a AnalysisInfo<'tcx>,
def_id: LocalDefId,
hir_sig: &rustc_hir::FnSig<'_>,
context: BodyContext,
) -> (BodyInfo, BTreeSet<BodyPat>) {
let mut anly = Self::new(info, hir_sig.decl.inputs.len());

visit_body(&mut anly, info);
anly.check_fn_relations(def_id);

let body_info = BodyInfo::from_sig(hir_sig, info, context);

anly.stats.arg_relation_possibly_missed_due_generics =
info.stats.borrow().arg_relation_possibly_missed_due_generics;
anly.stats.arg_relation_possibly_missed_due_to_late_bounds =
info.stats.borrow().arg_relation_possibly_missed_due_to_late_bounds;
info.stats.replace(anly.stats);
(body_info, anly.pats)
}

fn check_fn_relations(&mut self, def_id: LocalDefId) {
let mut rels = calc_fn_arg_relations(self.info.cx.tcx, def_id);
let return_rels = rels.remove(&RETURN_LOCAL).unwrap_or_default();

// Argument relations
for (child, maybe_parents) in &rels {
self.check_arg_relation(*child, maybe_parents);
}

self.check_return_relations(&return_rels, def_id);
}

fn check_return_relations(&mut self, sig_parents: &[Local], def_id: LocalDefId) {
self.stats.return_relations_signature = sig_parents.len();

let arg_ctn = self.info.body.arg_count;
let args: Vec<_> = (0..arg_ctn).map(|i| Local::from(i + 1)).collect();

let mut checker = DfWalker::new(self.info, &self.data_flow, RETURN_LOCAL, &args);
checker.walk();

for arg in &args {
if checker.found_connection(*arg) {
// These two branches are mutually exclusive:
if sig_parents.contains(arg) {
self.stats.return_relations_found += 1;
}
// FIXME: It would be nice if we can say, if an argument was
// returned, but it feels like all we can say is that there is an connection between
// this and the other thing else if !self.info.body.local_decls[*
// arg].ty.is_ref() { println!("Track owned argument returned");
// }
}
}

// check for static returns
let mut all_regions_static = true;
let mut region_count = 0;
let fn_sig = self.info.cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder();
for_each_region(fn_sig.output(), &mut |region: Region<'_>| {
region_count += 1;
all_regions_static &= region.is_static();
});

// Check if there can be static returns
if region_count >= 1 && !all_regions_static {
let mut pending_return_df = std::mem::take(&mut self.data_flow[RETURN_LOCAL]);
let mut checked_return_df = SmallVec::with_capacity(pending_return_df.len());
// We check for every assignment, if it's constant and therefore static
while let Some(return_df) = pending_return_df.pop() {
self.data_flow[RETURN_LOCAL].push(return_df);

let mut checker = DfWalker::new(self.info, &self.data_flow, RETURN_LOCAL, &args);
checker.walk();
let all_const = checker.all_const();

checked_return_df.push(self.data_flow[RETURN_LOCAL].pop().unwrap());

if all_const {
self.pats.insert(BodyPat::ReturnedStaticLoanForNonStatic);
break;
}
}

checked_return_df.append(&mut pending_return_df);
self.data_flow[RETURN_LOCAL] = checked_return_df;
}
}

fn check_arg_relation(&mut self, child: Local, maybe_parents: &[Local]) {
let mut checker = DfWalker::new(self.info, &self.data_flow, child, maybe_parents);
checker.walk();

self.stats.arg_relations_signature += maybe_parents.len();
self.stats.arg_relations_found += checker.connection_count();
}
}

impl<'a, 'tcx> Visitor<'tcx> for BodyAnalysis<'a, 'tcx> {
fn visit_assign(&mut self, target: &Place<'tcx>, rval: &Rvalue<'tcx>, _loc: mir::Location) {
match rval {
Rvalue::Ref(_reg, BorrowKind::Fake, _src) => {
#[allow(clippy::needless_return)]
return;
},
Rvalue::Ref(_reg, kind, src) => {
self.stats.ref_stmt_ctn += 1;

let is_mut = matches!(kind, BorrowKind::Mut { .. });
if is_mut {
self.data_flow[src.local].push(MutInfo::MutRef(target.local));
}
if matches!(src.projection.as_slice(), [mir::PlaceElem::Deref]) {
// &(*_1) => Copy
self.data_flow[target.local].push(MutInfo::Place(src.local));
return;
}

// _1 = &_2 => simple loan
self.data_flow[target.local].push(MutInfo::Loan(src.local));
},
Rvalue::Cast(_, op, _) | Rvalue::Use(op) => {
let event = match &op {
Operand::Constant(_) => MutInfo::Const,
Operand::Copy(from) | Operand::Move(from) => MutInfo::Place(from.local),
};
self.data_flow[target.local].push(event);
},
Rvalue::CopyForDeref(from) => {
self.data_flow[target.local].push(MutInfo::Place(from.local));
},
Rvalue::Repeat(op, _) => {
let event = match &op {
Operand::Constant(_) => MutInfo::Const,
Operand::Copy(from) | Operand::Move(from) => MutInfo::Ctor(vec![from.local]),
};
self.data_flow[target.local].push(event);
},
// Constructed Values
Rvalue::Aggregate(_, fields) => {
let args = fields
.iter()
.filter_map(rustc_middle::mir::Operand::place)
.map(|place| place.local)
.collect();
self.data_flow[target.local].push(MutInfo::Ctor(args));
},
// Casts should depend on the input data
Rvalue::ThreadLocalRef(_)
| Rvalue::NullaryOp(_, _)
| Rvalue::AddressOf(_, _)
| Rvalue::Discriminant(_)
| Rvalue::ShallowInitBox(_, _)
| Rvalue::Len(_)
| Rvalue::BinaryOp(_, _)
| Rvalue::UnaryOp(_, _)
| Rvalue::CheckedBinaryOp(_, _) => {
self.data_flow[target.local].push(MutInfo::Calc);
},
}
}

fn visit_terminator(&mut self, term: &Terminator<'tcx>, loc: Location) {
let TerminatorKind::Call {
destination: dest,
args,
..
} = &term.kind
else {
return;
};

for arg in args {
if let Some(place) = arg.node.place() {
let ty = self.info.body.local_decls[place.local].ty;
if has_mut_ref(ty) {
self.data_flow[place.local].push(MutInfo::Calc);
}
}
}

assert!(dest.just_local());
self.data_flow[dest.local].push(MutInfo::Calc);

let rels = &self.info.terms[&loc.block];
for (target, sources) in rels {
self.data_flow[*target].push(MutInfo::Dep(sources.clone()));
}
}
}

pub(crate) fn update_pats_from_stats(pats: &mut BTreeSet<BodyPat>, info: &AnalysisInfo<'_>) {
let stats = info.stats.borrow();

if stats.ref_stmt_ctn > 0 {
pats.insert(BodyPat::Borrow);
}

if stats.owned.named_borrow_count > 0 {
pats.insert(BodyPat::OwnedNamedBorrow);
}
if stats.owned.named_borrow_mut_count > 0 {
pats.insert(BodyPat::OwnedNamedBorrowMut);
}

if stats.owned.arg_borrow_count > 0 {
pats.insert(BodyPat::OwnedArgBorrow);
}
if stats.owned.arg_borrow_mut_count > 0 {
pats.insert(BodyPat::OwnedArgBorrowMut);
}

if stats.owned.two_phased_borrows > 0 {
pats.insert(BodyPat::OwnedTwoPhaseBorrow);
}

if stats.owned.borrowed_for_closure_count > 0 {
pats.insert(BodyPat::OwnedClosureBorrow);
}
if stats.owned.borrowed_mut_for_closure_count > 0 {
pats.insert(BodyPat::OwnedClosureBorrowMut);
}
}
Loading

0 comments on commit 31ff2b0

Please sign in to comment.