Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize substs before resolving instance in NoopMethodCall lint #102489

Merged
merged 2 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions compiler/rustc_lint/src/noop_method_call.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::context::LintContext;
use crate::rustc_middle::ty::TypeVisitable;
use crate::LateContext;
use crate::LateLintPass;
use rustc_errors::fluent;
Expand Down Expand Up @@ -46,7 +45,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.
Expand All @@ -56,21 +55,17 @@ 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);
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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it made sense to use the param-env of the trait here...?

let substs = cx
.tcx
.normalize_erasing_regions(cx.param_env, cx.typeck_results().node_substs(expr.hir_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.
Expand Down
23 changes: 23 additions & 0 deletions src/test/ui/const-generics/generic_const_exprs/issue-102074.rs
Original file line number Diff line number Diff line change
@@ -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() {}
1 change: 1 addition & 0 deletions src/test/ui/lint/noop-method-call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ fn main() {

fn generic<T>(non_clone_type: &PlainType<T>) {
non_clone_type.clone();
//~^ WARNING call to `.clone()` on a reference in this situation does nothing
}

fn non_generic(non_clone_type: &PlainType<u32>) {
Expand Down
12 changes: 10 additions & 2 deletions src/test/ui/lint/noop-method-call.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,20 @@ LL | let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
= note: the type `&PlainType<u32>` 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<T>` 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<u32>` 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