diff options
Diffstat (limited to '')
5 files changed, 173 insertions, 74 deletions
diff --git a/src/tools/clippy/clippy_lints/src/functions/mod.rs b/src/tools/clippy/clippy_lints/src/functions/mod.rs index 73261fb8a..90911e0bf 100644 --- a/src/tools/clippy/clippy_lints/src/functions/mod.rs +++ b/src/tools/clippy/clippy_lints/src/functions/mod.rs @@ -1,6 +1,6 @@ mod must_use; mod not_unsafe_ptr_arg_deref; -mod result_unit_err; +mod result; mod too_many_arguments; mod too_many_lines; @@ -217,17 +217,62 @@ declare_clippy_lint! { "public function returning `Result` with an `Err` type of `()`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for functions that return `Result` with an unusually large + /// `Err`-variant. + /// + /// ### Why is this bad? + /// A `Result` is at least as large as the `Err`-variant. While we + /// expect that variant to be seldomly used, the compiler needs to reserve + /// and move that much memory every single time. + /// + /// ### Known problems + /// The size determined by Clippy is platform-dependent. + /// + /// ### Examples + /// ```rust + /// pub enum ParseError { + /// UnparsedBytes([u8; 512]), + /// UnexpectedEof, + /// } + /// + /// // The `Result` has at least 512 bytes, even in the `Ok`-case + /// pub fn parse() -> Result<(), ParseError> { + /// Ok(()) + /// } + /// ``` + /// should be + /// ``` + /// pub enum ParseError { + /// UnparsedBytes(Box<[u8; 512]>), + /// UnexpectedEof, + /// } + /// + /// // The `Result` is slightly larger than a pointer + /// pub fn parse() -> Result<(), ParseError> { + /// Ok(()) + /// } + /// ``` + #[clippy::version = "1.64.0"] + pub RESULT_LARGE_ERR, + perf, + "function returning `Result` with large `Err` type" +} + #[derive(Copy, Clone)] pub struct Functions { too_many_arguments_threshold: u64, too_many_lines_threshold: u64, + large_error_threshold: u64, } impl Functions { - pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64) -> Self { + pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64, large_error_threshold: u64) -> Self { Self { too_many_arguments_threshold, too_many_lines_threshold, + large_error_threshold, } } } @@ -240,6 +285,7 @@ impl_lint_pass!(Functions => [ DOUBLE_MUST_USE, MUST_USE_CANDIDATE, RESULT_UNIT_ERR, + RESULT_LARGE_ERR, ]); impl<'tcx> LateLintPass<'tcx> for Functions { @@ -259,18 +305,18 @@ impl<'tcx> LateLintPass<'tcx> for Functions { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { must_use::check_item(cx, item); - result_unit_err::check_item(cx, item); + result::check_item(cx, item, self.large_error_threshold); } fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) { must_use::check_impl_item(cx, item); - result_unit_err::check_impl_item(cx, item); + result::check_impl_item(cx, item, self.large_error_threshold); } fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { too_many_arguments::check_trait_item(cx, item, self.too_many_arguments_threshold); not_unsafe_ptr_arg_deref::check_trait_item(cx, item); must_use::check_trait_item(cx, item); - result_unit_err::check_trait_item(cx, item); + result::check_trait_item(cx, item, self.large_error_threshold); } } diff --git a/src/tools/clippy/clippy_lints/src/functions/must_use.rs b/src/tools/clippy/clippy_lints/src/functions/must_use.rs index 6672a6cb0..00a493776 100644 --- a/src/tools/clippy/clippy_lints/src/functions/must_use.rs +++ b/src/tools/clippy/clippy_lints/src/functions/must_use.rs @@ -212,7 +212,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> { return; } match expr.kind { - Call(_, args) | MethodCall(_, args, _) => { + Call(_, args) => { let mut tys = DefIdSet::default(); for arg in args { if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) @@ -230,6 +230,24 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> { tys.clear(); } }, + MethodCall(_, receiver, args, _) => { + let mut tys = DefIdSet::default(); + for arg in std::iter::once(receiver).chain(args.iter()) { + if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) + && is_mutable_ty( + self.cx, + self.cx.tcx.typeck(arg.hir_id.owner).expr_ty(arg), + arg.span, + &mut tys, + ) + && is_mutated_static(arg) + { + self.mutates_static = true; + return; + } + tys.clear(); + } + }, Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target) => { self.mutates_static |= is_mutated_static(target); }, @@ -254,6 +272,6 @@ fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bo cx, mutates_static: false, }; - intravisit::walk_expr(&mut v, &body.value); + intravisit::walk_expr(&mut v, body.value); v.mutates_static } diff --git a/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs b/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs index 565a1c871..3bbfa52e8 100644 --- a/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs +++ b/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs @@ -88,11 +88,12 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { } } }, - hir::ExprKind::MethodCall(_, args, _) => { + hir::ExprKind::MethodCall(_, receiver, args, _) => { let def_id = self.typeck_results.type_dependent_def_id(expr.hir_id).unwrap(); let base_type = self.cx.tcx.type_of(def_id); if type_is_unsafe_function(self.cx, base_type) { + self.check_arg(receiver); for arg in args { self.check_arg(arg); } diff --git a/src/tools/clippy/clippy_lints/src/functions/result.rs b/src/tools/clippy/clippy_lints/src/functions/result.rs new file mode 100644 index 000000000..9591405cb --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/functions/result.rs @@ -0,0 +1,100 @@ +use rustc_errors::Diagnostic; +use rustc_hir as hir; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::{self, Ty}; +use rustc_span::{sym, Span}; + +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then}; +use clippy_utils::trait_ref_of_method; +use clippy_utils::ty::{approx_ty_size, is_type_diagnostic_item}; + +use super::{RESULT_LARGE_ERR, RESULT_UNIT_ERR}; + +/// The type of the `Err`-variant in a `std::result::Result` returned by the +/// given `FnDecl` +fn result_err_ty<'tcx>( + cx: &LateContext<'tcx>, + decl: &hir::FnDecl<'tcx>, + id: hir::def_id::LocalDefId, + item_span: Span, +) -> Option<(&'tcx hir::Ty<'tcx>, Ty<'tcx>)> { + if !in_external_macro(cx.sess(), item_span) + && let hir::FnRetTy::Return(hir_ty) = decl.output + && let ty = cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(id).output()) + && is_type_diagnostic_item(cx, ty, sym::Result) + && let ty::Adt(_, substs) = ty.kind() + { + let err_ty = substs.type_at(1); + Some((hir_ty, err_ty)) + } else { + None + } +} + +pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &hir::Item<'tcx>, large_err_threshold: u64) { + if let hir::ItemKind::Fn(ref sig, _generics, _) = item.kind + && let Some((hir_ty, err_ty)) = result_err_ty(cx, sig.decl, item.def_id, item.span) + { + if cx.access_levels.is_exported(item.def_id) { + let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); + check_result_unit_err(cx, err_ty, fn_header_span); + } + check_result_large_err(cx, err_ty, hir_ty.span, large_err_threshold); + } +} + +pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &hir::ImplItem<'tcx>, large_err_threshold: u64) { + // Don't lint if method is a trait's implementation, we can't do anything about those + if let hir::ImplItemKind::Fn(ref sig, _) = item.kind + && let Some((hir_ty, err_ty)) = result_err_ty(cx, sig.decl, item.def_id, item.span) + && trait_ref_of_method(cx, item.def_id).is_none() + { + if cx.access_levels.is_exported(item.def_id) { + let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); + check_result_unit_err(cx, err_ty, fn_header_span); + } + check_result_large_err(cx, err_ty, hir_ty.span, large_err_threshold); + } +} + +pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &hir::TraitItem<'tcx>, large_err_threshold: u64) { + if let hir::TraitItemKind::Fn(ref sig, _) = item.kind { + let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); + if let Some((hir_ty, err_ty)) = result_err_ty(cx, sig.decl, item.def_id, item.span) { + if cx.access_levels.is_exported(item.def_id) { + check_result_unit_err(cx, err_ty, fn_header_span); + } + check_result_large_err(cx, err_ty, hir_ty.span, large_err_threshold); + } + } +} + +fn check_result_unit_err(cx: &LateContext<'_>, err_ty: Ty<'_>, fn_header_span: Span) { + if err_ty.is_unit() { + span_lint_and_help( + cx, + RESULT_UNIT_ERR, + fn_header_span, + "this returns a `Result<_, ()>`", + None, + "use a custom `Error` type instead", + ); + } +} + +fn check_result_large_err<'tcx>(cx: &LateContext<'tcx>, err_ty: Ty<'tcx>, hir_ty_span: Span, large_err_threshold: u64) { + let ty_size = approx_ty_size(cx, err_ty); + if ty_size >= large_err_threshold { + span_lint_and_then( + cx, + RESULT_LARGE_ERR, + hir_ty_span, + "the `Err`-variant returned from this function is very large", + |diag: &mut Diagnostic| { + diag.span_label(hir_ty_span, format!("the `Err`-variant is at least {ty_size} bytes")); + diag.help(format!("try reducing the size of `{err_ty}`, for example by boxing large elements or replacing it with `Box<{err_ty}>`")); + }, + ); + } +} diff --git a/src/tools/clippy/clippy_lints/src/functions/result_unit_err.rs b/src/tools/clippy/clippy_lints/src/functions/result_unit_err.rs deleted file mode 100644 index 2e63a1f92..000000000 --- a/src/tools/clippy/clippy_lints/src/functions/result_unit_err.rs +++ /dev/null @@ -1,66 +0,0 @@ -use rustc_hir as hir; -use rustc_lint::{LateContext, LintContext}; -use rustc_middle::lint::in_external_macro; -use rustc_middle::ty; -use rustc_span::{sym, Span}; -use rustc_typeck::hir_ty_to_ty; - -use if_chain::if_chain; - -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::trait_ref_of_method; -use clippy_utils::ty::is_type_diagnostic_item; - -use super::RESULT_UNIT_ERR; - -pub(super) fn check_item(cx: &LateContext<'_>, item: &hir::Item<'_>) { - if let hir::ItemKind::Fn(ref sig, _generics, _) = item.kind { - let is_public = cx.access_levels.is_exported(item.def_id); - let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); - if is_public { - check_result_unit_err(cx, sig.decl, item.span, fn_header_span); - } - } -} - -pub(super) fn check_impl_item(cx: &LateContext<'_>, item: &hir::ImplItem<'_>) { - if let hir::ImplItemKind::Fn(ref sig, _) = item.kind { - let is_public = cx.access_levels.is_exported(item.def_id); - let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); - if is_public && trait_ref_of_method(cx, item.def_id).is_none() { - check_result_unit_err(cx, sig.decl, item.span, fn_header_span); - } - } -} - -pub(super) fn check_trait_item(cx: &LateContext<'_>, item: &hir::TraitItem<'_>) { - if let hir::TraitItemKind::Fn(ref sig, _) = item.kind { - let is_public = cx.access_levels.is_exported(item.def_id); - let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); - if is_public { - check_result_unit_err(cx, sig.decl, item.span, fn_header_span); - } - } -} - -fn check_result_unit_err(cx: &LateContext<'_>, decl: &hir::FnDecl<'_>, item_span: Span, fn_header_span: Span) { - if_chain! { - if !in_external_macro(cx.sess(), item_span); - if let hir::FnRetTy::Return(ty) = decl.output; - let ty = hir_ty_to_ty(cx.tcx, ty); - if is_type_diagnostic_item(cx, ty, sym::Result); - if let ty::Adt(_, substs) = ty.kind(); - let err_ty = substs.type_at(1); - if err_ty.is_unit(); - then { - span_lint_and_help( - cx, - RESULT_UNIT_ERR, - fn_header_span, - "this returns a `Result<_, ()>`", - None, - "use a custom `Error` type instead", - ); - } - } -} |