diff --git a/compiler/rustc_codegen_ssa/src/back/rpath.rs b/compiler/rustc_codegen_ssa/src/back/rpath.rs index 005d2efdd3b26..5f21046b05e47 100644 --- a/compiler/rustc_codegen_ssa/src/back/rpath.rs +++ b/compiler/rustc_codegen_ssa/src/back/rpath.rs @@ -24,7 +24,7 @@ pub fn get_rpath_flags(config: &mut RPathConfig<'_>) -> Vec { debug!("preparing the RPATH!"); - let libs = config.used_crates.clone(); + let libs = config.used_crates; let libs = libs.iter().filter_map(|&(_, ref l)| l.option()).collect::>(); let rpaths = get_rpaths(config, &libs); let mut flags = rpaths_to_flags(&rpaths); diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 638b73c27a8d7..b0c22154c0b5b 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -57,6 +57,7 @@ mod methods; mod non_ascii_idents; mod non_fmt_panic; mod nonstandard_style; +mod noop_method_call; mod passes; mod redundant_semicolon; mod traits; @@ -83,6 +84,7 @@ use methods::*; use non_ascii_idents::*; use non_fmt_panic::NonPanicFmt; use nonstandard_style::*; +use noop_method_call::*; use redundant_semicolon::*; use traits::*; use types::*; @@ -170,6 +172,7 @@ macro_rules! late_lint_passes { DropTraitConstraints: DropTraitConstraints, TemporaryCStringAsPtr: TemporaryCStringAsPtr, NonPanicFmt: NonPanicFmt, + NoopMethodCall: NoopMethodCall, ] ); }; diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs new file mode 100644 index 0000000000000..479cc00199f6a --- /dev/null +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -0,0 +1,111 @@ +use crate::context::LintContext; +use crate::rustc_middle::ty::TypeFoldable; +use crate::LateContext; +use crate::LateLintPass; +use rustc_hir::def::DefKind; +use rustc_hir::{Expr, ExprKind}; +use rustc_middle::ty; +use rustc_span::symbol::sym; + +declare_lint! { + /// The `noop_method_call` lint detects specific calls to noop methods + /// such as a calling `<&T as Clone>::clone` where `T: !Clone`. + /// + /// ### Example + /// + /// ```rust + /// # #![allow(unused)] + /// #![warn(noop_method_call)] + /// struct Foo; + /// let foo = &Foo; + /// let clone: &Foo = foo.clone(); + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Some method calls are noops meaning that they do nothing. Usually such methods + /// are the result of blanket implementations that happen to create some method invocations + /// that end up not doing anything. For instance, `Clone` is implemented on all `&T`, but + /// calling `clone` on a `&T` where `T` does not implement clone, actually doesn't do anything + /// as references are copy. This lint detects these calls and warns the user about them. + pub NOOP_METHOD_CALL, + Allow, + "detects the use of well-known noop methods" +} + +declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]); + +impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + // We only care about method calls. + let (call, elements) = match expr.kind { + ExprKind::MethodCall(call, _, elements, _) => (call, elements), + _ => return, + }; + // We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow` + // traits and ignore any other method call. + let (trait_id, 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 [sym::Clone, sym::Deref, sym::Borrow] + .iter() + .any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) => + { + (trait_id, did) + } + _ => return, + }, + _ => return, + }; + let substs = cx.typeck_results().node_substs(expr.hir_id); + if substs.needs_subst() { + // We can't resolve on types that require monomorphization, so we don't handle them if + // we need to perfom substitution. + return; + } + let param_env = cx.tcx.param_env(trait_id); + // Resolve the trait method instance. + let i = match ty::Instance::resolve(cx.tcx, param_env, did, substs) { + Ok(Some(i)) => i, + _ => return, + }; + // (Re)check that it implements the noop diagnostic. + for s in [sym::noop_method_clone, sym::noop_method_deref, sym::noop_method_borrow].iter() { + if cx.tcx.is_diagnostic_item(*s, i.def_id()) { + let method = &call.ident.name; + let receiver = &elements[0]; + let receiver_ty = cx.typeck_results().expr_ty(receiver); + let expr_ty = cx.typeck_results().expr_ty_adjusted(expr); + if receiver_ty != expr_ty { + // This lint will only trigger if the receiver type and resulting expression \ + // type are the same, implying that the method call is unnecessary. + return; + } + let expr_span = expr.span; + let note = format!( + "the type `{:?}` which `{}` is being called on is the same as \ + the type returned from `{}`, so the method call does not do \ + anything and can be removed", + receiver_ty, method, method, + ); + + let span = expr_span.with_lo(receiver.span.hi()); + cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| { + let method = &call.ident.name; + let message = format!( + "call to `.{}()` on a reference in this situation does nothing", + &method, + ); + lint.build(&message) + .span_label(span, "unnecessary method call") + .note(¬e) + .emit() + }); + } + } + } +} diff --git a/compiler/rustc_middle/src/ty/error.rs b/compiler/rustc_middle/src/ty/error.rs index bf315c81588a9..f19cc99844926 100644 --- a/compiler/rustc_middle/src/ty/error.rs +++ b/compiler/rustc_middle/src/ty/error.rs @@ -12,7 +12,6 @@ use rustc_target::spec::abi; use std::borrow::Cow; use std::fmt; -use std::ops::Deref; #[derive(Clone, Copy, Debug, PartialEq, Eq, TypeFoldable)] pub struct ExpectedFound { @@ -548,7 +547,6 @@ impl Trait for X { TargetFeatureCast(def_id) => { let attrs = self.get_attrs(*def_id); let target_spans = attrs - .deref() .iter() .filter(|attr| attr.has_name(sym::target_feature)) .map(|attr| attr.span); diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 8c05e6fd5d0e4..e423e449746fc 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -165,7 +165,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, value); // Invalidate all borrows of local places - let borrow_set = self.borrow_set.clone(); + let borrow_set = self.borrow_set; let resume = self.location_table.start_index(resume.start_location()); for (i, data) in borrow_set.iter_enumerated() { if borrow_of_local_data(data.borrowed_place) { @@ -177,7 +177,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { } TerminatorKind::Resume | TerminatorKind::Return | TerminatorKind::GeneratorDrop => { // Invalidate all borrows of local places - let borrow_set = self.borrow_set.clone(); + let borrow_set = self.borrow_set; let start = self.location_table.start_index(location); for (i, data) in borrow_set.iter_enumerated() { if borrow_of_local_data(data.borrowed_place) { @@ -369,7 +369,7 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> { ); let tcx = self.tcx; let body = self.body; - let borrow_set = self.borrow_set.clone(); + let borrow_set = self.borrow_set; let indices = self.borrow_set.indices(); each_borrow_involving_path( self, @@ -377,7 +377,7 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> { body, location, (sd, place), - &borrow_set.clone(), + borrow_set, indices, |this, borrow_index, borrow| { match (rw, borrow.kind) { diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 126fb957a6a99..4db7debee7e8f 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -51,7 +51,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { PatKind::Constant { value } => Test { span: match_pair.pattern.span, - kind: TestKind::Eq { value, ty: match_pair.pattern.ty.clone() }, + kind: TestKind::Eq { value, ty: match_pair.pattern.ty }, }, PatKind::Range(range) => { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 27bb45bcc8512..61f9a080a5217 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -129,6 +129,7 @@ symbols! { BTreeMap, BTreeSet, BinaryHeap, + Borrow, C, CString, Center, @@ -141,6 +142,7 @@ symbols! { Decodable, Decoder, Default, + Deref, Encodable, Encoder, Eq, @@ -789,6 +791,9 @@ symbols! { none_error, nontemporal_store, nontrapping_dash_fptoint: "nontrapping-fptoint", + noop_method_borrow, + noop_method_clone, + noop_method_deref, noreturn, nostack, not, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index bfb5ebcea58b1..a3faf4cb7d4c1 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -819,7 +819,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { sig.decl .inputs .iter() - .map(|arg| match arg.clone().kind { + .map(|arg| match arg.kind { hir::TyKind::Tup(ref tys) => ArgKind::Tuple( Some(arg.span), vec![("_".to_owned(), "_".to_owned()); tys.len()], diff --git a/compiler/rustc_traits/src/chalk/mod.rs b/compiler/rustc_traits/src/chalk/mod.rs index d98f18182c843..b7275bac19048 100644 --- a/compiler/rustc_traits/src/chalk/mod.rs +++ b/compiler/rustc_traits/src/chalk/mod.rs @@ -165,7 +165,7 @@ crate fn evaluate_goal<'tcx>( // let's just ignore that let sol = Canonical { max_universe: ty::UniverseIndex::from_usize(0), - variables: obligation.variables.clone(), + variables: obligation.variables, value: QueryResponse { var_values: CanonicalVarValues { var_values: IndexVec::new() } .make_identity(tcx), diff --git a/compiler/rustc_typeck/src/check/callee.rs b/compiler/rustc_typeck/src/check/callee.rs index ca1e79fac73e9..6ef63bcbbbf21 100644 --- a/compiler/rustc_typeck/src/check/callee.rs +++ b/compiler/rustc_typeck/src/check/callee.rs @@ -465,7 +465,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let expected_arg_tys = self.expected_inputs_for_expected_output( call_expr.span, expected, - fn_sig.output().clone(), + fn_sig.output(), fn_sig.inputs(), ); diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 2faf128c491fd..48740e533da8e 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -711,7 +711,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }); let ret_ty = ret_coercion.borrow().expected_ty(); - let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty.clone()); + let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty); ret_coercion.borrow_mut().coerce( self, &self.cause(return_expr.span, ObligationCauseCode::ReturnValue(return_expr.hir_id)), diff --git a/library/alloc/src/collections/btree/map/tests.rs b/library/alloc/src/collections/btree/map/tests.rs index 4e48db7f49305..e636e490e1bf4 100644 --- a/library/alloc/src/collections/btree/map/tests.rs +++ b/library/alloc/src/collections/btree/map/tests.rs @@ -1801,11 +1801,11 @@ fn test_occupied_entry_key() { let key = "hello there"; let value = "value goes here"; assert!(a.is_empty()); - a.insert(key.clone(), value.clone()); + a.insert(key, value); assert_eq!(a.len(), 1); assert_eq!(a[key], value); - match a.entry(key.clone()) { + match a.entry(key) { Vacant(_) => panic!(), Occupied(e) => assert_eq!(key, *e.key()), } @@ -1821,11 +1821,11 @@ fn test_vacant_entry_key() { let value = "value goes here"; assert!(a.is_empty()); - match a.entry(key.clone()) { + match a.entry(key) { Occupied(_) => panic!(), Vacant(e) => { assert_eq!(key, *e.key()); - e.insert(value.clone()); + e.insert(value); } } assert_eq!(a.len(), 1); diff --git a/library/core/src/borrow.rs b/library/core/src/borrow.rs index c9040cd0a1670..f28be20aaa1e6 100644 --- a/library/core/src/borrow.rs +++ b/library/core/src/borrow.rs @@ -153,6 +153,7 @@ /// [`HashMap`]: ../../std/collections/struct.HashMap.html /// [`String`]: ../../std/string/struct.String.html #[stable(feature = "rust1", since = "1.0.0")] +#[rustc_diagnostic_item = "Borrow"] pub trait Borrow { /// Immutably borrows from an owned value. /// @@ -205,6 +206,7 @@ pub trait BorrowMut: Borrow { #[stable(feature = "rust1", since = "1.0.0")] impl Borrow for T { + #[rustc_diagnostic_item = "noop_method_borrow"] fn borrow(&self) -> &T { self } diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index a953a3a4182bc..51a2dc03de318 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -104,12 +104,14 @@ /// [impls]: #implementors #[stable(feature = "rust1", since = "1.0.0")] #[lang = "clone"] +#[rustc_diagnostic_item = "Clone"] pub trait Clone: Sized { /// Returns a copy of the value. /// /// # Examples /// /// ``` + /// # #![allow(noop_method_call)] /// let hello = "Hello"; // &str implements Clone /// /// assert_eq!("Hello", hello.clone()); @@ -221,6 +223,7 @@ mod impls { #[stable(feature = "rust1", since = "1.0.0")] impl Clone for &T { #[inline] + #[rustc_diagnostic_item = "noop_method_clone"] fn clone(&self) -> Self { *self } diff --git a/library/core/src/ops/deref.rs b/library/core/src/ops/deref.rs index 2419771eae212..10e3ce67448c8 100644 --- a/library/core/src/ops/deref.rs +++ b/library/core/src/ops/deref.rs @@ -60,6 +60,7 @@ #[doc(alias = "*")] #[doc(alias = "&*")] #[stable(feature = "rust1", since = "1.0.0")] +#[rustc_diagnostic_item = "Deref"] pub trait Deref { /// The resulting type after dereferencing. #[stable(feature = "rust1", since = "1.0.0")] @@ -78,6 +79,7 @@ pub trait Deref { impl Deref for &T { type Target = T; + #[rustc_diagnostic_item = "noop_method_deref"] fn deref(&self) -> &T { *self } diff --git a/library/core/tests/iter/adapters/intersperse.rs b/library/core/tests/iter/adapters/intersperse.rs index 9dbe232e4eec8..b336c03b5adbe 100644 --- a/library/core/tests/iter/adapters/intersperse.rs +++ b/library/core/tests/iter/adapters/intersperse.rs @@ -9,7 +9,7 @@ fn test_intersperse() { assert_eq!(v, vec![1]); let xs = ["a", "", "b", "c"]; - let v: Vec<&str> = xs.iter().map(|x| x.clone()).intersperse(", ").collect(); + let v: Vec<&str> = xs.iter().map(|x| *x).intersperse(", ").collect(); let text: String = v.concat(); assert_eq!(text, "a, , b, c".to_string()); @@ -24,7 +24,7 @@ fn test_intersperse_size_hint() { assert_eq!(iter.size_hint(), (0, Some(0))); let xs = ["a", "", "b", "c"]; - let mut iter = xs.iter().map(|x| x.clone()).intersperse(", "); + let mut iter = xs.iter().map(|x| *x).intersperse(", "); assert_eq!(iter.size_hint(), (7, Some(7))); assert_eq!(iter.next(), Some("a")); diff --git a/library/std/src/collections/hash/map/tests.rs b/library/std/src/collections/hash/map/tests.rs index 467968354e25d..819be14222752 100644 --- a/library/std/src/collections/hash/map/tests.rs +++ b/library/std/src/collections/hash/map/tests.rs @@ -774,11 +774,11 @@ fn test_occupied_entry_key() { let key = "hello there"; let value = "value goes here"; assert!(a.is_empty()); - a.insert(key.clone(), value.clone()); + a.insert(key, value); assert_eq!(a.len(), 1); assert_eq!(a[key], value); - match a.entry(key.clone()) { + match a.entry(key) { Vacant(_) => panic!(), Occupied(e) => assert_eq!(key, *e.key()), } @@ -793,11 +793,11 @@ fn test_vacant_entry_key() { let value = "value goes here"; assert!(a.is_empty()); - match a.entry(key.clone()) { + match a.entry(key) { Occupied(_) => panic!(), Vacant(e) => { assert_eq!(key, *e.key()); - e.insert(value.clone()); + e.insert(value); } } assert_eq!(a.len(), 1); diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs new file mode 100644 index 0000000000000..9870c813572e3 --- /dev/null +++ b/src/test/ui/lint/noop-method-call.rs @@ -0,0 +1,54 @@ +// check-pass + +#![allow(unused)] +#![warn(noop_method_call)] + +use std::borrow::Borrow; +use std::ops::Deref; + +struct PlainType(T); + +#[derive(Clone)] +struct CloneType(T); + +fn main() { + let non_clone_type_ref = &PlainType(1u32); + let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); + //~^ WARNING call to `.clone()` on a reference in this situation does nothing + + let clone_type_ref = &CloneType(1u32); + let clone_type_ref_clone: CloneType = clone_type_ref.clone(); + + // Calling clone on a double reference doesn't warn since the method call itself + // peels the outer reference off + let clone_type_ref = &&CloneType(1u32); + let clone_type_ref_clone: &CloneType = clone_type_ref.clone(); + + let non_deref_type = &PlainType(1u32); + let non_deref_type_deref: &PlainType = non_deref_type.deref(); + //~^ WARNING call to `.deref()` on a reference in this situation does nothing + + // Dereferencing a &&T does not warn since it has collapsed the double reference + let non_deref_type = &&PlainType(1u32); + let non_deref_type_deref: &PlainType = non_deref_type.deref(); + + let non_borrow_type = &PlainType(1u32); + let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); + //~^ WARNING call to `.borrow()` on a reference in this situation does nothing + + // Borrowing a &&T does not warn since it has collapsed the double reference + let non_borrow_type = &&PlainType(1u32); + let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); + + let xs = ["a", "b", "c"]; + let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // ok, but could use `*x` instead +} + +fn generic(non_clone_type: &PlainType) { + non_clone_type.clone(); +} + +fn non_generic(non_clone_type: &PlainType) { + non_clone_type.clone(); + //~^ WARNING call to `.clone()` on a reference in this situation does nothing +} diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr new file mode 100644 index 0000000000000..7f6f96bf1d142 --- /dev/null +++ b/src/test/ui/lint/noop-method-call.stderr @@ -0,0 +1,39 @@ +warning: call to `.clone()` on a reference in this situation does nothing + --> $DIR/noop-method-call.rs:16:71 + | +LL | let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); + | ^^^^^^^^ unnecessary method call + | +note: the lint level is defined here + --> $DIR/noop-method-call.rs:4:9 + | +LL | #![warn(noop_method_call)] + | ^^^^^^^^^^^^^^^^ + = note: the type `&PlainType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed + +warning: call to `.deref()` on a reference in this situation does nothing + --> $DIR/noop-method-call.rs:28:63 + | +LL | let non_deref_type_deref: &PlainType = non_deref_type.deref(); + | ^^^^^^^^ unnecessary method call + | + = note: the type `&PlainType` which `deref` is being called on is the same as the type returned from `deref`, so the method call does not do anything and can be removed + +warning: call to `.borrow()` on a reference in this situation does nothing + --> $DIR/noop-method-call.rs:36:66 + | +LL | let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); + | ^^^^^^^^^ unnecessary method call + | + = note: the type `&PlainType` which `borrow` is being called on is the same as the type returned from `borrow`, so the method call does not do anything and can be removed + +warning: call to `.clone()` on a reference in this situation does nothing + --> $DIR/noop-method-call.rs:52:19 + | +LL | non_clone_type.clone(); + | ^^^^^^^^ unnecessary method call + | + = note: the type `&PlainType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed + +warning: 4 warnings emitted + diff --git a/src/test/ui/underscore-imports/hygiene-2.rs b/src/test/ui/underscore-imports/hygiene-2.rs index bea61eae6b51a..510d91d0d4624 100644 --- a/src/test/ui/underscore-imports/hygiene-2.rs +++ b/src/test/ui/underscore-imports/hygiene-2.rs @@ -29,5 +29,6 @@ m!(y); fn main() { use crate::y::*; + #[allow(noop_method_call)] (&()).deref(); }