[EXPERIMENT] Ownership analysis [DO NOT MERGE]

Cargo.lock

@@ -649,6 +649,7 @@ dependencies = [
  "semver",
  "serde",
  "serde_json",
+ "smallvec",
  "tempfile",
  "toml 0.7.8",
  "unicode-normalization",

src/tools/clippy/.gitignore

@@ -4,6 +4,8 @@ rustc-ice-*
 # Used by CI to be able to push:
 /.github/deploy_key
 out
+output.txt
+rustc-ice*
 
 # Compiled files
 *.o

src/tools/clippy/CHANGELOG.md

@@ -5171,6 +5171,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

src/tools/clippy/clippy_lints/Cargo.toml

@@ -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 }
@@ -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)]

src/tools/clippy/clippy_lints/src/assigning_clones.rs

@@ -2,9 +2,9 @@ use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::macros::HirNode;
 use clippy_utils::sugg::Sugg;
-use clippy_utils::{is_trait_method, path_to_local};
+use clippy_utils::{is_trait_method, local_is_initialized, path_to_local};
 use rustc_errors::Applicability;
-use rustc_hir::{self as hir, Expr, ExprKind, Node};
+use rustc_hir::{self as hir, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::{self, Instance, Mutability};
 use rustc_session::impl_lint_pass;
@@ -36,6 +36,7 @@ declare_clippy_lint! {
     /// Use instead:
     /// ```rust
     /// struct Thing;
+    ///
     /// impl Clone for Thing {
     ///     fn clone(&self) -> Self { todo!() }
     ///     fn clone_from(&mut self, other: &Self) { todo!() }
@@ -163,9 +164,7 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC
         // TODO: This check currently bails if the local variable has no initializer.
         // That is overly conservative - the lint should fire even if there was no initializer,
         // but the variable has been initialized before `lhs` was evaluated.
-        if let Some(Node::LetStmt(local)) = cx.tcx.hir().parent_id_iter(local).next().map(|p| cx.tcx.hir_node(p))
-            && local.init.is_none()
-        {
+        if !local_is_initialized(cx, local) {
             return false;
         }
     }

src/tools/clippy/clippy_lints/src/borrow_pats/body.rs

@@ -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);
+    }
+}