From 20f501a9d9f253bfe503388755bdebfe72910e0e Mon Sep 17 00:00:00 2001 From: Niki4tap Date: Sun, 18 Dec 2022 22:39:06 +0300 Subject: [PATCH] Improve code style further --- clippy_lints/src/fn_null_check.rs | 84 +++++++++---------- .../src/transmute/transmute_null_to_fn.rs | 69 ++++++++------- 2 files changed, 76 insertions(+), 77 deletions(-) diff --git a/clippy_lints/src/fn_null_check.rs b/clippy_lints/src/fn_null_check.rs index 6eb5ddd94d7..4f79ce6f8fe 100644 --- a/clippy_lints/src/fn_null_check.rs +++ b/clippy_lints/src/fn_null_check.rs @@ -55,50 +55,50 @@ fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { impl<'tcx> LateLintPass<'tcx> for FnNullCheck { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - // Catching: - // (fn_ptr as * ).is_null() - if let ExprKind::MethodCall(method_name, receiver, _, _) = expr.kind - && method_name.ident.as_str() == "is_null" - && is_fn_ptr_cast(cx, receiver) - { - lint_expr(cx, expr); - return; - } - - if let ExprKind::Binary(op, left, right) = expr.kind - && let BinOpKind::Eq = op.node - { - let to_check: &Expr<'_>; - if is_fn_ptr_cast(cx, left) { - to_check = right; - } else if is_fn_ptr_cast(cx, right) { - to_check = left; - } else { - return; - } - - // Catching: - // (fn_ptr as * ) == - let c = constant(cx, cx.typeck_results(), to_check); - if let Some((Constant::RawPtr(0), _)) = c { - lint_expr(cx, expr); - return; - } - - // Catching: - // (fn_ptr as * ) == (0 as ) - if let ExprKind::Cast(cast_expr, _) = to_check.kind && is_integer_literal(cast_expr, 0) { - lint_expr(cx, expr); - return; - } - - // Catching: - // (fn_ptr as * ) == std::ptr::null() - if let ExprKind::Call(func, []) = to_check.kind && - is_path_diagnostic_item(cx, func, sym::ptr_null) + match expr.kind { + ExprKind::MethodCall(method_name, receiver, _, _) + if method_name.ident.as_str() == "is_null" && is_fn_ptr_cast(cx, receiver) => { lint_expr(cx, expr); - } + }, + + ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq) => { + let to_check: &Expr<'_>; + if is_fn_ptr_cast(cx, left) { + to_check = right; + } else if is_fn_ptr_cast(cx, right) { + to_check = left; + } else { + return; + } + + match to_check.kind { + // Catching: + // (fn_ptr as * ) == (0 as ) + ExprKind::Cast(cast_expr, _) if is_integer_literal(cast_expr, 0) => { + lint_expr(cx, expr); + }, + + // Catching: + // (fn_ptr as * ) == std::ptr::null() + ExprKind::Call(func, []) if is_path_diagnostic_item(cx, func, sym::ptr_null) => { + lint_expr(cx, expr); + }, + + // Catching: + // (fn_ptr as * ) == + _ if matches!( + constant(cx, cx.typeck_results(), to_check), + Some((Constant::RawPtr(0), _)) + ) => + { + lint_expr(cx, expr); + }, + + _ => {}, + } + }, + _ => {}, } } } diff --git a/clippy_lints/src/transmute/transmute_null_to_fn.rs b/clippy_lints/src/transmute/transmute_null_to_fn.rs index 074a5d31763..79d8cb084fd 100644 --- a/clippy_lints/src/transmute/transmute_null_to_fn.rs +++ b/clippy_lints/src/transmute/transmute_null_to_fn.rs @@ -13,6 +13,13 @@ const NOTE_MSG: &str = "this transmute results in undefined behavior"; const HELP_MSG: &str = "try wrapping your function pointer type in `Option` instead, and using `None` as a null pointer value"; +fn lint_expr(cx: &LateContext<'_>, expr: &Expr<'_>) { + span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { + diag.span_label(expr.span, NOTE_MSG); + diag.help(HELP_MSG); + }); +} + pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'tcx Expr<'_>, to_ty: Ty<'tcx>) -> bool { if !to_ty.is_fn() { return false; @@ -21,42 +28,34 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t // Catching: // transmute over constants that resolve to `null`. let mut const_eval_context = constant_context(cx, cx.typeck_results()); - if let ExprKind::Path(ref _qpath) = arg.kind && - let Some(Constant::RawPtr(0)) = const_eval_context.expr(arg) - { - span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { - diag.span_label(expr.span, NOTE_MSG); - diag.help(HELP_MSG); - }); - return true; - } - // Catching: - // `std::mem::transmute(0 as *const i32)` - if let ExprKind::Cast(inner_expr, _cast_ty) = arg.kind && is_integer_literal(inner_expr, 0) { - span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { - diag.span_label(expr.span, NOTE_MSG); - diag.help(HELP_MSG); - }); - return true; - } + match arg.kind { + ExprKind::Path(ref _qpath) if matches!(const_eval_context.expr(arg), Some(Constant::RawPtr(0))) => { + lint_expr(cx, expr); + true + }, - // Catching: - // `std::mem::transmute(std::ptr::null::())` - if let ExprKind::Call(func1, []) = arg.kind && - is_path_diagnostic_item(cx, func1, sym::ptr_null) - { - span_lint_and_then(cx, TRANSMUTE_NULL_TO_FN, expr.span, LINT_MSG, |diag| { - diag.span_label(expr.span, NOTE_MSG); - diag.help(HELP_MSG); - }); - return true; - } + // Catching: + // `std::mem::transmute(0 as *const i32)` + ExprKind::Cast(inner_expr, _cast_ty) if is_integer_literal(inner_expr, 0) => { + lint_expr(cx, expr); + true + }, - // FIXME: - // Also catch transmutations of variables which are known nulls. - // To do this, MIR const propagation seems to be the better tool. - // Whenever MIR const prop routines are more developed, this will - // become available. As of this writing (25/03/19) it is not yet. - false + // Catching: + // `std::mem::transmute(std::ptr::null::())` + ExprKind::Call(func1, []) if is_path_diagnostic_item(cx, func1, sym::ptr_null) => { + lint_expr(cx, expr); + true + }, + + _ => { + // FIXME: + // Also catch transmutations of variables which are known nulls. + // To do this, MIR const propagation seems to be the better tool. + // Whenever MIR const prop routines are more developed, this will + // become available. As of this writing (25/03/19) it is not yet. + false + }, + } }