Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-12-01 00:49:30 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #7639 - Jarcho:whitelist_cheap_functions, r=camsteffen

Browse source 
Improve heuristic for eagerness suggestion

Still to be done:

* a more complete list of cheap functions
* a way to limit the complexity of cheap expressions

changelog: Improve heuristics for `or_fun_call` and `unnecessary_lazy_evaluations`
This commit is contained in:
bors 2021-11-16 03:12:30 +00:00
commit 5b1b65b876
17 changed files with 416 additions and 248 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
clippy_lints/src/functions/too_many_lines.rs
View file

@ -56,8 +56,8 @@ pub(super) fn check_fn(
continue; continue;
} }
} else { } else {
let multi_idx = line.find("/*").unwrap_or_else(|| line.len()); let multi_idx = line.find("/*").unwrap_or(line.len());
let single_idx = line.find("//").unwrap_or_else(|| line.len()); let single_idx = line.find("//").unwrap_or(line.len());
code_in_line |= multi_idx > 0 && single_idx > 0; code_in_line |= multi_idx > 0 && single_idx > 0;
// Implies multi_idx is below line.len() // Implies multi_idx is below line.len()
if multi_idx < single_idx { if multi_idx < single_idx {

2
clippy_lints/src/methods/from_iter_instead_of_collect.rs
View file

@ -69,7 +69,7 @@ fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) -
// i.e.: 2 wildcards in `std::collections::BTreeMap<&i32, &char>` // i.e.: 2 wildcards in `std::collections::BTreeMap<&i32, &char>`
let ty_str = ty.to_string(); let ty_str = ty.to_string();
let start = ty_str.find('<').unwrap_or(0); 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 nb_wildcard = ty_str[start..end].split(',').count();
let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1)); let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1));
format!("{}<{}>", elements.join("::"), wildcards) format!("{}<{}>", elements.join("::"), wildcards)

59
clippy_lints/src/methods/or_fun_call.rs
View file

@ -1,16 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::eager_or_lazy::is_lazyness_candidate; use clippy_utils::eager_or_lazy::switch_to_lazy_eval;
use clippy_utils::is_trait_item;
use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_macro_callsite}; use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_macro_callsite};
use clippy_utils::ty::implements_trait; use clippy_utils::ty::{implements_trait, match_type};
use clippy_utils::ty::{is_type_diagnostic_item, match_type}; use clippy_utils::{contains_return, is_trait_item, last_path_segment, paths};
use clippy_utils::{contains_return, last_path_segment, paths};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::{BlockCheckMode, UnsafeSource};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::source_map::Span; use rustc_span::source_map::Span;
use rustc_span::symbol::{kw, sym}; use rustc_span::symbol::{kw, sym};
use std::borrow::Cow; use std::borrow::Cow;
@ -96,25 +92,10 @@ pub(super) fn check<'tcx>(
(&paths::RESULT, true, &["or", "unwrap_or"], "else"), (&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_chain! {
if KNOW_TYPES.iter().any(|k| k.2.contains(&name)); 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); if !contains_return(arg);
let self_ty = cx.typeck_results().expr_ty(self_expr); let self_ty = cx.typeck_results().expr_ty(self_expr);
@ -166,26 +147,30 @@ pub(super) fn check<'tcx>(
} }
} }
if args.len() == 2 { if let [self_arg, arg] = args {
match args[1].kind { 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) => { hir::ExprKind::Call(fun, or_args) => {
let or_has_args = !or_args.is_empty(); 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) }; 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(..) => { hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
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);
},
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);
}
}
}, },
_ => (), _ => (),
} }

2
clippy_lints/src/methods/unnecessary_lazy_eval.rs
View file

@ -30,7 +30,7 @@ pub(super) fn check<'tcx>(
return; 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 { let msg = if is_option {
"unnecessary closure used to substitute value for `Option::None`" "unnecessary closure used to substitute value for `Option::None`"
} else { } else {

6
clippy_lints/src/option_if_let_else.rs
View 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 capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" };
let some_body = extract_body_from_expr(if_then)?; let some_body = extract_body_from_expr(if_then)?;
let none_body = extract_body_from_expr(if_else)?; let none_body = extract_body_from_expr(if_else)?;
let method_sugg = if eager_or_lazy::is_eagerness_candidate(cx, none_body) { let method_sugg = if eager_or_lazy::switch_to_eager_eval(cx, none_body) { "map_or" } else { "map_or_else" };
"map_or"
} else {
"map_or_else"
};
let capture_name = id.name.to_ident_string(); let capture_name = id.name.to_ident_string();
let (as_ref, as_mut) = match &let_expr.kind { let (as_ref, as_mut) = match &let_expr.kind {
ExprKind::AddrOf(_, Mutability::Not, _) => (true, false), ExprKind::AddrOf(_, Mutability::Not, _) => (true, false),

313
clippy_utils/src/eager_or_lazy.rs
View file

@ -9,128 +9,227 @@
//! - or-fun-call //! - or-fun-call
//! - option-if-let-else //! - option-if-let-else
use crate::is_ctor_or_promotable_const_function; use crate::ty::{all_predicates_of, is_copy};
use crate::ty::is_type_diagnostic_item; use crate::visitors::is_const_evaluatable;
use rustc_hir::def::{DefKind, Res}; use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor};
use rustc_hir::intravisit; use rustc_hir::{def_id::DefId, Block, Expr, ExprKind, QPath, UnOp};
use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
use rustc_hir::{Block, Expr, ExprKind, Path, QPath};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::hir::map::Map; use rustc_middle::ty::{self, PredicateKind};
use rustc_span::sym; use rustc_span::{sym, Symbol};
use std::cmp;
use std::ops;
/// Is the expr pure (is it free from side-effects)? #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
/// This function is named so to stress that it isn't exhaustive and returns FNs. enum EagernessSuggestion {
fn identify_some_pure_patterns(expr: &Expr<'_>) -> bool { // The expression is cheap and should be evaluated eagerly
match expr.kind { Eager,
ExprKind::Lit(..) | ExprKind::ConstBlock(..) | ExprKind::Path(..) | ExprKind::Field(..) => true, // The expression may be cheap, so don't suggested lazy evaluation; or the expression may not be safe to switch to
ExprKind::AddrOf(_, _, addr_of_expr) => identify_some_pure_patterns(addr_of_expr), // eager evaluation.
ExprKind::Tup(tup_exprs) => tup_exprs.iter().all(identify_some_pure_patterns), NoChange,
ExprKind::Struct(_, fields, expr) => { // The expression is likely expensive and should be evaluated lazily.
fields.iter().all(|f| identify_some_pure_patterns(f.expr)) && expr.map_or(true, identify_some_pure_patterns) Lazy,
}, // The expression cannot be placed into a closure.
ExprKind::Call( ForceNoChange,
&Expr { }
kind: impl ops::BitOr for EagernessSuggestion {
ExprKind::Path(QPath::Resolved( type Output = Self;
_, fn bitor(self, rhs: Self) -> Self {
Path { cmp::max(self, rhs)
res: Res::Def(DefKind::Ctor(..) | DefKind::Variant, ..), }
.. }
}, impl ops::BitOrAssign for EagernessSuggestion {
)), fn bitor_assign(&mut self, rhs: Self) {
.. *self = *self | rhs;
},
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,
} }
} }
/// Identify some potentially computationally expensive patterns. /// Determine the eagerness of the given function call.
/// This function is named so to stress that its implementation is non-exhaustive. fn fn_eagerness(cx: &LateContext<'tcx>, fn_id: DefId, name: Symbol, args: &'tcx [Expr<'_>]) -> EagernessSuggestion {
/// It returns FNs and FPs. use EagernessSuggestion::{Eager, Lazy, NoChange};
fn identify_some_potentially_expensive_patterns<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { let name = &*name.as_str();
// Searches an expression for method calls or function calls that aren't ctors
struct FunCallFinder<'a, 'tcx> { let ty = match cx.tcx.impl_of_method(fn_id) {
cx: &'a LateContext<'tcx>, Some(id) => cx.tcx.type_of(id),
found: bool, None => return Lazy,
};
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<'a, 'tcx> intravisit::Visitor<'tcx> for FunCallFinder<'a, 'tcx> { impl<'cx, 'tcx> Visitor<'tcx> for V<'cx, 'tcx> {
type Map = Map<'tcx>; type Map = ErasedMap<'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,
};
if call_found {
self.found |= true;
}
if !self.found {
intravisit::walk_expr(self, expr);
}
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None 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 }; let mut v = V {
finder.visit_expr(expr); cx,
finder.found 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 { /// Whether the given expression should be changed to evaluate eagerly
!identify_some_potentially_expensive_patterns(cx, expr) && identify_some_pure_patterns(expr) 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 { /// Whether the given expression should be changed to evaluate lazily
identify_some_potentially_expensive_patterns(cx, expr) pub fn switch_to_lazy_eval(cx: &'_ LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
expr_eagerness(cx, expr) == EagernessSuggestion::Lazy
} }

1
clippy_utils/src/paths.rs
View file

@ -28,6 +28,7 @@ pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"];
pub(super) const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"]; pub(super) const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"];
/// Preferably use the diagnostic item `sym::Borrow` where possible /// Preferably use the diagnostic item `sym::Borrow` where possible
pub const BORROW_TRAIT: [&str; 3] = ["core", "borrow", "Borrow"]; 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_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_ENTRY: [&str; 6] = ["alloc", "collections", "btree", "map", "entry", "Entry"];
pub const BTREEMAP_INSERT: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "insert"]; pub const BTREEMAP_INSERT: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "insert"];

21
clippy_utils/src/ty.rs
View file

@ -10,12 +10,12 @@ use rustc_hir::{TyKind, Unsafety};
use rustc_infer::infer::TyCtxtInferExt; use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::ty::subst::{GenericArg, GenericArgKind}; use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
use rustc_middle::ty::{self, AdtDef, IntTy, Ty, TyCtxt, TypeFoldable, UintTy}; use rustc_middle::ty::{self, AdtDef, IntTy, Predicate, Ty, TyCtxt, TypeFoldable, UintTy};
use rustc_span::sym; use rustc_span::symbol::Ident;
use rustc_span::symbol::{Ident, Symbol}; use rustc_span::{sym, Span, Symbol, DUMMY_SP};
use rustc_span::DUMMY_SP;
use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::query::normalize::AtExt; use rustc_trait_selection::traits::query::normalize::AtExt;
use std::iter;
use crate::{match_def_path, must_use_attr}; 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, _ => 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()
}

94
clippy_utils/src/visitors.rs
View file

@ -1,9 +1,11 @@
use crate::path_to_local_id; use crate::path_to_local_id;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{self, walk_expr, NestedVisitorMap, Visitor}; 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_lint::LateContext;
use rustc_middle::hir::map::Map; use rustc_middle::hir::map::Map;
use rustc_middle::ty;
/// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested /// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested
/// bodies (i.e. closures) are visited. /// 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); drop(visitor);
is_used 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
}

1
tests/ui/needless_return.fixed
View file

@ -108,6 +108,7 @@ fn test_return_in_macro() {
} }
mod issue6501 { mod issue6501 {
#[allow(clippy::unnecessary_lazy_evaluations)]
fn foo(bar: Result<(), ()>) { fn foo(bar: Result<(), ()>) {
bar.unwrap_or_else(|_| {}) bar.unwrap_or_else(|_| {})
} }

1
tests/ui/needless_return.rs
View file

@ -108,6 +108,7 @@ fn test_return_in_macro() {
} }
mod issue6501 { mod issue6501 {
#[allow(clippy::unnecessary_lazy_evaluations)]
fn foo(bar: Result<(), ()>) { fn foo(bar: Result<(), ()>) {
bar.unwrap_or_else(|_| return) bar.unwrap_or_else(|_| return)
} }

36
tests/ui/needless_return.stderr
View file

@ -85,109 +85,109 @@ LL | return String::new();
| ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:112:32 --> $DIR/needless_return.rs:113:32
| |
LL | bar.unwrap_or_else(|_| return) LL | bar.unwrap_or_else(|_| return)
| ^^^^^^ help: replace `return` with an empty block: `{}` | ^^^^^^ help: replace `return` with an empty block: `{}`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:117:13 --> $DIR/needless_return.rs:118:13
| |
LL | return; LL | return;
| ^^^^^^^ help: remove `return` | ^^^^^^^ help: remove `return`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:119:20 --> $DIR/needless_return.rs:120:20
| |
LL | let _ = || return; LL | let _ = || return;
| ^^^^^^ help: replace `return` with an empty block: `{}` | ^^^^^^ help: replace `return` with an empty block: `{}`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:125:32 --> $DIR/needless_return.rs:126:32
| |
LL | res.unwrap_or_else(|_| return Foo) LL | res.unwrap_or_else(|_| return Foo)
| ^^^^^^^^^^ help: remove `return`: `Foo` | ^^^^^^^^^^ help: remove `return`: `Foo`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:134:5 --> $DIR/needless_return.rs:135:5
| |
LL | return true; LL | return true;
| ^^^^^^^^^^^^ help: remove `return`: `true` | ^^^^^^^^^^^^ help: remove `return`: `true`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:138:5 --> $DIR/needless_return.rs:139:5
| |
LL | return true; LL | return true;
| ^^^^^^^^^^^^ help: remove `return`: `true` | ^^^^^^^^^^^^ help: remove `return`: `true`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:143:9 --> $DIR/needless_return.rs:144:9
| |
LL | return true; LL | return true;
| ^^^^^^^^^^^^ help: remove `return`: `true` | ^^^^^^^^^^^^ help: remove `return`: `true`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:145:9 --> $DIR/needless_return.rs:146:9
| |
LL | return false; LL | return false;
| ^^^^^^^^^^^^^ help: remove `return`: `false` | ^^^^^^^^^^^^^ help: remove `return`: `false`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:151:17 --> $DIR/needless_return.rs:152:17
| |
LL | true => return false, LL | true => return false,
| ^^^^^^^^^^^^ help: remove `return`: `false` | ^^^^^^^^^^^^ help: remove `return`: `false`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:153:13 --> $DIR/needless_return.rs:154:13
| |
LL | return true; LL | return true;
| ^^^^^^^^^^^^ help: remove `return`: `true` | ^^^^^^^^^^^^ help: remove `return`: `true`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:160:9 --> $DIR/needless_return.rs:161:9
| |
LL | return true; LL | return true;
| ^^^^^^^^^^^^ help: remove `return`: `true` | ^^^^^^^^^^^^ help: remove `return`: `true`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:162:16 --> $DIR/needless_return.rs:163:16
| |
LL | let _ = || return true; LL | let _ = || return true;
| ^^^^^^^^^^^ help: remove `return`: `true` | ^^^^^^^^^^^ help: remove `return`: `true`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:170:5 --> $DIR/needless_return.rs:171:5
| |
LL | return; LL | return;
| ^^^^^^^ help: remove `return` | ^^^^^^^ help: remove `return`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:175:9 --> $DIR/needless_return.rs:176:9
| |
LL | return; LL | return;
| ^^^^^^^ help: remove `return` | ^^^^^^^ help: remove `return`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:177:9 --> $DIR/needless_return.rs:178:9
| |
LL | return; LL | return;
| ^^^^^^^ help: remove `return` | ^^^^^^^ help: remove `return`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:184:14 --> $DIR/needless_return.rs:185:14
| |
LL | _ => return, LL | _ => return,
| ^^^^^^ help: replace `return` with an empty block: `{}` | ^^^^^^ help: replace `return` with an empty block: `{}`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:199:9 --> $DIR/needless_return.rs:200:9
| |
LL | return String::from("test"); LL | return String::from("test");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")`
error: unneeded `return` statement error: unneeded `return` statement
--> $DIR/needless_return.rs:201:9 --> $DIR/needless_return.rs:202:9
| |
LL | return String::new(); LL | return String::new();
| ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`

2
tests/ui/option_if_let_else.fixed
View file

@ -121,7 +121,7 @@ fn main() {
let s = String::new(); let s = String::new();
// Lint, both branches immutably borrow `s`. // 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(); let s = String::new();
// Lint, `Some` branch consumes `s`, but else branch doesn't use `s`. // Lint, `Some` branch consumes `s`, but else branch doesn't use `s`.

4
tests/ui/option_if_let_else.stderr
View file

@ -180,11 +180,11 @@ LL + }
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 --> $DIR/option_if_let_else.rs:151:13
| |
LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() }; 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 error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:155:13 --> $DIR/option_if_let_else.rs:155:13

30
tests/ui/or_fun_call.fixed
View file

@ -43,7 +43,7 @@ fn or_fun_call() {
with_enum.unwrap_or(Enum::A(5)); with_enum.unwrap_or(Enum::A(5));
let with_const_fn = Some(Duration::from_secs(1)); 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]); let with_constructor = Some(vec![1]);
with_constructor.unwrap_or_else(make); with_constructor.unwrap_or_else(make);
@ -79,16 +79,16 @@ fn or_fun_call() {
without_default.unwrap_or_else(Foo::new); without_default.unwrap_or_else(Foo::new);
let mut map = HashMap::<u64, String>::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(); 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(); 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(); 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 = Some(String::from(""));
let _ = stringy.unwrap_or_else(|| "".to_owned()); let _ = stringy.unwrap_or_else(|| "".to_owned());
@ -129,7 +129,7 @@ fn test_or_with_ctors() {
let b = "b".to_string(); let b = "b".to_string();
let _ = Some(Bar("a".to_string(), Duration::from_secs(1))) 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 vec = vec!["foo"];
let _ = opt.ok_or(vec.len()); let _ = opt.ok_or(vec.len());
@ -155,16 +155,24 @@ fn f() -> Option<()> {
} }
mod issue6675 { 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() { unsafe fn foo() {
let mut s = "test".to_owned(); let s = "test".to_owned();
None.unwrap_or_else(|| s.as_mut_vec()); let s = &s as *const _;
None.unwrap_or_else(|| ptr_to_ref(s));
} }
fn bar() { fn bar() {
let mut s = "test".to_owned(); let s = "test".to_owned();
None.unwrap_or_else(|| unsafe { s.as_mut_vec() }); let s = &s as *const _;
None.unwrap_or_else(|| unsafe { ptr_to_ref(s) });
#[rustfmt::skip] #[rustfmt::skip]
None.unwrap_or_else(|| unsafe { s.as_mut_vec() }); None.unwrap_or_else(|| unsafe { ptr_to_ref(s) });
} }
} }

18
tests/ui/or_fun_call.rs
View file

@ -155,16 +155,24 @@ fn f() -> Option<()> {
} }
mod issue6675 { 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() { unsafe fn foo() {
let mut s = "test".to_owned(); let s = "test".to_owned();
None.unwrap_or(s.as_mut_vec()); let s = &s as *const _;
None.unwrap_or(ptr_to_ref(s));
} }
fn bar() { fn bar() {
let mut s = "test".to_owned(); let s = "test".to_owned();
None.unwrap_or(unsafe { s.as_mut_vec() }); let s = &s as *const _;
None.unwrap_or(unsafe { ptr_to_ref(s) });
#[rustfmt::skip] #[rustfmt::skip]
None.unwrap_or( unsafe { s.as_mut_vec() } ); None.unwrap_or( unsafe { ptr_to_ref(s) } );
} }
} }

70
tests/ui/or_fun_call.stderr
View 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 error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:49:22 --> $DIR/or_fun_call.rs:49:22
| |
LL | with_constructor.unwrap_or(make()); LL | with_constructor.unwrap_or(make());
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(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` error: use of `unwrap_or` followed by a call to `new`
--> $DIR/or_fun_call.rs:52:5 --> $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()); LL | without_default.unwrap_or(Foo::new());
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(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 error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:94:21 --> $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())); LL | let _ = Some("a".to_string()).or(Some("b".to_string()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| 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
|
LL | None.unwrap_or(s.as_mut_vec());
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| s.as_mut_vec())`
error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:165:14
|
LL | None.unwrap_or(unsafe { s.as_mut_vec() });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { s.as_mut_vec() })`
error: use of `unwrap_or` followed by a function call error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:167:14 --> $DIR/or_fun_call.rs:167:14
| |
LL | None.unwrap_or( unsafe { s.as_mut_vec() } ); LL | None.unwrap_or(ptr_to_ref(s));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { s.as_mut_vec() })` | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| ptr_to_ref(s))`
error: aborting due to 24 previous errors error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:173:14
|
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:175:14
|
LL | None.unwrap_or( unsafe { ptr_to_ref(s) } );
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })`
error: aborting due to 18 previous errors