From 8c600120e6abd8c25a88443451d190c68142180f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 29 Sep 2022 22:03:03 +0000 Subject: [PATCH 1/2] Normalize substs before resolving instance in NoopMethodCall lint --- compiler/rustc_lint/src/noop_method_call.rs | 11 +++++---- .../generic_const_exprs/issue-102074.rs | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/const-generics/generic_const_exprs/issue-102074.rs diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 19188d5c376..2a3ff3a7546 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -46,7 +46,7 @@ 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 (trait_id, did) = match cx.typeck_results().type_dependent_def(expr.hir_id) { + 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. @@ -56,21 +56,22 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { Some(sym::Borrow | sym::Clone | sym::Deref) ) => { - (trait_id, did) + did } _ => return, }, _ => return, }; - let substs = cx.typeck_results().node_substs(expr.hir_id); + let substs = cx + .tcx + .normalize_erasing_regions(cx.param_env, 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 perform substitution. return; } - let param_env = cx.tcx.param_env(trait_id); // Resolve the trait method instance. - let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) else { + let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, cx.param_env, did, substs) else { return }; // (Re)check that it implements the noop diagnostic. diff --git a/src/test/ui/const-generics/generic_const_exprs/issue-102074.rs b/src/test/ui/const-generics/generic_const_exprs/issue-102074.rs new file mode 100644 index 00000000000..66d15cf1215 --- /dev/null +++ b/src/test/ui/const-generics/generic_const_exprs/issue-102074.rs @@ -0,0 +1,23 @@ +// check-pass +// Checks that the NoopMethodCall lint doesn't call Instance::resolve on unresolved consts + +#![feature(generic_const_exprs)] +#![allow(incomplete_features)] + +#[derive(Debug, Clone)] +pub struct Aes128CipherKey([u8; Aes128Cipher::KEY_LEN]); + +impl Aes128CipherKey { + pub fn new(key: &[u8; Aes128Cipher::KEY_LEN]) -> Self { + Self(key.clone()) + } +} + +#[derive(Debug, Clone)] +pub struct Aes128Cipher; + +impl Aes128Cipher { + const KEY_LEN: usize = 16; +} + +fn main() {} From e1b313af46b74a446d7772a261e006c199a5b2e0 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 4 Oct 2022 03:22:45 +0000 Subject: [PATCH 2/2] We are able to resolve methods even if they need subst --- compiler/rustc_lint/src/noop_method_call.rs | 6 ------ src/test/ui/lint/noop-method-call.rs | 1 + src/test/ui/lint/noop-method-call.stderr | 12 ++++++++++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 2a3ff3a7546..9a62afd3caf 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -1,5 +1,4 @@ use crate::context::LintContext; -use crate::rustc_middle::ty::TypeVisitable; use crate::LateContext; use crate::LateLintPass; use rustc_errors::fluent; @@ -65,11 +64,6 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { let substs = cx .tcx .normalize_erasing_regions(cx.param_env, 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 perform substitution. - return; - } // Resolve the trait method instance. let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, cx.param_env, did, substs) else { return diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs index 9870c813572..89b29663595 100644 --- a/src/test/ui/lint/noop-method-call.rs +++ b/src/test/ui/lint/noop-method-call.rs @@ -46,6 +46,7 @@ fn main() { fn generic(non_clone_type: &PlainType) { non_clone_type.clone(); + //~^ WARNING call to `.clone()` on a reference in this situation does nothing } fn non_generic(non_clone_type: &PlainType) { diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr index c71de44dc71..6a904d01abc 100644 --- a/src/test/ui/lint/noop-method-call.stderr +++ b/src/test/ui/lint/noop-method-call.stderr @@ -28,12 +28,20 @@ LL | let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); = 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 + --> $DIR/noop-method-call.rs:48: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: call to `.clone()` on a reference in this situation does nothing + --> $DIR/noop-method-call.rs:53: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 +warning: 5 warnings emitted