Auto merge of #9373 - lukaslueg:result_large_err, r=Alexendoo
Initial implementation `result_large_err` This is a shot at #6560, #4652, and #3884. The lint checks for `Result` being returned from functions/methods where the `Err` variant is larger than a configurable threshold (the default of which is 128 bytes). There has been some discussion around this, which I'll try to quickly summarize: * A large `Err`-variant may force an equally large `Result` if `Err` is actually bigger than `Ok`. * There is a cost involved in large `Result`, as LLVM may choose to `memcpy` them around above a certain size. * We usually expect the `Err` variant to be seldomly used, but pay the cost every time. * `Result` returned from library code has a high chance of bubbling up the call stack, getting stuffed into `MyLibError { IoError(std::io::Error), ParseError(parselib::Error), ...}`, exacerbating the problem. This PR deliberately does not take into account comparing the `Ok` to the `Err` variant (e.g. a ratio, or one being larger than the other). Rather we choose an absolute threshold for `Err`'s size, above which we warn. The reason for this is that `Err`s probably get `map_err`'ed further up the call stack, and we can't draw conclusions from the ratio at the point where the `Result` is returned. A relative threshold would also be less predictable, while not accounting for the cost of LLVM being forced to generate less efficient code if the `Err`-variant is _large_ in absolute terms. We lint private functions as well as public functions, as the perf-cost applies to in-crate code as well. In order to account for type-parameters, I conjured up `fn approx_ty_size`. The function relies on `LateContext::layout_of` to compute the actual size, and in case of failure (e.g. due to generics) tries to come up with an "at least size". In the latter case, the size of obviously wrong, but the inspected size certainly can't be smaller than that. Please give the approach a heavy dose of review, as I'm not actually familiar with the type-system at all (read: I have no idea what I'm doing). The approach does, however flimsy it is, allow us to successfully lint situations like ```rust pub union UnionError<T: Copy> { _maybe: T, _or_perhaps_even: (T, [u8; 512]), } // We know `UnionError<T>` will be at least 512 bytes, no matter what `T` is pub fn param_large_union<T: Copy>() -> Result<(), UnionError<T>> { Ok(()) } ``` I've given some refactoring to `functions/result_unit_err.rs` to re-use some bits. This is also the groundwork for #6409 The default threshold is 128 because of https://github.com/rust-lang/rust-clippy/issues/4652#issue-505670554 `lintcheck` does not trigger this lint for a threshold of 128. It does warn for 64, though. The suggestion currently is the following, which is just a placeholder for discussion to be had. I did have the computed size in a `span_label`. However, that might cause both ui-tests here and lints elsewhere to become flaky wrt to their output (as the size is platform dependent). ``` error: the `Err`-variant returned via this `Result` is very large --> $DIR/result_large_err.rs:36:34 | LL | pub fn param_large_error<R>() -> Result<(), (u128, R, FullyDefinedLargeError)> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The `Err` variant is unusually large, at least 128 bytes ``` changelog: Add [`result_large_err`] lint
This commit is contained in:
commit
09e4659a86
@ -4025,6 +4025,7 @@ Released 2018-09-13
|
|||||||
[`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
|
[`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
|
||||||
[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
|
[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
|
||||||
[`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
|
[`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
|
||||||
|
[`result_large_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err
|
||||||
[`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option
|
[`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option
|
||||||
[`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
|
[`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
|
||||||
[`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
|
[`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
|
||||||
|
@ -1,6 +1,6 @@
|
|||||||
mod must_use;
|
mod must_use;
|
||||||
mod not_unsafe_ptr_arg_deref;
|
mod not_unsafe_ptr_arg_deref;
|
||||||
mod result_unit_err;
|
mod result;
|
||||||
mod too_many_arguments;
|
mod too_many_arguments;
|
||||||
mod too_many_lines;
|
mod too_many_lines;
|
||||||
|
|
||||||
@ -217,17 +217,62 @@
|
|||||||
"public function returning `Result` with an `Err` type of `()`"
|
"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)]
|
#[derive(Copy, Clone)]
|
||||||
pub struct Functions {
|
pub struct Functions {
|
||||||
too_many_arguments_threshold: u64,
|
too_many_arguments_threshold: u64,
|
||||||
too_many_lines_threshold: u64,
|
too_many_lines_threshold: u64,
|
||||||
|
large_error_threshold: u64,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Functions {
|
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 {
|
Self {
|
||||||
too_many_arguments_threshold,
|
too_many_arguments_threshold,
|
||||||
too_many_lines_threshold,
|
too_many_lines_threshold,
|
||||||
|
large_error_threshold,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -240,6 +285,7 @@ pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64) ->
|
|||||||
DOUBLE_MUST_USE,
|
DOUBLE_MUST_USE,
|
||||||
MUST_USE_CANDIDATE,
|
MUST_USE_CANDIDATE,
|
||||||
RESULT_UNIT_ERR,
|
RESULT_UNIT_ERR,
|
||||||
|
RESULT_LARGE_ERR,
|
||||||
]);
|
]);
|
||||||
|
|
||||||
impl<'tcx> LateLintPass<'tcx> for Functions {
|
impl<'tcx> LateLintPass<'tcx> for Functions {
|
||||||
@ -259,18 +305,18 @@ fn check_fn(
|
|||||||
|
|
||||||
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
|
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
|
||||||
must_use::check_item(cx, 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<'_>) {
|
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) {
|
||||||
must_use::check_impl_item(cx, item);
|
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<'_>) {
|
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);
|
too_many_arguments::check_trait_item(cx, item, self.too_many_arguments_threshold);
|
||||||
not_unsafe_ptr_arg_deref::check_trait_item(cx, item);
|
not_unsafe_ptr_arg_deref::check_trait_item(cx, item);
|
||||||
must_use::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);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
100
clippy_lints/src/functions/result.rs
Normal file
100
clippy_lints/src/functions/result.rs
Normal file
@ -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 rustc_typeck::hir_ty_to_ty;
|
||||||
|
|
||||||
|
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>,
|
||||||
|
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 = hir_ty_to_ty(cx.tcx, hir_ty)
|
||||||
|
&& 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.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.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.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}>`"));
|
||||||
|
},
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
@ -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",
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
@ -80,6 +80,7 @@
|
|||||||
LintId::of(functions::DOUBLE_MUST_USE),
|
LintId::of(functions::DOUBLE_MUST_USE),
|
||||||
LintId::of(functions::MUST_USE_UNIT),
|
LintId::of(functions::MUST_USE_UNIT),
|
||||||
LintId::of(functions::NOT_UNSAFE_PTR_ARG_DEREF),
|
LintId::of(functions::NOT_UNSAFE_PTR_ARG_DEREF),
|
||||||
|
LintId::of(functions::RESULT_LARGE_ERR),
|
||||||
LintId::of(functions::RESULT_UNIT_ERR),
|
LintId::of(functions::RESULT_UNIT_ERR),
|
||||||
LintId::of(functions::TOO_MANY_ARGUMENTS),
|
LintId::of(functions::TOO_MANY_ARGUMENTS),
|
||||||
LintId::of(if_let_mutex::IF_LET_MUTEX),
|
LintId::of(if_let_mutex::IF_LET_MUTEX),
|
||||||
|
@ -171,6 +171,7 @@
|
|||||||
functions::MUST_USE_CANDIDATE,
|
functions::MUST_USE_CANDIDATE,
|
||||||
functions::MUST_USE_UNIT,
|
functions::MUST_USE_UNIT,
|
||||||
functions::NOT_UNSAFE_PTR_ARG_DEREF,
|
functions::NOT_UNSAFE_PTR_ARG_DEREF,
|
||||||
|
functions::RESULT_LARGE_ERR,
|
||||||
functions::RESULT_UNIT_ERR,
|
functions::RESULT_UNIT_ERR,
|
||||||
functions::TOO_MANY_ARGUMENTS,
|
functions::TOO_MANY_ARGUMENTS,
|
||||||
functions::TOO_MANY_LINES,
|
functions::TOO_MANY_LINES,
|
||||||
|
@ -7,6 +7,7 @@
|
|||||||
LintId::of(escape::BOXED_LOCAL),
|
LintId::of(escape::BOXED_LOCAL),
|
||||||
LintId::of(format_args::FORMAT_IN_FORMAT_ARGS),
|
LintId::of(format_args::FORMAT_IN_FORMAT_ARGS),
|
||||||
LintId::of(format_args::TO_STRING_IN_FORMAT_ARGS),
|
LintId::of(format_args::TO_STRING_IN_FORMAT_ARGS),
|
||||||
|
LintId::of(functions::RESULT_LARGE_ERR),
|
||||||
LintId::of(large_const_arrays::LARGE_CONST_ARRAYS),
|
LintId::of(large_const_arrays::LARGE_CONST_ARRAYS),
|
||||||
LintId::of(large_enum_variant::LARGE_ENUM_VARIANT),
|
LintId::of(large_enum_variant::LARGE_ENUM_VARIANT),
|
||||||
LintId::of(loops::MANUAL_MEMCPY),
|
LintId::of(loops::MANUAL_MEMCPY),
|
||||||
|
@ -668,10 +668,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||||||
store.register_late_pass(move || Box::new(disallowed_names::DisallowedNames::new(disallowed_names.clone())));
|
store.register_late_pass(move || Box::new(disallowed_names::DisallowedNames::new(disallowed_names.clone())));
|
||||||
let too_many_arguments_threshold = conf.too_many_arguments_threshold;
|
let too_many_arguments_threshold = conf.too_many_arguments_threshold;
|
||||||
let too_many_lines_threshold = conf.too_many_lines_threshold;
|
let too_many_lines_threshold = conf.too_many_lines_threshold;
|
||||||
|
let large_error_threshold = conf.large_error_threshold;
|
||||||
store.register_late_pass(move || {
|
store.register_late_pass(move || {
|
||||||
Box::new(functions::Functions::new(
|
Box::new(functions::Functions::new(
|
||||||
too_many_arguments_threshold,
|
too_many_arguments_threshold,
|
||||||
too_many_lines_threshold,
|
too_many_lines_threshold,
|
||||||
|
large_error_threshold,
|
||||||
))
|
))
|
||||||
});
|
});
|
||||||
let doc_valid_idents = conf.doc_valid_idents.iter().cloned().collect::<FxHashSet<_>>();
|
let doc_valid_idents = conf.doc_valid_idents.iter().cloned().collect::<FxHashSet<_>>();
|
||||||
|
@ -379,6 +379,10 @@ pub(crate) fn get_configuration_metadata() -> Vec<ClippyConfiguration> {
|
|||||||
///
|
///
|
||||||
/// Whether `dbg!` should be allowed in test functions
|
/// Whether `dbg!` should be allowed in test functions
|
||||||
(allow_dbg_in_tests: bool = false),
|
(allow_dbg_in_tests: bool = false),
|
||||||
|
/// Lint: RESULT_LARGE_ERR
|
||||||
|
///
|
||||||
|
/// The maximum size of the `Err`-variant in a `Result` returned from a function
|
||||||
|
(large_error_threshold: u64 = 128),
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Search for the configuration file.
|
/// Search for the configuration file.
|
||||||
|
@ -831,3 +831,53 @@ pub fn ty_is_fn_once_param<'tcx>(tcx: TyCtxt<'_>, ty: Ty<'tcx>, predicates: &'tc
|
|||||||
})
|
})
|
||||||
.unwrap_or(false)
|
.unwrap_or(false)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Comes up with an "at least" guesstimate for the type's size, not taking into
|
||||||
|
/// account the layout of type parameters.
|
||||||
|
pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
|
||||||
|
use rustc_middle::ty::layout::LayoutOf;
|
||||||
|
if !is_normalizable(cx, cx.param_env, ty) {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
match (cx.layout_of(ty).map(|layout| layout.size.bytes()), ty.kind()) {
|
||||||
|
(Ok(size), _) => size,
|
||||||
|
(Err(_), ty::Tuple(list)) => list.as_substs().types().map(|t| approx_ty_size(cx, t)).sum(),
|
||||||
|
(Err(_), ty::Array(t, n)) => {
|
||||||
|
n.try_eval_usize(cx.tcx, cx.param_env).unwrap_or_default() * approx_ty_size(cx, *t)
|
||||||
|
},
|
||||||
|
(Err(_), ty::Adt(def, subst)) if def.is_struct() => def
|
||||||
|
.variants()
|
||||||
|
.iter()
|
||||||
|
.map(|v| {
|
||||||
|
v.fields
|
||||||
|
.iter()
|
||||||
|
.map(|field| approx_ty_size(cx, field.ty(cx.tcx, subst)))
|
||||||
|
.sum::<u64>()
|
||||||
|
})
|
||||||
|
.sum(),
|
||||||
|
(Err(_), ty::Adt(def, subst)) if def.is_enum() => def
|
||||||
|
.variants()
|
||||||
|
.iter()
|
||||||
|
.map(|v| {
|
||||||
|
v.fields
|
||||||
|
.iter()
|
||||||
|
.map(|field| approx_ty_size(cx, field.ty(cx.tcx, subst)))
|
||||||
|
.sum::<u64>()
|
||||||
|
})
|
||||||
|
.max()
|
||||||
|
.unwrap_or_default(),
|
||||||
|
(Err(_), ty::Adt(def, subst)) if def.is_union() => def
|
||||||
|
.variants()
|
||||||
|
.iter()
|
||||||
|
.map(|v| {
|
||||||
|
v.fields
|
||||||
|
.iter()
|
||||||
|
.map(|field| approx_ty_size(cx, field.ty(cx.tcx, subst)))
|
||||||
|
.max()
|
||||||
|
.unwrap_or_default()
|
||||||
|
})
|
||||||
|
.max()
|
||||||
|
.unwrap_or_default(),
|
||||||
|
(Err(_), _) => 0,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@ -19,6 +19,7 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown fie
|
|||||||
enforced-import-renames
|
enforced-import-renames
|
||||||
enum-variant-name-threshold
|
enum-variant-name-threshold
|
||||||
enum-variant-size-threshold
|
enum-variant-size-threshold
|
||||||
|
large-error-threshold
|
||||||
literal-representation-threshold
|
literal-representation-threshold
|
||||||
max-fn-params-bools
|
max-fn-params-bools
|
||||||
max-include-file-size
|
max-include-file-size
|
||||||
|
98
tests/ui/result_large_err.rs
Normal file
98
tests/ui/result_large_err.rs
Normal file
@ -0,0 +1,98 @@
|
|||||||
|
#![warn(clippy::result_large_err)]
|
||||||
|
|
||||||
|
pub fn small_err() -> Result<(), u128> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn large_err() -> Result<(), [u8; 512]> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
pub struct FullyDefinedLargeError {
|
||||||
|
_foo: u128,
|
||||||
|
_bar: [u8; 100],
|
||||||
|
_foobar: [u8; 120],
|
||||||
|
}
|
||||||
|
|
||||||
|
impl FullyDefinedLargeError {
|
||||||
|
pub fn ret() -> Result<(), Self> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn struct_error() -> Result<(), FullyDefinedLargeError> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
type Fdlr<T> = std::result::Result<T, FullyDefinedLargeError>;
|
||||||
|
pub fn large_err_via_type_alias<T>(x: T) -> Fdlr<T> {
|
||||||
|
Ok(x)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn param_small_error<R>() -> Result<(), (R, u128)> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn param_large_error<R>() -> Result<(), (u128, R, FullyDefinedLargeError)> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
pub enum LargeErrorVariants<T> {
|
||||||
|
_Small(u8),
|
||||||
|
_Omg([u8; 512]),
|
||||||
|
_Param(T),
|
||||||
|
}
|
||||||
|
|
||||||
|
impl LargeErrorVariants<()> {
|
||||||
|
pub fn large_enum_error() -> Result<(), Self> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
trait TraitForcesLargeError {
|
||||||
|
fn large_error() -> Result<(), [u8; 512]> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
struct TraitImpl;
|
||||||
|
|
||||||
|
impl TraitForcesLargeError for TraitImpl {
|
||||||
|
// Should not lint
|
||||||
|
fn large_error() -> Result<(), [u8; 512]> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub union FullyDefinedUnionError {
|
||||||
|
_maybe: u8,
|
||||||
|
_or_even: [[u8; 16]; 32],
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn large_union_err() -> Result<(), FullyDefinedUnionError> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
pub union UnionError<T: Copy> {
|
||||||
|
_maybe: T,
|
||||||
|
_or_perhaps_even: (T, [u8; 512]),
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn param_large_union<T: Copy>() -> Result<(), UnionError<T>> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
pub struct ArrayError<T, U> {
|
||||||
|
_large_array: [T; 32],
|
||||||
|
_other_stuff: U,
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn array_error_subst<U>() -> Result<(), ArrayError<i32, U>> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn array_error<T, U>() -> Result<(), ArrayError<(i32, T), U>> {
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn main() {}
|
91
tests/ui/result_large_err.stderr
Normal file
91
tests/ui/result_large_err.stderr
Normal file
@ -0,0 +1,91 @@
|
|||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:7:23
|
||||||
|
|
|
||||||
|
LL | pub fn large_err() -> Result<(), [u8; 512]> {
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 512 bytes
|
||||||
|
|
|
||||||
|
= note: `-D clippy::result-large-err` implied by `-D warnings`
|
||||||
|
= help: try reducing the size of `[u8; 512]`, for example by boxing large elements or replacing it with `Box<[u8; 512]>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:18:21
|
||||||
|
|
|
||||||
|
LL | pub fn ret() -> Result<(), Self> {
|
||||||
|
| ^^^^^^^^^^^^^^^^ the `Err`-variant is at least 240 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `FullyDefinedLargeError`, for example by boxing large elements or replacing it with `Box<FullyDefinedLargeError>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:23:26
|
||||||
|
|
|
||||||
|
LL | pub fn struct_error() -> Result<(), FullyDefinedLargeError> {
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 240 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `FullyDefinedLargeError`, for example by boxing large elements or replacing it with `Box<FullyDefinedLargeError>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:28:45
|
||||||
|
|
|
||||||
|
LL | pub fn large_err_via_type_alias<T>(x: T) -> Fdlr<T> {
|
||||||
|
| ^^^^^^^ the `Err`-variant is at least 240 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `FullyDefinedLargeError`, for example by boxing large elements or replacing it with `Box<FullyDefinedLargeError>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:36:34
|
||||||
|
|
|
||||||
|
LL | pub fn param_large_error<R>() -> Result<(), (u128, R, FullyDefinedLargeError)> {
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 256 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `(u128, R, FullyDefinedLargeError)`, for example by boxing large elements or replacing it with `Box<(u128, R, FullyDefinedLargeError)>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:47:34
|
||||||
|
|
|
||||||
|
LL | pub fn large_enum_error() -> Result<(), Self> {
|
||||||
|
| ^^^^^^^^^^^^^^^^ the `Err`-variant is at least 513 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `LargeErrorVariants<()>`, for example by boxing large elements or replacing it with `Box<LargeErrorVariants<()>>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:53:25
|
||||||
|
|
|
||||||
|
LL | fn large_error() -> Result<(), [u8; 512]> {
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 512 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `[u8; 512]`, for example by boxing large elements or replacing it with `Box<[u8; 512]>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:72:29
|
||||||
|
|
|
||||||
|
LL | pub fn large_union_err() -> Result<(), FullyDefinedUnionError> {
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 512 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `FullyDefinedUnionError`, for example by boxing large elements or replacing it with `Box<FullyDefinedUnionError>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:81:40
|
||||||
|
|
|
||||||
|
LL | pub fn param_large_union<T: Copy>() -> Result<(), UnionError<T>> {
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 512 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `UnionError<T>`, for example by boxing large elements or replacing it with `Box<UnionError<T>>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:90:34
|
||||||
|
|
|
||||||
|
LL | pub fn array_error_subst<U>() -> Result<(), ArrayError<i32, U>> {
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 128 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `ArrayError<i32, U>`, for example by boxing large elements or replacing it with `Box<ArrayError<i32, U>>`
|
||||||
|
|
||||||
|
error: the `Err`-variant returned from this function is very large
|
||||||
|
--> $DIR/result_large_err.rs:94:31
|
||||||
|
|
|
||||||
|
LL | pub fn array_error<T, U>() -> Result<(), ArrayError<(i32, T), U>> {
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the `Err`-variant is at least 128 bytes
|
||||||
|
|
|
||||||
|
= help: try reducing the size of `ArrayError<(i32, T), U>`, for example by boxing large elements or replacing it with `Box<ArrayError<(i32, T), U>>`
|
||||||
|
|
||||||
|
error: aborting due to 11 previous errors
|
||||||
|
|
Loading…
Reference in New Issue
Block a user