From 738ed383066a561ae4f3ef0a6a2d570eaba7b6fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Paulo=20Taylor=20Ienczak=20Zanette?= Date: Wed, 7 Oct 2020 21:41:54 -0300 Subject: [PATCH 1/6] clippy_lints: Do not warn against Box parameter in C FFI Fixes #5542. When using C FFI, to handle pointers in parameters it is needed to declare them as `Box` in its Rust-side signature. However, the current linter warns against the usage of Box stating that "local variable doesn't need to be boxed here". This commit fixes it by ignoring functions whose Abi is Cdecl. --- clippy_lints/src/escape.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 82549c12d0a20..ffe81b3966c7c 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -6,6 +6,7 @@ use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; use rustc_target::abi::LayoutOf; +use rustc_target::spec::abi::Abi; use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use crate::utils::span_lint; @@ -60,12 +61,18 @@ impl<'tcx> LateLintPass<'tcx> for BoxedLocal { fn check_fn( &mut self, cx: &LateContext<'tcx>, - _: intravisit::FnKind<'tcx>, + fn_kind: intravisit::FnKind<'tcx>, _: &'tcx FnDecl<'_>, body: &'tcx Body<'_>, _: Span, hir_id: HirId, ) { + if let Some(header) = fn_kind.header() { + if header.abi == Abi::Cdecl { + return; + } + } + // If the method is an impl for a trait, don't warn. let parent_id = cx.tcx.hir().get_parent_item(hir_id); let parent_node = cx.tcx.hir().find(parent_id); From 15150c07eaeec50138a9c96c5bdecbdf92c3101e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Paulo=20Taylor=20Ienczak=20Zanette?= Date: Wed, 7 Oct 2020 22:49:50 -0300 Subject: [PATCH 2/6] clippy_lint: Test for BoxedLocal false-positive in C-FFI and fix C-FFI Abi comparison. --- clippy_lints/src/escape.rs | 2 +- tests/ui/escape_analysis.rs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index ffe81b3966c7c..b222475486d93 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -68,7 +68,7 @@ impl<'tcx> LateLintPass<'tcx> for BoxedLocal { hir_id: HirId, ) { if let Some(header) = fn_kind.header() { - if header.abi == Abi::Cdecl { + if header.abi == Abi::C { return; } } diff --git a/tests/ui/escape_analysis.rs b/tests/ui/escape_analysis.rs index c0a52d832c00a..377a0fb502f46 100644 --- a/tests/ui/escape_analysis.rs +++ b/tests/ui/escape_analysis.rs @@ -174,3 +174,8 @@ mod issue_3739 { }; } } + +/// Issue #5542 +/// +/// This shouldn't warn for `boxed_local` as it is a function implemented in C. +pub extern "C" fn do_now_warn_me(_c_pointer: Box) -> () {} From cc26924cce6d6e6ee7913ba5026d205eefd42702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Paulo=20Taylor=20Ienczak=20Zanette?= Date: Thu, 8 Oct 2020 09:03:11 -0300 Subject: [PATCH 3/6] clippy_lint: Extend BoxedLocal ignored ABI to all non-rust ABIs. --- clippy_lints/src/escape.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index b222475486d93..8b0229125738a 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -68,7 +68,7 @@ impl<'tcx> LateLintPass<'tcx> for BoxedLocal { hir_id: HirId, ) { if let Some(header) = fn_kind.header() { - if header.abi == Abi::C { + if header.abi != Abi::Rust { return; } } From 418cde0389d47628b32e533c47e64874fd1050fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Paulo=20Taylor=20Ienczak=20Zanette?= Date: Thu, 8 Oct 2020 09:06:19 -0300 Subject: [PATCH 4/6] clippy_lint: Fix typo (now -> not) --- tests/ui/escape_analysis.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ui/escape_analysis.rs b/tests/ui/escape_analysis.rs index 377a0fb502f46..3d5a02bf705c0 100644 --- a/tests/ui/escape_analysis.rs +++ b/tests/ui/escape_analysis.rs @@ -177,5 +177,5 @@ mod issue_3739 { /// Issue #5542 /// -/// This shouldn't warn for `boxed_local` as it is a function implemented in C. -pub extern "C" fn do_now_warn_me(_c_pointer: Box) -> () {} +/// This shouldn't warn for `boxed_local` as it is a function to be used in C. +pub extern "C" fn do_not_warn_me(_c_pointer: Box) -> () {} From 5ae0f2644d4c402c8ea3561de1c9fa829b11b40e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Paulo=20Taylor=20Ienczak=20Zanette?= Date: Thu, 8 Oct 2020 09:07:24 -0300 Subject: [PATCH 5/6] clippy_lint: extern definition is in Rust, not C --- tests/ui/escape_analysis.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui/escape_analysis.rs b/tests/ui/escape_analysis.rs index 3d5a02bf705c0..7d3f4011e9996 100644 --- a/tests/ui/escape_analysis.rs +++ b/tests/ui/escape_analysis.rs @@ -177,5 +177,5 @@ mod issue_3739 { /// Issue #5542 /// -/// This shouldn't warn for `boxed_local` as it is a function to be used in C. +/// This shouldn't warn for `boxed_local` as it is intended to called from non-Rust code. pub extern "C" fn do_not_warn_me(_c_pointer: Box) -> () {} From b709b873632dc001b381704c1deb6be702111706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Paulo=20Taylor=20Ienczak=20Zanette?= Date: Thu, 8 Oct 2020 18:50:52 -0300 Subject: [PATCH 6/6] tests: Add test function that does not specify ABI --- tests/ui/escape_analysis.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ui/escape_analysis.rs b/tests/ui/escape_analysis.rs index 7d3f4011e9996..07004489610d0 100644 --- a/tests/ui/escape_analysis.rs +++ b/tests/ui/escape_analysis.rs @@ -179,3 +179,6 @@ mod issue_3739 { /// /// This shouldn't warn for `boxed_local` as it is intended to called from non-Rust code. pub extern "C" fn do_not_warn_me(_c_pointer: Box) -> () {} + +#[rustfmt::skip] // Forces rustfmt to not add ABI +pub extern fn do_not_warn_me_no_abi(_c_pointer: Box) -> () {}