dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Initial implementation of `result_large_err`

This commit is contained in:
Lukas Lueg 2022-08-24 23:11:19 +02:00
parent 4df6032100
commit 66a97055b2
13 changed files with 401 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -4025,6 +4025,7 @@ Released 2018-09-13
[`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
[`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_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

56
clippy_lints/src/functions/mod.rs
View File

@ -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);
}
}

100
clippy_lints/src/functions/result.rs Normal file
View 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}>`"));
},
);
}
}

66
clippy_lints/src/functions/result_unit_err.rs
View File

@ -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",
);
}
}
}

1
clippy_lints/src/lib.register_all.rs
View File

@ -80,6 +80,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(functions::DOUBLE_MUST_USE),
LintId::of(functions::MUST_USE_UNIT),
LintId::of(functions::NOT_UNSAFE_PTR_ARG_DEREF),
LintId::of(functions::RESULT_LARGE_ERR),
LintId::of(functions::RESULT_UNIT_ERR),
LintId::of(functions::TOO_MANY_ARGUMENTS),
LintId::of(if_let_mutex::IF_LET_MUTEX),

1
clippy_lints/src/lib.register_lints.rs
View File

@ -171,6 +171,7 @@ store.register_lints(&[
functions::MUST_USE_CANDIDATE,
functions::MUST_USE_UNIT,
functions::NOT_UNSAFE_PTR_ARG_DEREF,
functions::RESULT_LARGE_ERR,
functions::RESULT_UNIT_ERR,
functions::TOO_MANY_ARGUMENTS,
functions::TOO_MANY_LINES,

1
clippy_lints/src/lib.register_perf.rs
View File

@ -7,6 +7,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![
LintId::of(escape::BOXED_LOCAL),
LintId::of(format_args::FORMAT_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_enum_variant::LARGE_ENUM_VARIANT),
LintId::of(loops::MANUAL_MEMCPY),

2
clippy_lints/src/lib.rs
View File

@ -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())));
let too_many_arguments_threshold = conf.too_many_arguments_threshold;
let too_many_lines_threshold = conf.too_many_lines_threshold;
let large_error_threshold = conf.large_error_threshold;
store.register_late_pass(move || {
Box::new(functions::Functions::new(
too_many_arguments_threshold,
too_many_lines_threshold,
large_error_threshold,
))
});
let doc_valid_idents = conf.doc_valid_idents.iter().cloned().collect::<FxHashSet<_>>();

4
clippy_lints/src/utils/conf.rs
View File

@ -379,6 +379,10 @@ define_Conf! {
///
/// Whether `dbg!` should be allowed in test functions
(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.

50
clippy_utils/src/ty.rs
View File

@ -831,3 +831,53 @@ pub fn ty_is_fn_once_param<'tcx>(tcx: TyCtxt<'_>, ty: Ty<'tcx>, predicates: &'tc
})
.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,
}
}

1
tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
View File

@ -19,6 +19,7 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown fie
enforced-import-renames
enum-variant-name-threshold
enum-variant-size-threshold
large-error-threshold
literal-representation-threshold
max-fn-params-bools
max-include-file-size

98
tests/ui/result_large_err.rs Normal file
View 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
View 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