Skip to content

Commit 8663c6f

Browse files
committed
Thesis: Humble amount of squashed commits
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
1 parent 2800251 commit 8663c6f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+11218
-6
lines changed

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ rustc-ice-*
44
# Used by CI to be able to push:
55
/.github/deploy_key
66
out
7+
output.txt
8+
rustc-ice*
79

810
# Compiled files
911
*.o

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5171,6 +5171,7 @@ Released 2018-09-13
51715171
[`borrow_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr
51725172
[`borrow_deref_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_deref_ref
51735173
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
5174+
[`borrow_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_pats
51745175
[`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box
51755176
[`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection
51765177
[`box_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_default

clippy_lints/Cargo.toml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ declare_clippy_lint = { path = "../declare_clippy_lint" }
1717
itertools = "0.12"
1818
quine-mc_cluskey = "0.2"
1919
regex-syntax = "0.8"
20-
serde = { version = "1.0", features = ["derive"] }
21-
serde_json = { version = "1.0", optional = true }
20+
serde = { version = "1.0", features = ["derive", "std"] }
21+
serde_json = { version = "1.0" }
2222
tempfile = { version = "3.3.0", optional = true }
2323
toml = "0.7.3"
2424
regex = { version = "1.5", optional = true }
@@ -27,14 +27,15 @@ unicode-script = { version = "0.5", default-features = false }
2727
semver = "1.0"
2828
rustc-semver = "1.1"
2929
url = "2.2"
30+
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
3031

3132
[dev-dependencies]
3233
walkdir = "2.3"
3334

3435
[features]
3536
deny-warnings = ["clippy_config/deny-warnings", "clippy_utils/deny-warnings"]
3637
# build clippy with internal lints enabled, off by default
37-
internal = ["serde_json", "tempfile", "regex"]
38+
internal = ["tempfile", "regex"]
3839

3940
[package.metadata.rust-analyzer]
4041
# This crate uses #[feature(rustc_private)]

clippy_lints/src/borrow_pats/body.rs

Lines changed: 294 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,294 @@
1+
//! This module analyzes the relationship between the function signature and
2+
//! the internal dataflow. Specifically, it checks for the following things:
3+
//!
4+
//! - Might an owned argument be returned
5+
//! - Are arguments stored in `&mut` loans
6+
//! - Are dependent loans returned
7+
//! - Might a returned loan be `'static`
8+
//! - Are all returned values const
9+
//! - Is the unit type returned
10+
//! - How often do `&mut self` refs need to be `&mut`
11+
//! - Are all the dependencies from the function signature used.
12+
13+
#![warn(unused)]
14+
15+
use super::prelude::*;
16+
use super::{calc_fn_arg_relations, has_mut_ref, visit_body, BodyStats};
17+
18+
use clippy_utils::ty::for_each_region;
19+
20+
mod pattern;
21+
pub use pattern::*;
22+
mod flow;
23+
use flow::DfWalker;
24+
use rustc_middle::ty::Region;
25+
26+
#[derive(Debug)]
27+
pub struct BodyAnalysis<'a, 'tcx> {
28+
info: &'a AnalysisInfo<'tcx>,
29+
pats: BTreeSet<BodyPat>,
30+
data_flow: IndexVec<Local, SmallVec<[MutInfo; 2]>>,
31+
stats: BodyStats,
32+
}
33+
34+
/// This indicates an assignment to `to`. In most cases, there is also a `from`.
35+
#[derive(Debug, Clone)]
36+
enum MutInfo {
37+
/// A different place was copied or moved into this one
38+
Place(Local),
39+
Const,
40+
Arg,
41+
Calc,
42+
/// This is typical for loans and function calls.
43+
Dep(Vec<Local>),
44+
/// A value was constructed from this data
45+
Ctor(Vec<Local>),
46+
/// This is not an assignment, but the notification that a mut borrow was created
47+
Loan(Local),
48+
MutRef(Local),
49+
}
50+
51+
impl<'a, 'tcx> BodyAnalysis<'a, 'tcx> {
52+
fn new(info: &'a AnalysisInfo<'tcx>, arg_ctn: usize) -> Self {
53+
let mut data_flow: IndexVec<Local, SmallVec<[MutInfo; 2]>> = IndexVec::default();
54+
data_flow.resize(info.locals.len(), SmallVec::default());
55+
56+
(0..arg_ctn).for_each(|idx| data_flow[Local::from_usize(idx + 1)].push(MutInfo::Arg));
57+
58+
let mut pats = BTreeSet::default();
59+
if arg_ctn == 0 {
60+
pats.insert(BodyPat::NoArguments);
61+
}
62+
63+
Self {
64+
info,
65+
pats,
66+
data_flow,
67+
stats: Default::default(),
68+
}
69+
}
70+
71+
pub fn run(
72+
info: &'a AnalysisInfo<'tcx>,
73+
def_id: LocalDefId,
74+
hir_sig: &rustc_hir::FnSig<'_>,
75+
context: BodyContext,
76+
) -> (BodyInfo, BTreeSet<BodyPat>) {
77+
let mut anly = Self::new(info, hir_sig.decl.inputs.len());
78+
79+
visit_body(&mut anly, info);
80+
anly.check_fn_relations(def_id);
81+
82+
let body_info = BodyInfo::from_sig(hir_sig, info, context);
83+
84+
anly.stats.arg_relation_possibly_missed_due_generics =
85+
info.stats.borrow().arg_relation_possibly_missed_due_generics;
86+
anly.stats.arg_relation_possibly_missed_due_to_late_bounds =
87+
info.stats.borrow().arg_relation_possibly_missed_due_to_late_bounds;
88+
info.stats.replace(anly.stats);
89+
(body_info, anly.pats)
90+
}
91+
92+
fn check_fn_relations(&mut self, def_id: LocalDefId) {
93+
let mut rels = calc_fn_arg_relations(self.info.cx.tcx, def_id);
94+
let return_rels = rels.remove(&RETURN_LOCAL).unwrap_or_default();
95+
96+
// Argument relations
97+
for (child, maybe_parents) in &rels {
98+
self.check_arg_relation(*child, maybe_parents);
99+
}
100+
101+
self.check_return_relations(&return_rels, def_id);
102+
}
103+
104+
fn check_return_relations(&mut self, sig_parents: &[Local], def_id: LocalDefId) {
105+
self.stats.return_relations_signature = sig_parents.len();
106+
107+
let arg_ctn = self.info.body.arg_count;
108+
let args: Vec<_> = (0..arg_ctn).map(|i| Local::from(i + 1)).collect();
109+
110+
let mut checker = DfWalker::new(self.info, &self.data_flow, RETURN_LOCAL, &args);
111+
checker.walk();
112+
113+
for arg in &args {
114+
if checker.found_connection(*arg) {
115+
// These two branches are mutually exclusive:
116+
if sig_parents.contains(arg) {
117+
self.stats.return_relations_found += 1;
118+
}
119+
// FIXME: It would be nice if we can say, if an argument was
120+
// returned, but it feels like all we can say is that there is an connection between
121+
// this and the other thing else if !self.info.body.local_decls[*
122+
// arg].ty.is_ref() { println!("Track owned argument returned");
123+
// }
124+
}
125+
}
126+
127+
// check for static returns
128+
let mut all_regions_static = true;
129+
let mut region_count = 0;
130+
let fn_sig = self.info.cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder();
131+
for_each_region(fn_sig.output(), &mut |region: Region<'_>| {
132+
region_count += 1;
133+
all_regions_static &= region.is_static();
134+
});
135+
136+
// Check if there can be static returns
137+
if region_count >= 1 && !all_regions_static {
138+
let mut pending_return_df = std::mem::take(&mut self.data_flow[RETURN_LOCAL]);
139+
let mut checked_return_df = SmallVec::with_capacity(pending_return_df.len());
140+
// We check for every assignment, if it's constant and therefore static
141+
while let Some(return_df) = pending_return_df.pop() {
142+
self.data_flow[RETURN_LOCAL].push(return_df);
143+
144+
let mut checker = DfWalker::new(self.info, &self.data_flow, RETURN_LOCAL, &args);
145+
checker.walk();
146+
let all_const = checker.all_const();
147+
148+
checked_return_df.push(self.data_flow[RETURN_LOCAL].pop().unwrap());
149+
150+
if all_const {
151+
self.pats.insert(BodyPat::ReturnedStaticLoanForNonStatic);
152+
break;
153+
}
154+
}
155+
156+
checked_return_df.append(&mut pending_return_df);
157+
self.data_flow[RETURN_LOCAL] = checked_return_df;
158+
}
159+
}
160+
161+
fn check_arg_relation(&mut self, child: Local, maybe_parents: &[Local]) {
162+
let mut checker = DfWalker::new(self.info, &self.data_flow, child, maybe_parents);
163+
checker.walk();
164+
165+
self.stats.arg_relations_signature += maybe_parents.len();
166+
self.stats.arg_relations_found += checker.connection_count();
167+
}
168+
}
169+
170+
impl<'a, 'tcx> Visitor<'tcx> for BodyAnalysis<'a, 'tcx> {
171+
fn visit_assign(&mut self, target: &Place<'tcx>, rval: &Rvalue<'tcx>, _loc: mir::Location) {
172+
match rval {
173+
Rvalue::Ref(_reg, BorrowKind::Fake, _src) => {
174+
#[allow(clippy::needless_return)]
175+
return;
176+
},
177+
Rvalue::Ref(_reg, kind, src) => {
178+
self.stats.ref_stmt_ctn += 1;
179+
180+
let is_mut = matches!(kind, BorrowKind::Mut { .. });
181+
if is_mut {
182+
self.data_flow[src.local].push(MutInfo::MutRef(target.local));
183+
}
184+
if matches!(src.projection.as_slice(), [mir::PlaceElem::Deref]) {
185+
// &(*_1) => Copy
186+
self.data_flow[target.local].push(MutInfo::Place(src.local));
187+
return;
188+
}
189+
190+
// _1 = &_2 => simple loan
191+
self.data_flow[target.local].push(MutInfo::Loan(src.local));
192+
},
193+
Rvalue::Cast(_, op, _) | Rvalue::Use(op) => {
194+
let event = match &op {
195+
Operand::Constant(_) => MutInfo::Const,
196+
Operand::Copy(from) | Operand::Move(from) => MutInfo::Place(from.local),
197+
};
198+
self.data_flow[target.local].push(event);
199+
},
200+
Rvalue::CopyForDeref(from) => {
201+
self.data_flow[target.local].push(MutInfo::Place(from.local));
202+
},
203+
Rvalue::Repeat(op, _) => {
204+
let event = match &op {
205+
Operand::Constant(_) => MutInfo::Const,
206+
Operand::Copy(from) | Operand::Move(from) => MutInfo::Ctor(vec![from.local]),
207+
};
208+
self.data_flow[target.local].push(event);
209+
},
210+
// Constructed Values
211+
Rvalue::Aggregate(_, fields) => {
212+
let args = fields
213+
.iter()
214+
.filter_map(rustc_middle::mir::Operand::place)
215+
.map(|place| place.local)
216+
.collect();
217+
self.data_flow[target.local].push(MutInfo::Ctor(args));
218+
},
219+
// Casts should depend on the input data
220+
Rvalue::ThreadLocalRef(_)
221+
| Rvalue::NullaryOp(_, _)
222+
| Rvalue::AddressOf(_, _)
223+
| Rvalue::Discriminant(_)
224+
| Rvalue::ShallowInitBox(_, _)
225+
| Rvalue::Len(_)
226+
| Rvalue::BinaryOp(_, _)
227+
| Rvalue::UnaryOp(_, _)
228+
| Rvalue::CheckedBinaryOp(_, _) => {
229+
self.data_flow[target.local].push(MutInfo::Calc);
230+
},
231+
}
232+
}
233+
234+
fn visit_terminator(&mut self, term: &Terminator<'tcx>, loc: Location) {
235+
let TerminatorKind::Call {
236+
destination: dest,
237+
args,
238+
..
239+
} = &term.kind
240+
else {
241+
return;
242+
};
243+
244+
for arg in args {
245+
if let Some(place) = arg.node.place() {
246+
let ty = self.info.body.local_decls[place.local].ty;
247+
if has_mut_ref(ty) {
248+
self.data_flow[place.local].push(MutInfo::Calc);
249+
}
250+
}
251+
}
252+
253+
assert!(dest.just_local());
254+
self.data_flow[dest.local].push(MutInfo::Calc);
255+
256+
let rels = &self.info.terms[&loc.block];
257+
for (target, sources) in rels {
258+
self.data_flow[*target].push(MutInfo::Dep(sources.clone()));
259+
}
260+
}
261+
}
262+
263+
pub(crate) fn update_pats_from_stats(pats: &mut BTreeSet<BodyPat>, info: &AnalysisInfo<'_>) {
264+
let stats = info.stats.borrow();
265+
266+
if stats.ref_stmt_ctn > 0 {
267+
pats.insert(BodyPat::Borrow);
268+
}
269+
270+
if stats.owned.named_borrow_count > 0 {
271+
pats.insert(BodyPat::OwnedNamedBorrow);
272+
}
273+
if stats.owned.named_borrow_mut_count > 0 {
274+
pats.insert(BodyPat::OwnedNamedBorrowMut);
275+
}
276+
277+
if stats.owned.arg_borrow_count > 0 {
278+
pats.insert(BodyPat::OwnedArgBorrow);
279+
}
280+
if stats.owned.arg_borrow_mut_count > 0 {
281+
pats.insert(BodyPat::OwnedArgBorrowMut);
282+
}
283+
284+
if stats.owned.two_phased_borrows > 0 {
285+
pats.insert(BodyPat::OwnedTwoPhaseBorrow);
286+
}
287+
288+
if stats.owned.borrowed_for_closure_count > 0 {
289+
pats.insert(BodyPat::OwnedClosureBorrow);
290+
}
291+
if stats.owned.borrowed_mut_for_closure_count > 0 {
292+
pats.insert(BodyPat::OwnedClosureBorrowMut);
293+
}
294+
}

0 commit comments

Comments
 (0)