From abb7c31ab038f38e33057062ae8b66b4e3cd699c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 15 Jun 2023 22:04:55 +0200 Subject: [PATCH] Rollup merge of #112517 - fee1-dead-contrib:sus-op-no-borrow, r=compiler-errors `suspicious_double_ref_op`: don't lint on `.borrow()` closes #112489 (cherry picked from commit db7d8374c1b6f1e2e8297f43e6a2cbffeff21882) --- compiler/rustc_lint/messages.ftl | 12 ++-- compiler/rustc_lint/src/lints.rs | 12 ++-- compiler/rustc_lint/src/noop_method_call.rs | 62 +++++++++++---------- tests/ui/lint/issue-112489.rs | 17 ++++++ 4 files changed, 64 insertions(+), 39 deletions(-) create mode 100644 tests/ui/lint/issue-112489.rs diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index d34a3afcba53..0fa67cdb391f 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -463,13 +463,11 @@ lint_requested_level = requested on the command line with `{$level} {$lint_name} lint_supertrait_as_deref_target = `{$t}` implements `Deref` with supertrait `{$target_principal}` as target .label = target type is set here -lint_suspicious_double_ref_op = - using `.{$call}()` on a double reference, which returns `{$ty}` instead of {$op -> - *[should_not_happen] [{$op}] - [deref] dereferencing - [borrow] borrowing - [clone] cloning - } the inner type +lint_suspicious_double_ref_clone = + using `.clone()` on a double reference, which returns `{$ty}` instead of cloning the inner type + +lint_suspicious_double_ref_deref = + using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type lint_trivial_untranslatable_diag = diagnostic with static strings only diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index de1c2be28757..d96723a68eb6 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1188,11 +1188,15 @@ pub struct NoopMethodCallDiag<'a> { } #[derive(LintDiagnostic)] -#[diag(lint_suspicious_double_ref_op)] -pub struct SuspiciousDoubleRefDiag<'a> { - pub call: Symbol, +#[diag(lint_suspicious_double_ref_deref)] +pub struct SuspiciousDoubleRefDerefDiag<'a> { + pub ty: Ty<'a>, +} + +#[derive(LintDiagnostic)] +#[diag(lint_suspicious_double_ref_clone)] +pub struct SuspiciousDoubleRefCloneDiag<'a> { pub ty: Ty<'a>, - pub op: &'static str, } // pass_by_value.rs diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index d054966459d8..d56c35bb677a 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -1,5 +1,7 @@ use crate::context::LintContext; -use crate::lints::{NoopMethodCallDiag, SuspiciousDoubleRefDiag}; +use crate::lints::{ + NoopMethodCallDiag, SuspiciousDoubleRefCloneDiag, SuspiciousDoubleRefDerefDiag, +}; use crate::LateContext; use crate::LateLintPass; use rustc_hir::def::DefKind; @@ -76,22 +78,22 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow` // traits and ignore any other method call. - let did = match cx.typeck_results().type_dependent_def(expr.hir_id) { - // Verify we are dealing with a method/associated function. - Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) { - // Check that we're dealing with a trait method for one of the traits we care about. - Some(trait_id) - if matches!( - cx.tcx.get_diagnostic_name(trait_id), - Some(sym::Borrow | sym::Clone | sym::Deref) - ) => - { - did - } - _ => return, - }, - _ => return, + + let Some((DefKind::AssocFn, did)) = + cx.typeck_results().type_dependent_def(expr.hir_id) + else { + return; + }; + + let Some(trait_id) = cx.tcx.trait_of_item(did) else { return }; + + if !matches!( + cx.tcx.get_diagnostic_name(trait_id), + Some(sym::Borrow | sym::Clone | sym::Deref) + ) { + return; }; + let substs = cx .tcx .normalize_erasing_regions(cx.param_env, cx.typeck_results().node_substs(expr.hir_id)); @@ -102,13 +104,6 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // (Re)check that it implements the noop diagnostic. let Some(name) = cx.tcx.get_diagnostic_name(i.def_id()) else { return }; - let op = match name { - sym::noop_method_borrow => "borrow", - sym::noop_method_clone => "clone", - sym::noop_method_deref => "deref", - _ => return, - }; - let receiver_ty = cx.typeck_results().expr_ty(receiver); let expr_ty = cx.typeck_results().expr_ty_adjusted(expr); let arg_adjustments = cx.typeck_results().expr_adjustments(receiver); @@ -129,11 +124,22 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { NoopMethodCallDiag { method: call.ident.name, receiver_ty, label: span }, ); } else { - cx.emit_spanned_lint( - SUSPICIOUS_DOUBLE_REF_OP, - span, - SuspiciousDoubleRefDiag { call: call.ident.name, ty: expr_ty, op }, - ) + match name { + // If `type_of(x) == T` and `x.borrow()` is used to get `&T`, + // then that should be allowed + sym::noop_method_borrow => return, + sym::noop_method_clone => cx.emit_spanned_lint( + SUSPICIOUS_DOUBLE_REF_OP, + span, + SuspiciousDoubleRefCloneDiag { ty: expr_ty }, + ), + sym::noop_method_deref => cx.emit_spanned_lint( + SUSPICIOUS_DOUBLE_REF_OP, + span, + SuspiciousDoubleRefDerefDiag { ty: expr_ty }, + ), + _ => return, + } } } } diff --git a/tests/ui/lint/issue-112489.rs b/tests/ui/lint/issue-112489.rs new file mode 100644 index 000000000000..559edf0e4f23 --- /dev/null +++ b/tests/ui/lint/issue-112489.rs @@ -0,0 +1,17 @@ +// check-pass +use std::borrow::Borrow; + +struct S; + +trait T: Sized { + fn foo(self) {} +} + +impl T for S {} +impl T for &S {} + +fn main() { + let s = S; + s.borrow().foo(); + s.foo(); +} -- 2.41.0