From cf88b4969835c000f9d6988b1755c04162a8e493 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 8 Aug 2024 18:37:43 +0200 Subject: [PATCH] add `call_missing_target_feature` lint --- CHANGELOG.md | 1 + .../src/call_missing_target_feature.rs | 141 ++++++++++++++++++ clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + tests/ui/call_missing_target_feature.fixed | 42 ++++++ tests/ui/call_missing_target_feature.rs | 38 +++++ tests/ui/call_missing_target_feature.stderr | 52 +++++++ 7 files changed, 277 insertions(+) create mode 100644 clippy_lints/src/call_missing_target_feature.rs create mode 100644 tests/ui/call_missing_target_feature.fixed create mode 100644 tests/ui/call_missing_target_feature.rs create mode 100644 tests/ui/call_missing_target_feature.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index cc966972939a..c8373bf73ac2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5417,6 +5417,7 @@ Released 2018-09-13 [`byte_char_slices`]: https://rust-lang.github.io/rust-clippy/master/index.html#byte_char_slices [`bytes_count_to_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#bytes_count_to_len [`bytes_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#bytes_nth +[`call_missing_target_feature`]: https://rust-lang.github.io/rust-clippy/master/index.html#call_missing_target_feature [`cargo_common_metadata`]: https://rust-lang.github.io/rust-clippy/master/index.html#cargo_common_metadata [`case_sensitive_file_extension_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#case_sensitive_file_extension_comparisons [`cast_abs_to_unsigned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_abs_to_unsigned diff --git a/clippy_lints/src/call_missing_target_feature.rs b/clippy_lints/src/call_missing_target_feature.rs new file mode 100644 index 000000000000..9af8ea5fac82 --- /dev/null +++ b/clippy_lints/src/call_missing_target_feature.rs @@ -0,0 +1,141 @@ +#![allow(clippy::similar_names)] +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_hir as hir; +use rustc_hir::def::Res; +use rustc_hir::def_id::DefId; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::lint::in_external_macro; +use rustc_session::declare_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// Checks that the caller enables the target features that the callee requires + /// + /// ### Why is this bad? + /// Not enabling target features can cause UB and limits optimization opportunities. + /// + /// ### Example + /// ```no_run + /// #[target_feature(enable = "avx2")] + /// unsafe fn f() -> u32 { + /// 0 + /// } + /// + /// fn g() { + /// unsafe { f() }; + /// // g does not enable the target features f requires + /// } + /// ``` + #[clippy::version = "1.82.0"] + pub CALL_MISSING_TARGET_FEATURE, + suspicious, + "call requires target features that the surrounding function does not enable" +} + +declare_lint_pass!(CallMissingTargetFeature => [CALL_MISSING_TARGET_FEATURE]); + +impl<'tcx> LateLintPass<'tcx> for CallMissingTargetFeature { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'tcx>) { + let Some(callee_def_id) = callee_def_id(cx, expr) else { + return; + }; + let callee_target_features = &cx.tcx.codegen_fn_attrs(callee_def_id).target_features; + + if callee_target_features.is_empty() { + return; + } + + let Some(caller_body_id) = cx.enclosing_body else { + return; + }; + let caller_def_id = cx.tcx.hir().body_owner_def_id(caller_body_id); + let caller_target_features = &cx.tcx.codegen_fn_attrs(caller_def_id).target_features; + + if in_external_macro(cx.tcx.sess, expr.span) { + return; + } + + // target features can imply other target features (e.g. avx2 implies sse4.2). We can safely skip + // implied target features and only warn for the more general missing target feature. + let missing: Vec<_> = callee_target_features + .iter() + .filter_map(|target_feature| { + if target_feature.implied || caller_target_features.iter().any(|tf| tf.name == target_feature.name) { + None + } else { + Some(target_feature.name.as_str()) + } + }) + .collect(); + + if missing.is_empty() { + return; + } + + let attr = format!("#[target_feature(enable = \"{}\")]", missing.join(",")); + + span_lint_and_then( + cx, + CALL_MISSING_TARGET_FEATURE, + expr.span, + "this call requires target features that the surrounding function does not enable", + |diag| { + diag.span_label( + expr.span, + "this function call requires target features to be enabled".to_string(), + ); + + let fn_sig = cx.tcx.fn_sig(caller_def_id).skip_binder(); + + let mut suggestions = Vec::with_capacity(2); + + let hir::Node::Item(caller_item) = cx.tcx.hir_node_by_def_id(caller_def_id) else { + return; + }; + + let Some(indent) = clippy_utils::source::snippet_indent(cx, caller_item.span) else { + return; + }; + + let lo_span = caller_item.span.with_hi(caller_item.span.lo()); + + match fn_sig.safety() { + hir::Safety::Safe => { + // the `target_feature` attribute can only be applied to unsafe functions + if caller_item.vis_span.is_empty() { + suggestions.push((lo_span, format!("{attr}\n{indent}unsafe "))); + } else { + suggestions.push((lo_span, format!("{attr}\n{indent}"))); + suggestions.push((caller_item.vis_span.shrink_to_hi(), " unsafe".to_string())); + } + }, + hir::Safety::Unsafe => { + suggestions.push((lo_span, format!("{attr}\n{indent}"))); + }, + } + + diag.multipart_suggestion_verbose( + "add the missing target features to the surrounding function", + suggestions, + rustc_errors::Applicability::MaybeIncorrect, + ); + }, + ); + } +} + +fn callee_def_id(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option { + match expr.kind { + hir::ExprKind::Call(path, _) => { + if let hir::ExprKind::Path(ref qpath) = path.kind + && let Res::Def(_, did) = cx.qpath_res(qpath, path.hir_id) + { + Some(did) + } else { + None + } + }, + hir::ExprKind::MethodCall(..) => cx.typeck_results().type_dependent_def_id(expr.hir_id), + _ => None, + } +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 304622afe530..adbe2efc6edc 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -69,6 +69,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::borrow_deref_ref::BORROW_DEREF_REF_INFO, crate::box_default::BOX_DEFAULT_INFO, crate::byte_char_slices::BYTE_CHAR_SLICES_INFO, + crate::call_missing_target_feature::CALL_MISSING_TARGET_FEATURE_INFO, crate::cargo::CARGO_COMMON_METADATA_INFO, crate::cargo::LINT_GROUPS_PRIORITY_INFO, crate::cargo::MULTIPLE_CRATE_VERSIONS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 017e6e2a1423..1d2a6eb7b885 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -92,6 +92,7 @@ mod booleans; mod borrow_deref_ref; mod box_default; mod byte_char_slices; +mod call_missing_target_feature; mod cargo; mod casts; mod cfg_not_test; @@ -784,6 +785,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(floating_point_arithmetic::FloatingPointArithmetic)); store.register_late_pass(|_| Box::new(as_conversions::AsConversions)); store.register_late_pass(|_| Box::new(let_underscore::LetUnderscore)); + store.register_late_pass(|_| Box::new(call_missing_target_feature::CallMissingTargetFeature)); store.register_early_pass(|| Box::::default()); store.register_late_pass(move |_| Box::new(excessive_bools::ExcessiveBools::new(conf))); store.register_early_pass(|| Box::new(option_env_unwrap::OptionEnvUnwrap)); diff --git a/tests/ui/call_missing_target_feature.fixed b/tests/ui/call_missing_target_feature.fixed new file mode 100644 index 000000000000..63113fce1e40 --- /dev/null +++ b/tests/ui/call_missing_target_feature.fixed @@ -0,0 +1,42 @@ +//@only-target: x86_64 +#![allow(clippy::missing_safety_doc)] + +#[target_feature(enable = "avx2")] +pub unsafe fn test_f() { + unsafe { f() }; + //~^ ERROR: this call requires target features +} + +#[target_feature(enable = "avx2,pclmulqdq")] +pub(crate) unsafe fn test_g() { + unsafe { g() }; + //~^ ERROR: this call requires target features +} + +#[target_feature(enable = "avx2,pclmulqdq")] +unsafe fn test_h() { + unsafe { h() }; + //~^ ERROR: this call requires target features +} + +#[target_feature(enable = "avx2,pclmulqdq")] +unsafe fn test_h_unsafe() { + unsafe { h() }; + //~^ ERROR: this call requires target features +} + +#[target_feature(enable = "avx2")] +unsafe fn f() -> u32 { + 0 +} + +#[target_feature(enable = "avx2,pclmulqdq")] +unsafe fn g() -> u32 { + 0 +} + +#[target_feature(enable = "avx2")] +#[target_feature(enable = "pclmulqdq")] +unsafe fn h() -> u32 { + 0 +} diff --git a/tests/ui/call_missing_target_feature.rs b/tests/ui/call_missing_target_feature.rs new file mode 100644 index 000000000000..f9817cec5f18 --- /dev/null +++ b/tests/ui/call_missing_target_feature.rs @@ -0,0 +1,38 @@ +//@only-target: x86_64 +#![allow(clippy::missing_safety_doc)] + +pub fn test_f() { + unsafe { f() }; + //~^ ERROR: this call requires target features +} + +pub(crate) fn test_g() { + unsafe { g() }; + //~^ ERROR: this call requires target features +} + +fn test_h() { + unsafe { h() }; + //~^ ERROR: this call requires target features +} + +unsafe fn test_h_unsafe() { + unsafe { h() }; + //~^ ERROR: this call requires target features +} + +#[target_feature(enable = "avx2")] +unsafe fn f() -> u32 { + 0 +} + +#[target_feature(enable = "avx2,pclmulqdq")] +unsafe fn g() -> u32 { + 0 +} + +#[target_feature(enable = "avx2")] +#[target_feature(enable = "pclmulqdq")] +unsafe fn h() -> u32 { + 0 +} diff --git a/tests/ui/call_missing_target_feature.stderr b/tests/ui/call_missing_target_feature.stderr new file mode 100644 index 000000000000..1f86fdba6794 --- /dev/null +++ b/tests/ui/call_missing_target_feature.stderr @@ -0,0 +1,52 @@ +error: this call requires target features that the surrounding function does not enable + --> tests/ui/call_missing_target_feature.rs:5:14 + | +LL | unsafe { f() }; + | ^^^ this function call requires target features to be enabled + | + = note: `-D clippy::call-missing-target-feature` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::call_missing_target_feature)]` +help: add the missing target features to the surrounding function + | +LL + #[target_feature(enable = "avx2")] +LL ~ pub unsafe fn test_f() { + | + +error: this call requires target features that the surrounding function does not enable + --> tests/ui/call_missing_target_feature.rs:10:14 + | +LL | unsafe { g() }; + | ^^^ this function call requires target features to be enabled + | +help: add the missing target features to the surrounding function + | +LL + #[target_feature(enable = "avx2,pclmulqdq")] +LL ~ pub(crate) unsafe fn test_g() { + | + +error: this call requires target features that the surrounding function does not enable + --> tests/ui/call_missing_target_feature.rs:15:14 + | +LL | unsafe { h() }; + | ^^^ this function call requires target features to be enabled + | +help: add the missing target features to the surrounding function + | +LL + #[target_feature(enable = "avx2,pclmulqdq")] +LL ~ unsafe fn test_h() { + | + +error: this call requires target features that the surrounding function does not enable + --> tests/ui/call_missing_target_feature.rs:20:14 + | +LL | unsafe { h() }; + | ^^^ this function call requires target features to be enabled + | +help: add the missing target features to the surrounding function + | +LL + #[target_feature(enable = "avx2,pclmulqdq")] +LL | unsafe fn test_h_unsafe() { + | + +error: aborting due to 4 previous errors +