]> git.lizzy.rs Git - rust.git/commitdiff
Improve heuristics for determining whether eager of lazy evaluation is preferred
authorJason Newcomb <jsnewcomb@pm.me>
Tue, 17 Aug 2021 20:22:48 +0000 (16:22 -0400)
committerJason Newcomb <jsnewcomb@pm.me>
Tue, 16 Nov 2021 01:54:13 +0000 (20:54 -0500)
17 files changed:
clippy_lints/src/functions/too_many_lines.rs
clippy_lints/src/methods/from_iter_instead_of_collect.rs
clippy_lints/src/methods/or_fun_call.rs
clippy_lints/src/methods/unnecessary_lazy_eval.rs
clippy_lints/src/option_if_let_else.rs
clippy_utils/src/eager_or_lazy.rs
clippy_utils/src/paths.rs
clippy_utils/src/ty.rs
clippy_utils/src/visitors.rs
tests/ui/needless_return.fixed
tests/ui/needless_return.rs
tests/ui/needless_return.stderr
tests/ui/option_if_let_else.fixed
tests/ui/option_if_let_else.stderr
tests/ui/or_fun_call.fixed
tests/ui/or_fun_call.rs
tests/ui/or_fun_call.stderr

index 008ef661b55f20a31c4e25e1e24e3a0de4b7a94a..65efbbab41a460f0ab4f9b2c84a400cbe22efeee 100644 (file)
@@ -56,8 +56,8 @@ pub(super) fn check_fn(
                     continue;
                 }
             } else {
-                let multi_idx = line.find("/*").unwrap_or_else(|| line.len());
-                let single_idx = line.find("//").unwrap_or_else(|| line.len());
+                let multi_idx = line.find("/*").unwrap_or(line.len());
+                let single_idx = line.find("//").unwrap_or(line.len());
                 code_in_line |= multi_idx > 0 && single_idx > 0;
                 // Implies multi_idx is below line.len()
                 if multi_idx < single_idx {
index 99c03844f49275e02461ca8a02c0195c81e96772..8ea9312c0f7075557211e94575f30d946a4fe040 100644 (file)
@@ -69,7 +69,7 @@ fn strip_angle_brackets(s: &str) -> Option<&str> {
                         // i.e.: 2 wildcards in `std::collections::BTreeMap<&i32, &char>`
                         let ty_str = ty.to_string();
                         let start = ty_str.find('<').unwrap_or(0);
-                        let end = ty_str.find('>').unwrap_or_else(|| ty_str.len());
+                        let end = ty_str.find('>').unwrap_or(ty_str.len());
                         let nb_wildcard = ty_str[start..end].split(',').count();
                         let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1));
                         format!("{}<{}>", elements.join("::"), wildcards)
index fe9ffde0d337c1f15506e73bdca1e0f373c88da5..4e4653dadcafcdc1bfe926c2c9cda0a58eea697b 100644 (file)
@@ -1,16 +1,12 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::eager_or_lazy::is_lazyness_candidate;
-use clippy_utils::is_trait_item;
+use clippy_utils::eager_or_lazy::switch_to_lazy_eval;
 use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_macro_callsite};
-use clippy_utils::ty::implements_trait;
-use clippy_utils::ty::{is_type_diagnostic_item, match_type};
-use clippy_utils::{contains_return, last_path_segment, paths};
+use clippy_utils::ty::{implements_trait, match_type};
+use clippy_utils::{contains_return, is_trait_item, last_path_segment, paths};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
-use rustc_hir::{BlockCheckMode, UnsafeSource};
 use rustc_lint::LateContext;
-use rustc_middle::ty;
 use rustc_span::source_map::Span;
 use rustc_span::symbol::{kw, sym};
 use std::borrow::Cow;
@@ -96,25 +92,10 @@ fn check_general_case<'tcx>(
             (&paths::RESULT, true, &["or", "unwrap_or"], "else"),
         ];
 
-        if let hir::ExprKind::MethodCall(path, _, [self_arg, ..], _) = &arg.kind {
-            if path.ident.name == sym::len {
-                let ty = cx.typeck_results().expr_ty(self_arg).peel_refs();
-
-                match ty.kind() {
-                    ty::Slice(_) | ty::Array(_, _) | ty::Str => return,
-                    _ => (),
-                }
-
-                if is_type_diagnostic_item(cx, ty, sym::Vec) {
-                    return;
-                }
-            }
-        }
-
         if_chain! {
             if KNOW_TYPES.iter().any(|k| k.2.contains(&name));
 
-            if is_lazyness_candidate(cx, arg);
+            if switch_to_lazy_eval(cx, arg);
             if !contains_return(arg);
 
             let self_ty = cx.typeck_results().expr_ty(self_expr);
@@ -166,26 +147,30 @@ fn check_general_case<'tcx>(
         }
     }
 
-    if args.len() == 2 {
-        match args[1].kind {
+    if let [self_arg, arg] = args {
+        let inner_arg = if let hir::ExprKind::Block(
+            hir::Block {
+                stmts: [],
+                expr: Some(expr),
+                ..
+            },
+            _,
+        ) = arg.kind
+        {
+            expr
+        } else {
+            arg
+        };
+        match inner_arg.kind {
             hir::ExprKind::Call(fun, or_args) => {
                 let or_has_args = !or_args.is_empty();
-                if !check_unwrap_or_default(cx, name, fun, &args[0], &args[1], or_has_args, expr.span) {
+                if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span) {
                     let fun_span = if or_has_args { None } else { Some(fun.span) };
-                    check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, fun_span);
+                    check_general_case(cx, name, method_span, self_arg, arg, expr.span, fun_span);
                 }
             },
             hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
-                check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, None);
-            },
-            hir::ExprKind::Block(block, _)
-                if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) =>
-            {
-                if let Some(block_expr) = block.expr {
-                    if let hir::ExprKind::MethodCall(..) = block_expr.kind {
-                        check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, None);
-                    }
-                }
+                check_general_case(cx, name, method_span, self_arg, arg, expr.span, None);
             },
             _ => (),
         }
index 740af750b48a7bdaf9fc1a50d47cd1392e21fb1e..1e2765263c87ddb0ed920c8da8a71b74394ee0b3 100644 (file)
@@ -30,7 +30,7 @@ pub(super) fn check<'tcx>(
                 return;
             }
 
-            if eager_or_lazy::is_eagerness_candidate(cx, body_expr) {
+            if eager_or_lazy::switch_to_eager_eval(cx, body_expr) {
                 let msg = if is_option {
                     "unnecessary closure used to substitute value for `Option::None`"
                 } else {
index f7c19baf7f92ecd59d983bd34519e2dc36e54b3e..262be17f61751d495d862f8d628e13c0a0c1ae79 100644 (file)
@@ -147,11 +147,7 @@ fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) ->
             let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" };
             let some_body = extract_body_from_expr(if_then)?;
             let none_body = extract_body_from_expr(if_else)?;
-            let method_sugg = if eager_or_lazy::is_eagerness_candidate(cx, none_body) {
-                "map_or"
-            } else {
-                "map_or_else"
-            };
+            let method_sugg = if eager_or_lazy::switch_to_eager_eval(cx, none_body) { "map_or" } else { "map_or_else" };
             let capture_name = id.name.to_ident_string();
             let (as_ref, as_mut) = match &let_expr.kind {
                 ExprKind::AddrOf(_, Mutability::Not, _) => (true, false),
index 1ea7ccfb75212bb5e9a12b3c04fe514dde7343db..c2645ac730a44322423b24e07a80930c695d50a4 100644 (file)
 //!  - or-fun-call
 //!  - option-if-let-else
 
-use crate::is_ctor_or_promotable_const_function;
-use crate::ty::is_type_diagnostic_item;
+use crate::ty::{all_predicates_of, is_copy};
+use crate::visitors::is_const_evaluatable;
 use rustc_hir::def::{DefKind, Res};
-
-use rustc_hir::intravisit;
-use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
-
-use rustc_hir::{Block, Expr, ExprKind, Path, QPath};
+use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor};
+use rustc_hir::{def_id::DefId, Block, Expr, ExprKind, QPath, UnOp};
 use rustc_lint::LateContext;
-use rustc_middle::hir::map::Map;
-use rustc_span::sym;
-
-/// Is the expr pure (is it free from side-effects)?
-/// This function is named so to stress that it isn't exhaustive and returns FNs.
-fn identify_some_pure_patterns(expr: &Expr<'_>) -> bool {
-    match expr.kind {
-        ExprKind::Lit(..) | ExprKind::ConstBlock(..) | ExprKind::Path(..) | ExprKind::Field(..) => true,
-        ExprKind::AddrOf(_, _, addr_of_expr) => identify_some_pure_patterns(addr_of_expr),
-        ExprKind::Tup(tup_exprs) => tup_exprs.iter().all(identify_some_pure_patterns),
-        ExprKind::Struct(_, fields, expr) => {
-            fields.iter().all(|f| identify_some_pure_patterns(f.expr)) && expr.map_or(true, identify_some_pure_patterns)
-        },
-        ExprKind::Call(
-            &Expr {
-                kind:
-                    ExprKind::Path(QPath::Resolved(
-                        _,
-                        Path {
-                            res: Res::Def(DefKind::Ctor(..) | DefKind::Variant, ..),
-                            ..
-                        },
-                    )),
-                ..
-            },
-            args,
-        ) => args.iter().all(identify_some_pure_patterns),
-        ExprKind::Block(
-            &Block {
-                stmts,
-                expr: Some(expr),
-                ..
-            },
-            _,
-        ) => stmts.is_empty() && identify_some_pure_patterns(expr),
-        ExprKind::Box(..)
-        | ExprKind::Array(..)
-        | ExprKind::Call(..)
-        | ExprKind::MethodCall(..)
-        | ExprKind::Binary(..)
-        | ExprKind::Unary(..)
-        | ExprKind::Let(..)
-        | ExprKind::Cast(..)
-        | ExprKind::Type(..)
-        | ExprKind::DropTemps(..)
-        | ExprKind::Loop(..)
-        | ExprKind::If(..)
-        | ExprKind::Match(..)
-        | ExprKind::Closure(..)
-        | ExprKind::Block(..)
-        | ExprKind::Assign(..)
-        | ExprKind::AssignOp(..)
-        | ExprKind::Index(..)
-        | ExprKind::Break(..)
-        | ExprKind::Continue(..)
-        | ExprKind::Ret(..)
-        | ExprKind::InlineAsm(..)
-        | ExprKind::LlvmInlineAsm(..)
-        | ExprKind::Repeat(..)
-        | ExprKind::Yield(..)
-        | ExprKind::Err => false,
+use rustc_middle::ty::{self, PredicateKind};
+use rustc_span::{sym, Symbol};
+use std::cmp;
+use std::ops;
+
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+enum EagernessSuggestion {
+    // The expression is cheap and should be evaluated eagerly
+    Eager,
+    // The expression may be cheap, so don't suggested lazy evaluation; or the expression may not be safe to switch to
+    // eager evaluation.
+    NoChange,
+    // The expression is likely expensive and should be evaluated lazily.
+    Lazy,
+    // The expression cannot be placed into a closure.
+    ForceNoChange,
+}
+impl ops::BitOr for EagernessSuggestion {
+    type Output = Self;
+    fn bitor(self, rhs: Self) -> Self {
+        cmp::max(self, rhs)
     }
 }
-
-/// Identify some potentially computationally expensive patterns.
-/// This function is named so to stress that its implementation is non-exhaustive.
-/// It returns FNs and FPs.
-fn identify_some_potentially_expensive_patterns<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
-    // Searches an expression for method calls or function calls that aren't ctors
-    struct FunCallFinder<'a, 'tcx> {
-        cx: &'a LateContext<'tcx>,
-        found: bool,
+impl ops::BitOrAssign for EagernessSuggestion {
+    fn bitor_assign(&mut self, rhs: Self) {
+        *self = *self | rhs;
     }
+}
 
-    impl<'a, 'tcx> intravisit::Visitor<'tcx> for FunCallFinder<'a, 'tcx> {
-        type Map = Map<'tcx>;
-
-        fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
-            let call_found = match &expr.kind {
-                // ignore enum and struct constructors
-                ExprKind::Call(..) => !is_ctor_or_promotable_const_function(self.cx, expr),
-                ExprKind::Index(obj, _) => {
-                    let ty = self.cx.typeck_results().expr_ty(obj);
-                    is_type_diagnostic_item(self.cx, ty, sym::HashMap)
-                        || is_type_diagnostic_item(self.cx, ty, sym::BTreeMap)
-                },
-                ExprKind::MethodCall(..) => true,
-                _ => false,
-            };
+/// Determine the eagerness of the given function call.
+fn fn_eagerness(cx: &LateContext<'tcx>, fn_id: DefId, name: Symbol, args: &'tcx [Expr<'_>]) -> EagernessSuggestion {
+    use EagernessSuggestion::{Eager, Lazy, NoChange};
+    let name = &*name.as_str();
 
-            if call_found {
-                self.found |= true;
-            }
+    let ty = match cx.tcx.impl_of_method(fn_id) {
+        Some(id) => cx.tcx.type_of(id),
+        None => return Lazy,
+    };
 
-            if !self.found {
-                intravisit::walk_expr(self, expr);
+    if (name.starts_with("as_") || name == "len" || name == "is_empty") && args.len() == 1 {
+        if matches!(
+            cx.tcx.crate_name(fn_id.krate),
+            sym::std | sym::core | sym::alloc | sym::proc_macro
+        ) {
+            Eager
+        } else {
+            NoChange
+        }
+    } else if let ty::Adt(def, subs) = ty.kind() {
+        // Types where the only fields are generic types (or references to) with no trait bounds other
+        // than marker traits.
+        // Due to the limited operations on these types functions should be fairly cheap.
+        if def
+            .variants
+            .iter()
+            .flat_map(|v| v.fields.iter())
+            .any(|x| matches!(cx.tcx.type_of(x.did).peel_refs().kind(), ty::Param(_)))
+            && all_predicates_of(cx.tcx, fn_id).all(|(pred, _)| match pred.kind().skip_binder() {
+                PredicateKind::Trait(pred) => cx.tcx.trait_def(pred.trait_ref.def_id).is_marker,
+                _ => true,
+            })
+            && subs.types().all(|x| matches!(x.peel_refs().kind(), ty::Param(_)))
+        {
+            // Limit the function to either `(self) -> bool` or `(&self) -> bool`
+            match &**cx.tcx.fn_sig(fn_id).skip_binder().inputs_and_output {
+                [arg, res] if !arg.is_mutable_ptr() && arg.peel_refs() == ty && res.is_bool() => NoChange,
+                _ => Lazy,
             }
+        } else {
+            Lazy
         }
+    } else {
+        Lazy
+    }
+}
 
+#[allow(clippy::too_many_lines)]
+fn expr_eagerness(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessSuggestion {
+    struct V<'cx, 'tcx> {
+        cx: &'cx LateContext<'tcx>,
+        eagerness: EagernessSuggestion,
+    }
+
+    impl<'cx, 'tcx> Visitor<'tcx> for V<'cx, 'tcx> {
+        type Map = ErasedMap<'tcx>;
         fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
             NestedVisitorMap::None
         }
+
+        fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
+            use EagernessSuggestion::{ForceNoChange, Lazy, NoChange};
+            if self.eagerness == ForceNoChange {
+                return;
+            }
+            match e.kind {
+                ExprKind::Call(
+                    &Expr {
+                        kind: ExprKind::Path(ref path),
+                        hir_id,
+                        ..
+                    },
+                    args,
+                ) => match self.cx.qpath_res(path, hir_id) {
+                    Res::Def(DefKind::Ctor(..) | DefKind::Variant, _) | Res::SelfCtor(_) => (),
+                    Res::Def(_, id) if self.cx.tcx.is_promotable_const_fn(id) => (),
+                    // No need to walk the arguments here, `is_const_evaluatable` already did
+                    Res::Def(..) if is_const_evaluatable(self.cx, e) => {
+                        self.eagerness |= NoChange;
+                        return;
+                    },
+                    Res::Def(_, id) => match path {
+                        QPath::Resolved(_, p) => {
+                            self.eagerness |= fn_eagerness(self.cx, id, p.segments.last().unwrap().ident.name, args);
+                        },
+                        QPath::TypeRelative(_, name) => {
+                            self.eagerness |= fn_eagerness(self.cx, id, name.ident.name, args);
+                        },
+                        QPath::LangItem(..) => self.eagerness = Lazy,
+                    },
+                    _ => self.eagerness = Lazy,
+                },
+                // No need to walk the arguments here, `is_const_evaluatable` already did
+                ExprKind::MethodCall(..) if is_const_evaluatable(self.cx, e) => {
+                    self.eagerness |= NoChange;
+                    return;
+                },
+                ExprKind::MethodCall(name, _, args, _) => {
+                    self.eagerness |= self
+                        .cx
+                        .typeck_results()
+                        .type_dependent_def_id(e.hir_id)
+                        .map_or(Lazy, |id| fn_eagerness(self.cx, id, name.ident.name, args));
+                },
+                ExprKind::Index(_, e) => {
+                    let ty = self.cx.typeck_results().expr_ty_adjusted(e);
+                    if is_copy(self.cx, ty) && !ty.is_ref() {
+                        self.eagerness |= NoChange;
+                    } else {
+                        self.eagerness = Lazy;
+                    }
+                },
+
+                // Dereferences should be cheap, but dereferencing a raw pointer earlier may not be safe.
+                ExprKind::Unary(UnOp::Deref, e) if !self.cx.typeck_results().expr_ty(e).is_unsafe_ptr() => (),
+                ExprKind::Unary(UnOp::Deref, _) => self.eagerness |= NoChange,
+
+                ExprKind::Unary(_, e)
+                    if matches!(
+                        self.cx.typeck_results().expr_ty(e).kind(),
+                        ty::Bool | ty::Int(_) | ty::Uint(_),
+                    ) => {},
+                ExprKind::Binary(_, lhs, rhs)
+                    if self.cx.typeck_results().expr_ty(lhs).is_primitive()
+                        && self.cx.typeck_results().expr_ty(rhs).is_primitive() => {},
+
+                // Can't be moved into a closure
+                ExprKind::Break(..)
+                | ExprKind::Continue(_)
+                | ExprKind::Ret(_)
+                | ExprKind::InlineAsm(_)
+                | ExprKind::LlvmInlineAsm(_)
+                | ExprKind::Yield(..)
+                | ExprKind::Err => {
+                    self.eagerness = ForceNoChange;
+                    return;
+                },
+
+                // Memory allocation, custom operator, loop, or call to an unknown function
+                ExprKind::Box(_)
+                | ExprKind::Unary(..)
+                | ExprKind::Binary(..)
+                | ExprKind::Loop(..)
+                | ExprKind::Call(..) => self.eagerness = Lazy,
+
+                ExprKind::ConstBlock(_)
+                | ExprKind::Array(_)
+                | ExprKind::Tup(_)
+                | ExprKind::Lit(_)
+                | ExprKind::Cast(..)
+                | ExprKind::Type(..)
+                | ExprKind::DropTemps(_)
+                | ExprKind::Let(..)
+                | ExprKind::If(..)
+                | ExprKind::Match(..)
+                | ExprKind::Closure(..)
+                | ExprKind::Field(..)
+                | ExprKind::Path(_)
+                | ExprKind::AddrOf(..)
+                | ExprKind::Struct(..)
+                | ExprKind::Repeat(..)
+                | ExprKind::Block(Block { stmts: [], .. }, _) => (),
+
+                // Assignment might be to a local defined earlier, so don't eagerly evaluate.
+                // Blocks with multiple statements might be expensive, so don't eagerly evaluate.
+                // TODO: Actually check if either of these are true here.
+                ExprKind::Assign(..) | ExprKind::AssignOp(..) | ExprKind::Block(..) => self.eagerness |= NoChange,
+            }
+            walk_expr(self, e);
+        }
     }
 
-    let mut finder = FunCallFinder { cx, found: false };
-    finder.visit_expr(expr);
-    finder.found
+    let mut v = V {
+        cx,
+        eagerness: EagernessSuggestion::Eager,
+    };
+    v.visit_expr(e);
+    v.eagerness
 }
 
-pub fn is_eagerness_candidate<'a, 'tcx>(cx: &'a LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
-    !identify_some_potentially_expensive_patterns(cx, expr) && identify_some_pure_patterns(expr)
+/// Whether the given expression should be changed to evaluate eagerly
+pub fn switch_to_eager_eval(cx: &'_ LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
+    expr_eagerness(cx, expr) == EagernessSuggestion::Eager
 }
 
-pub fn is_lazyness_candidate<'a, 'tcx>(cx: &'a LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
-    identify_some_potentially_expensive_patterns(cx, expr)
+/// Whether the given expression should be changed to evaluate lazily
+pub fn switch_to_lazy_eval(cx: &'_ LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
+    expr_eagerness(cx, expr) == EagernessSuggestion::Lazy
 }
index 480923bbb02da3dbe5bb8426426ed31fef0ce4c8..b04d17364264a032041b2208b7225c4bd4107f0f 100644 (file)
@@ -28,6 +28,7 @@
 pub(super) const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"];
 /// Preferably use the diagnostic item `sym::Borrow` where possible
 pub const BORROW_TRAIT: [&str; 3] = ["core", "borrow", "Borrow"];
+pub const BORROW_MUT_TRAIT: [&str; 3] = ["core", "borrow", "BorrowMut"];
 pub const BTREEMAP_CONTAINS_KEY: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "contains_key"];
 pub const BTREEMAP_ENTRY: [&str; 6] = ["alloc", "collections", "btree", "map", "entry", "Entry"];
 pub const BTREEMAP_INSERT: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "insert"];
index babab07ea9f6a1d4601404ddb6aaa0bb6f14ff88..438c39bea0a0ea6d4648bf81ed8ffa7b615638b8 100644 (file)
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::LateContext;
 use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
-use rustc_middle::ty::{self, AdtDef, IntTy, Ty, TyCtxt, TypeFoldable, UintTy};
-use rustc_span::sym;
-use rustc_span::symbol::{Ident, Symbol};
-use rustc_span::DUMMY_SP;
+use rustc_middle::ty::{self, AdtDef, IntTy, Predicate, Ty, TyCtxt, TypeFoldable, UintTy};
+use rustc_span::symbol::Ident;
+use rustc_span::{sym, Span, Symbol, DUMMY_SP};
 use rustc_trait_selection::infer::InferCtxtExt;
 use rustc_trait_selection::traits::query::normalize::AtExt;
+use std::iter;
 
 use crate::{match_def_path, must_use_attr};
 
@@ -391,3 +391,16 @@ pub fn is_uninit_value_valid_for_ty(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
         _ => false,
     }
 }
+
+/// Gets an iterator over all predicates which apply to the given item.
+pub fn all_predicates_of(tcx: TyCtxt<'_>, id: DefId) -> impl Iterator<Item = &(Predicate<'_>, Span)> {
+    let mut next_id = Some(id);
+    iter::from_fn(move || {
+        next_id.take().map(|id| {
+            let preds = tcx.predicates_of(id);
+            next_id = preds.parent;
+            preds.predicates.iter()
+        })
+    })
+    .flatten()
+}
index 988f6cf1d511338efb6fe8689cc24d5cfafc10c7..823df5cb7517eaeb641da7100dce261df64544df 100644 (file)
@@ -1,9 +1,11 @@
 use crate::path_to_local_id;
 use rustc_hir as hir;
+use rustc_hir::def::{DefKind, Res};
 use rustc_hir::intravisit::{self, walk_expr, NestedVisitorMap, Visitor};
-use rustc_hir::{def::Res, Arm, Block, Body, BodyId, Expr, ExprKind, HirId, Stmt};
+use rustc_hir::{Arm, Block, Body, BodyId, Expr, ExprKind, HirId, Stmt, UnOp};
 use rustc_lint::LateContext;
 use rustc_middle::hir::map::Map;
+use rustc_middle::ty;
 
 /// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested
 /// bodies (i.e. closures) are visited.
@@ -225,3 +227,93 @@ pub fn is_local_used(cx: &LateContext<'tcx>, visitable: impl Visitable<'tcx>, id
     drop(visitor);
     is_used
 }
+
+/// Checks if the given expression is a constant.
+pub fn is_const_evaluatable(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
+    struct V<'a, 'tcx> {
+        cx: &'a LateContext<'tcx>,
+        is_const: bool,
+    }
+    impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> {
+        type Map = Map<'tcx>;
+        fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+            NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
+        }
+
+        fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
+            if !self.is_const {
+                return;
+            }
+            match e.kind {
+                ExprKind::ConstBlock(_) => return,
+                ExprKind::Call(
+                    &Expr {
+                        kind: ExprKind::Path(ref p),
+                        hir_id,
+                        ..
+                    },
+                    _,
+                ) if self
+                    .cx
+                    .qpath_res(p, hir_id)
+                    .opt_def_id()
+                    .map_or(false, |id| self.cx.tcx.is_const_fn_raw(id)) => {},
+                ExprKind::MethodCall(..)
+                    if self
+                        .cx
+                        .typeck_results()
+                        .type_dependent_def_id(e.hir_id)
+                        .map_or(false, |id| self.cx.tcx.is_const_fn_raw(id)) => {},
+                ExprKind::Binary(_, lhs, rhs)
+                    if self.cx.typeck_results().expr_ty(lhs).peel_refs().is_primitive_ty()
+                        && self.cx.typeck_results().expr_ty(rhs).peel_refs().is_primitive_ty() => {},
+                ExprKind::Unary(UnOp::Deref, e) if self.cx.typeck_results().expr_ty(e).is_ref() => (),
+                ExprKind::Unary(_, e) if self.cx.typeck_results().expr_ty(e).peel_refs().is_primitive_ty() => (),
+                ExprKind::Index(base, _)
+                    if matches!(
+                        self.cx.typeck_results().expr_ty(base).peel_refs().kind(),
+                        ty::Slice(_) | ty::Array(..)
+                    ) => {},
+                ExprKind::Path(ref p)
+                    if matches!(
+                        self.cx.qpath_res(p, e.hir_id),
+                        Res::Def(
+                            DefKind::Const
+                                | DefKind::AssocConst
+                                | DefKind::AnonConst
+                                | DefKind::ConstParam
+                                | DefKind::Ctor(..)
+                                | DefKind::Fn
+                                | DefKind::AssocFn,
+                            _
+                        ) | Res::SelfCtor(_)
+                    ) => {},
+
+                ExprKind::AddrOf(..)
+                | ExprKind::Array(_)
+                | ExprKind::Block(..)
+                | ExprKind::Cast(..)
+                | ExprKind::DropTemps(_)
+                | ExprKind::Field(..)
+                | ExprKind::If(..)
+                | ExprKind::Let(..)
+                | ExprKind::Lit(_)
+                | ExprKind::Match(..)
+                | ExprKind::Repeat(..)
+                | ExprKind::Struct(..)
+                | ExprKind::Tup(_)
+                | ExprKind::Type(..) => (),
+
+                _ => {
+                    self.is_const = false;
+                    return;
+                },
+            }
+            walk_expr(self, e);
+        }
+    }
+
+    let mut v = V { cx, is_const: true };
+    v.visit_expr(e);
+    v.is_const
+}
index 812ce7163cd50ab215cd7130493b1e816d71ccf4..83f467c84002621244adf95dc5e8f848b6efb755 100644 (file)
@@ -108,6 +108,7 @@ fn test_return_in_macro() {
 }
 
 mod issue6501 {
+    #[allow(clippy::unnecessary_lazy_evaluations)]
     fn foo(bar: Result<(), ()>) {
         bar.unwrap_or_else(|_| {})
     }
index c42567b517c9115819b3446d5c5281c0da273884..341caf18bd60c83f73b41ddf81bc99b9f36fd83f 100644 (file)
@@ -108,6 +108,7 @@ fn test_return_in_macro() {
 }
 
 mod issue6501 {
+    #[allow(clippy::unnecessary_lazy_evaluations)]
     fn foo(bar: Result<(), ()>) {
         bar.unwrap_or_else(|_| return)
     }
index 74dda971fdabb632633f4a125ba15d6e2d5ad1a3..c0abc2c63dde1fb33c409476a3ab4ebdaac8823d 100644 (file)
@@ -85,109 +85,109 @@ LL |         return String::new();
    |         ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:112:32
+  --> $DIR/needless_return.rs:113:32
    |
 LL |         bar.unwrap_or_else(|_| return)
    |                                ^^^^^^ help: replace `return` with an empty block: `{}`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:117:13
+  --> $DIR/needless_return.rs:118:13
    |
 LL |             return;
    |             ^^^^^^^ help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:119:20
+  --> $DIR/needless_return.rs:120:20
    |
 LL |         let _ = || return;
    |                    ^^^^^^ help: replace `return` with an empty block: `{}`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:125:32
+  --> $DIR/needless_return.rs:126:32
    |
 LL |         res.unwrap_or_else(|_| return Foo)
    |                                ^^^^^^^^^^ help: remove `return`: `Foo`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:134:5
+  --> $DIR/needless_return.rs:135:5
    |
 LL |     return true;
    |     ^^^^^^^^^^^^ help: remove `return`: `true`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:138:5
+  --> $DIR/needless_return.rs:139:5
    |
 LL |     return true;
    |     ^^^^^^^^^^^^ help: remove `return`: `true`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:143:9
+  --> $DIR/needless_return.rs:144:9
    |
 LL |         return true;
    |         ^^^^^^^^^^^^ help: remove `return`: `true`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:145:9
+  --> $DIR/needless_return.rs:146:9
    |
 LL |         return false;
    |         ^^^^^^^^^^^^^ help: remove `return`: `false`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:151:17
+  --> $DIR/needless_return.rs:152:17
    |
 LL |         true => return false,
    |                 ^^^^^^^^^^^^ help: remove `return`: `false`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:153:13
+  --> $DIR/needless_return.rs:154:13
    |
 LL |             return true;
    |             ^^^^^^^^^^^^ help: remove `return`: `true`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:160:9
+  --> $DIR/needless_return.rs:161:9
    |
 LL |         return true;
    |         ^^^^^^^^^^^^ help: remove `return`: `true`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:162:16
+  --> $DIR/needless_return.rs:163:16
    |
 LL |     let _ = || return true;
    |                ^^^^^^^^^^^ help: remove `return`: `true`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:170:5
+  --> $DIR/needless_return.rs:171:5
    |
 LL |     return;
    |     ^^^^^^^ help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:175:9
+  --> $DIR/needless_return.rs:176:9
    |
 LL |         return;
    |         ^^^^^^^ help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:177:9
+  --> $DIR/needless_return.rs:178:9
    |
 LL |         return;
    |         ^^^^^^^ help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:184:14
+  --> $DIR/needless_return.rs:185:14
    |
 LL |         _ => return,
    |              ^^^^^^ help: replace `return` with an empty block: `{}`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:199:9
+  --> $DIR/needless_return.rs:200:9
    |
 LL |         return String::from("test");
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:201:9
+  --> $DIR/needless_return.rs:202:9
    |
 LL |         return String::new();
    |         ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`
index 9fdea79fe71b9531cba33caaca37f16fef58dbdd..ce3093c542ae0b7bfad86c1c10e665db25761cfa 100644 (file)
@@ -121,7 +121,7 @@ fn main() {
 
     let s = String::new();
     // Lint, both branches immutably borrow `s`.
-    let _ = Some(0).map_or_else(|| s.len(), |x| s.len() + x);
+    let _ = Some(0).map_or(s.len(), |x| s.len() + x);
 
     let s = String::new();
     // Lint, `Some` branch consumes `s`, but else branch doesn't use `s`.
index d7711bc1c47ad8fde71be741608d9c2e59070b36..4e64cd7cdb1d649a73edfaedf1a97bce0ac0b742 100644 (file)
@@ -180,11 +180,11 @@ LL +             }
 LL ~         });
    |
 
-error: use Option::map_or_else instead of an if let/else
+error: use Option::map_or instead of an if let/else
   --> $DIR/option_if_let_else.rs:151:13
    |
 LL |     let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() };
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or_else(|| s.len(), |x| s.len() + x)`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)`
 
 error: use Option::map_or instead of an if let/else
   --> $DIR/option_if_let_else.rs:155:13
index c2f94d0e8575650272656acba755013a0de37ddb..d6d6ab49734e782ba7887824e86d41a8f29bcc22 100644 (file)
@@ -43,7 +43,7 @@ fn or_fun_call() {
     with_enum.unwrap_or(Enum::A(5));
 
     let with_const_fn = Some(Duration::from_secs(1));
-    with_const_fn.unwrap_or_else(|| Duration::from_secs(5));
+    with_const_fn.unwrap_or(Duration::from_secs(5));
 
     let with_constructor = Some(vec![1]);
     with_constructor.unwrap_or_else(make);
@@ -79,16 +79,16 @@ fn or_fun_call() {
     without_default.unwrap_or_else(Foo::new);
 
     let mut map = HashMap::<u64, String>::new();
-    map.entry(42).or_insert_with(String::new);
+    map.entry(42).or_insert(String::new());
 
     let mut map_vec = HashMap::<u64, Vec<i32>>::new();
-    map_vec.entry(42).or_insert_with(Vec::new);
+    map_vec.entry(42).or_insert(vec![]);
 
     let mut btree = BTreeMap::<u64, String>::new();
-    btree.entry(42).or_insert_with(String::new);
+    btree.entry(42).or_insert(String::new());
 
     let mut btree_vec = BTreeMap::<u64, Vec<i32>>::new();
-    btree_vec.entry(42).or_insert_with(Vec::new);
+    btree_vec.entry(42).or_insert(vec![]);
 
     let stringy = Some(String::from(""));
     let _ = stringy.unwrap_or_else(|| "".to_owned());
@@ -129,7 +129,7 @@ fn test_or_with_ctors() {
 
     let b = "b".to_string();
     let _ = Some(Bar("a".to_string(), Duration::from_secs(1)))
-        .or_else(|| Some(Bar(b, Duration::from_secs(2))));
+        .or(Some(Bar(b, Duration::from_secs(2))));
 
     let vec = vec!["foo"];
     let _ = opt.ok_or(vec.len());
@@ -155,16 +155,24 @@ fn f() -> Option<()> {
 }
 
 mod issue6675 {
+    unsafe fn ptr_to_ref<'a, T>(p: *const T) -> &'a T {
+        #[allow(unused)]
+        let x = vec![0; 1000]; // future-proofing, make this function expensive.
+        &*p
+    }
+
     unsafe fn foo() {
-        let mut s = "test".to_owned();
-        None.unwrap_or_else(|| s.as_mut_vec());
+        let s = "test".to_owned();
+        let s = &s as *const _;
+        None.unwrap_or_else(|| ptr_to_ref(s));
     }
 
     fn bar() {
-        let mut s = "test".to_owned();
-        None.unwrap_or_else(|| unsafe { s.as_mut_vec() });
+        let s = "test".to_owned();
+        let s = &s as *const _;
+        None.unwrap_or_else(|| unsafe { ptr_to_ref(s) });
         #[rustfmt::skip]
-        None.unwrap_or_else(|| unsafe { s.as_mut_vec() });
+        None.unwrap_or_else(|| unsafe { ptr_to_ref(s) });
     }
 }
 
index afaf92961b0274f75d9e5ffbbd2f40b606fb876a..8eadc6ce3b47acace2c3fcf4cea29fd2db831b92 100644 (file)
@@ -155,16 +155,24 @@ fn f() -> Option<()> {
 }
 
 mod issue6675 {
+    unsafe fn ptr_to_ref<'a, T>(p: *const T) -> &'a T {
+        #[allow(unused)]
+        let x = vec![0; 1000]; // future-proofing, make this function expensive.
+        &*p
+    }
+
     unsafe fn foo() {
-        let mut s = "test".to_owned();
-        None.unwrap_or(s.as_mut_vec());
+        let s = "test".to_owned();
+        let s = &s as *const _;
+        None.unwrap_or(ptr_to_ref(s));
     }
 
     fn bar() {
-        let mut s = "test".to_owned();
-        None.unwrap_or(unsafe { s.as_mut_vec() });
+        let s = "test".to_owned();
+        let s = &s as *const _;
+        None.unwrap_or(unsafe { ptr_to_ref(s) });
         #[rustfmt::skip]
-        None.unwrap_or( unsafe { s.as_mut_vec() }    );
+        None.unwrap_or( unsafe { ptr_to_ref(s) }    );
     }
 }
 
index b2bcbd38c2df36061eefebf4f788821471955d6f..9d0c42b10c27f500eaa0469e43d1d1bcbfb0ae65 100644 (file)
@@ -1,16 +1,10 @@
-error: use of `unwrap_or` followed by a function call
-  --> $DIR/or_fun_call.rs:46:19
-   |
-LL |     with_const_fn.unwrap_or(Duration::from_secs(5));
-   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Duration::from_secs(5))`
-   |
-   = note: `-D clippy::or-fun-call` implied by `-D warnings`
-
 error: use of `unwrap_or` followed by a function call
   --> $DIR/or_fun_call.rs:49:22
    |
 LL |     with_constructor.unwrap_or(make());
    |                      ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(make)`
+   |
+   = note: `-D clippy::or-fun-call` implied by `-D warnings`
 
 error: use of `unwrap_or` followed by a call to `new`
   --> $DIR/or_fun_call.rs:52:5
@@ -72,30 +66,6 @@ error: use of `unwrap_or` followed by a function call
 LL |     without_default.unwrap_or(Foo::new());
    |                     ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)`
 
-error: use of `or_insert` followed by a function call
-  --> $DIR/or_fun_call.rs:82:19
-   |
-LL |     map.entry(42).or_insert(String::new());
-   |                   ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
-
-error: use of `or_insert` followed by a function call
-  --> $DIR/or_fun_call.rs:85:23
-   |
-LL |     map_vec.entry(42).or_insert(vec![]);
-   |                       ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(Vec::new)`
-
-error: use of `or_insert` followed by a function call
-  --> $DIR/or_fun_call.rs:88:21
-   |
-LL |     btree.entry(42).or_insert(String::new());
-   |                     ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
-
-error: use of `or_insert` followed by a function call
-  --> $DIR/or_fun_call.rs:91:25
-   |
-LL |     btree_vec.entry(42).or_insert(vec![]);
-   |                         ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(Vec::new)`
-
 error: use of `unwrap_or` followed by a function call
   --> $DIR/or_fun_call.rs:94:21
    |
@@ -120,29 +90,23 @@ error: use of `or` followed by a function call
 LL |     let _ = Some("a".to_string()).or(Some("b".to_string()));
    |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))`
 
-error: use of `or` followed by a function call
-  --> $DIR/or_fun_call.rs:132:10
-   |
-LL |         .or(Some(Bar(b, Duration::from_secs(2))));
-   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some(Bar(b, Duration::from_secs(2))))`
-
 error: use of `unwrap_or` followed by a function call
-  --> $DIR/or_fun_call.rs:160:14
+  --> $DIR/or_fun_call.rs:167:14
    |
-LL |         None.unwrap_or(s.as_mut_vec());
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| s.as_mut_vec())`
+LL |         None.unwrap_or(ptr_to_ref(s));
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| ptr_to_ref(s))`
 
 error: use of `unwrap_or` followed by a function call
-  --> $DIR/or_fun_call.rs:165:14
+  --> $DIR/or_fun_call.rs:173:14
    |
-LL |         None.unwrap_or(unsafe { s.as_mut_vec() });
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { s.as_mut_vec() })`
+LL |         None.unwrap_or(unsafe { ptr_to_ref(s) });
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })`
 
 error: use of `unwrap_or` followed by a function call
-  --> $DIR/or_fun_call.rs:167:14
+  --> $DIR/or_fun_call.rs:175:14
    |
-LL |         None.unwrap_or( unsafe { s.as_mut_vec() }    );
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { s.as_mut_vec() })`
+LL |         None.unwrap_or( unsafe { ptr_to_ref(s) }    );
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })`
 
-error: aborting due to 24 previous errors
+error: aborting due to 18 previous errors