use std::cmp;
+use std::iter;
-use crate::utils::is_self_ty;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::ty::is_copy;
+use clippy_utils::{is_self, is_self_ty};
use if_chain::if_chain;
use rustc_ast::attr;
use rustc_errors::Applicability;
use rustc_hir::{BindingAnnotation, Body, FnDecl, HirId, Impl, ItemKind, MutTy, Mutability, Node, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
+use rustc_middle::ty::layout::LayoutOf;
use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::def_id::LocalDefId;
use rustc_span::{sym, Span};
-use rustc_target::abi::LayoutOf;
use rustc_target::spec::abi::Abi;
use rustc_target::spec::Target;
declare_clippy_lint! {
- /// **What it does:** Checks for functions taking arguments by reference, where
+ /// ### What it does
+ /// Checks for functions taking arguments by reference, where
/// the argument type is `Copy` and small enough to be more efficient to always
/// pass by value.
///
- /// **Why is this bad?** In many calling conventions instances of structs will
+ /// ### Why is this bad?
+ /// In many calling conventions instances of structs will
/// be passed through registers if they fit into two or less general purpose
/// registers.
///
- /// **Known problems:** This lint is target register size dependent, it is
+ /// ### Known problems
+ /// This lint is target register size dependent, it is
/// limited to 32-bit to try and reduce portability problems between 32 and
/// 64-bit, but if you are compiling for 8 or 16-bit targets then the limit
/// will be different.
/// false positives in cases involving multiple lifetimes that are bounded by
/// each other.
///
- /// **Example:**
+ /// Also, it does not take account of other similar cases where getting memory addresses
+ /// matters; namely, returning the pointer to the argument in question,
+ /// and passing the argument, as both references and pointers,
+ /// to a function that needs the memory address. For further details, refer to
+ /// [this issue](https://github.com/rust-lang/rust-clippy/issues/5953)
+ /// that explains a real case in which this false positive
+ /// led to an **undefined behaviour** introduced with unsafe code.
+ ///
+ /// ### Example
///
/// ```rust
/// // Bad
/// // Better
/// fn foo(v: u32) {}
/// ```
+ #[clippy::version = "pre 1.29.0"]
pub TRIVIALLY_COPY_PASS_BY_REF,
pedantic,
"functions taking small copyable arguments by reference"
}
declare_clippy_lint! {
- /// **What it does:** Checks for functions taking arguments by value, where
+ /// ### What it does
+ /// Checks for functions taking arguments by value, where
/// the argument type is `Copy` and large enough to be worth considering
/// passing by reference. Does not trigger if the function is being exported,
/// because that might induce API breakage, if the parameter is declared as mutable,
/// or if the argument is a `self`.
///
- /// **Why is this bad?** Arguments passed by value might result in an unnecessary
+ /// ### Why is this bad?
+ /// Arguments passed by value might result in an unnecessary
/// shallow copy, taking up more space in the stack and requiring a call to
/// `memcpy`, which can be expensive.
///
- /// **Example:**
- ///
+ /// ### Example
/// ```rust
/// #[derive(Clone, Copy)]
/// struct TooLarge([u8; 2048]);
/// // Good
/// fn foo(v: &TooLarge) {}
/// ```
+ #[clippy::version = "1.49.0"]
pub LARGE_TYPES_PASSED_BY_VALUE,
pedantic,
"functions taking large arguments by value"
pub struct PassByRefOrValue {
ref_min_size: u64,
value_max_size: u64,
+ avoid_breaking_exported_api: bool,
}
impl<'tcx> PassByRefOrValue {
- pub fn new(ref_min_size: Option<u64>, value_max_size: u64, target: &Target) -> Self {
+ pub fn new(
+ ref_min_size: Option<u64>,
+ value_max_size: u64,
+ avoid_breaking_exported_api: bool,
+ target: &Target,
+ ) -> Self {
let ref_min_size = ref_min_size.unwrap_or_else(|| {
let bit_width = u64::from(target.pointer_width);
// Cap the calculated bit width at 32-bits to reduce
Self {
ref_min_size,
value_max_size,
+ avoid_breaking_exported_api,
}
}
- fn check_poly_fn(&mut self, cx: &LateContext<'tcx>, hir_id: HirId, decl: &FnDecl<'_>, span: Option<Span>) {
- let fn_def_id = cx.tcx.hir().local_def_id(hir_id);
+ fn check_poly_fn(&mut self, cx: &LateContext<'tcx>, def_id: LocalDefId, decl: &FnDecl<'_>, span: Option<Span>) {
+ if self.avoid_breaking_exported_api && cx.access_levels.is_exported(def_id) {
+ return;
+ }
- let fn_sig = cx.tcx.fn_sig(fn_def_id);
+ let fn_sig = cx.tcx.fn_sig(def_id);
let fn_sig = cx.tcx.erase_late_bound_regions(fn_sig);
let fn_body = cx.enclosing_body.map(|id| cx.tcx.hir().body(id));
- for (index, (input, &ty)) in decl.inputs.iter().zip(fn_sig.inputs()).enumerate() {
+ for (index, (input, &ty)) in iter::zip(decl.inputs, fn_sig.inputs()).enumerate() {
// All spans generated from a proc-macro invocation are the same...
match span {
Some(s) if s == input.span => return,
};
if_chain! {
- if !output_lts.contains(&input_lt);
+ if !output_lts.contains(input_lt);
if is_copy(cx, ty);
if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes());
if size <= self.ref_min_size;
- if let hir::TyKind::Rptr(_, MutTy { ty: ref decl_ty, .. }) = input.kind;
+ if let hir::TyKind::Rptr(_, MutTy { ty: decl_ty, .. }) = input.kind;
then {
- let value_type = if is_self_ty(decl_ty) {
+ let value_type = if fn_body.and_then(|body| body.params.get(index)).map_or(false, is_self) {
"self".into()
} else {
snippet(cx, decl_ty.span, "_").into()
}
if_chain! {
- if !cx.access_levels.is_exported(hir_id);
if is_copy(cx, ty);
if !is_self_ty(input);
if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes());
}
if let hir::TraitItemKind::Fn(method_sig, _) = &item.kind {
- self.check_poly_fn(cx, item.hir_id(), &*method_sig.decl, None);
+ self.check_poly_fn(cx, item.def_id, &*method_sig.decl, None);
}
}
}
}
- self.check_poly_fn(cx, hir_id, decl, Some(span));
+ self.check_poly_fn(cx, cx.tcx.hir().local_def_id(hir_id), decl, Some(span));
}
}
projects / rust.git / blobdiff