Skip to content

Commit

Permalink
Rollup merge of #112517 - fee1-dead-contrib:sus-op-no-borrow, r=compi…
Browse files Browse the repository at this point in the history
…ler-errors

`suspicious_double_ref_op`: don't lint on `.borrow()`

closes #112489
  • Loading branch information
GuillaumeGomez authored Jun 15, 2023
2 parents ab314a5 + 1caed51 commit db7d837
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 39 deletions.
12 changes: 5 additions & 7 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,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
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1231,11 +1231,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
Expand Down
62 changes: 34 additions & 28 deletions compiler/rustc_lint/src/noop_method_call.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -76,22 +78,22 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {

// 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));
Expand All @@ -102,13 +104,6 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
// (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);
Expand All @@ -129,11 +124,22 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
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,
}
}
}
}
17 changes: 17 additions & 0 deletions tests/ui/lint/issue-112489.rs
Original file line number Diff line number Diff line change
@@ -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();
}

0 comments on commit db7d837

Please # to comment.