make `noop_method_call` warn by default #111916

fee1-dead · 2023-05-24T16:06:04Z

erikdesjardins · 2023-05-24T21:14:14Z

tests/ui/lint/noop-method-call.stderr

@@ -1,62 +1,58 @@
 warning: call to `.clone()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:16:71
+  --> $DIR/noop-method-call.rs:15:71
   |
 LL |     let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.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


This note doesn't really make sense. Vec<i32>::clone also returns the same type it's called on, but that doesn't mean it does nothing. (Alternatively, if you argue that autoref means Vec::new().clone() actually calls clone on &Vec, then clone never returns the type it's called on and the note is still incorrect, because then we're calling clone on &&PlainType<u32>)

I would suggest something like:

Suggested change

= 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

= note: the type `PlainType<u32>` does not implement `Clone`, so calling `clone` on `&PlainType<u32>` clones the reference, which does not do anything and can be removed

bors · 2023-05-27T07:59:07Z

☔ The latest upstream changes (presumably #111348) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2023-05-28T05:35:17Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler-errors

Does bumping a lint from allow -> warn need an fcp or anything? Implementation looks fine now -- I like the new rewording too.

compiler/rustc_lint/src/noop_method_call.rs

fee1-dead · 2023-05-30T00:18:16Z

Not sure about whether a fcp is needed.

cc @rust-lang/compiler-contributors

RalfJung · 2023-05-30T05:19:02Z

cc @rust-lang/compiler-contributors

(That's a huge ping group with more than 40 people, most of whom do not have FCP power. It's probably not what you wanted to ping here.)

fee1-dead · 2023-05-30T05:22:32Z

It's the group that gets pinged for MCPs so I pinged.

oli-obk · 2023-05-30T09:23:45Z

This lint was made allow-by-default in the original PR, but there was never the proposed follow up to make it warn-by-default. I think it was mostly allow-by-default to make it easier for clippy to transition?

This implemented the lint proposed in the following lang-team MCP: rust-lang/lang-team#67

The lang team would like to see a crater run with this lint denied to judge its effects on existing code.

fee1-dead · 2023-06-01T04:16:57Z

experiment PR now up at #112160

fee1-dead · 2023-06-06T13:22:27Z

experiment completed. Should I add a suggestion to the diagnostic?

fee1-dead · 2023-06-07T10:45:36Z

Marking as S-waiting-on(-lang)-team

oli-obk · 2023-06-08T05:48:30Z

experiment completed. Should I add a suggestion to the diagnostic?

Please summarize the crater results. You can then add the lang team nominated label so their triage picks it up

fee1-dead · 2023-07-23T15:44:52Z

I have added a suggestion.

rfcbot · 2023-07-28T19:02:24Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

compiler-errors · 2023-07-28T19:12:47Z

@bors r+

bors · 2023-07-28T19:12:49Z

📌 Commit 2a76c57 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-07-29T01:40:53Z

⌛ Testing commit 2a76c57 with merge 4734ac0...

bors · 2023-07-29T04:18:39Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 4734ac0 to master...

rust-timer · 2023-07-29T12:30:49Z

Finished benchmarking commit (4734ac0): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.9%	[3.9%, 3.9%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.6%	[-0.6%, -0.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[-0.6%, 3.9%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 651.453s -> 650.829s (-0.10%)

…arn, r=compiler-errors make `noop_method_call` warn by default r? `@compiler-errors`

Language -------- - [Uplift `clippy::fn_null_check` lint as `useless_ptr_null_checks`.] (rust-lang/rust#111717) - [Make `noop_method_call` warn by default.] (rust-lang/rust#111916) - [Support interpolated block for `try` and `async` in macros.] (rust-lang/rust#112953) - [Make `unconditional_recursion` lint detect recursive drops.] (rust-lang/rust#113902) - [Future compatibility warning for some impls being incorrectly considered not overlapping.] (rust-lang/rust#114023) - [The `invalid_reference_casting` lint is now **deny-by-default** (instead of allow-by-default)] (rust-lang/rust#112431) Compiler -------- - [Write version information in a `.comment` section like GCC/Clang.] (rust-lang/rust#97550) - [Add documentation on v0 symbol mangling.] (rust-lang/rust#97571) - [Stabilize `extern "thiscall"` and `"thiscall-unwind"` ABIs.] (rust-lang/rust#114562) - [Only check outlives goals on impl compared to trait.] (rust-lang/rust#109356) - [Infer type in irrefutable slice patterns with fixed length as array.] (rust-lang/rust#113199) - [Discard default auto trait impls if explicit ones exist.] (rust-lang/rust#113312) - Add several new tier 3 targets: - [`aarch64-unknown-teeos`] (rust-lang/rust#113480) - [`csky-unknown-linux-gnuabiv2`] (rust-lang/rust#113658) - [`riscv64-linux-android`] (rust-lang/rust#112858) - [`riscv64gc-unknown-hermit`] (rust-lang/rust#114004) - [`x86_64-unikraft-linux-musl`] (rust-lang/rust#113411) - [`x86_64-unknown-linux-ohos`] (rust-lang/rust#113061) - [Add `wasm32-wasi-preview1-threads` as a tier 2 target.] (rust-lang/rust#112922) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Add `Read`, `Write` and `Seek` impls for `Arc<File>`.] (rust-lang/rust#94748) - [Merge functionality of `io::Sink` into `io::Empty`.] (rust-lang/rust#98154) - [Implement `RefUnwindSafe` for `Backtrace`] (rust-lang/rust#100455) - [Make `ExitStatus` implement `Default`] (rust-lang/rust#106425) - [`impl SliceIndex<str> for (Bound<usize>, Bound<usize>)`] (rust-lang/rust#111081) - [Change default panic handler message format.] (rust-lang/rust#112849) - [Cleaner `assert_eq!` & `assert_ne!` panic messages.] (rust-lang/rust#111071) - [Correct the (deprecated) Android `stat` struct definitions.] (rust-lang/rust#113130) Stabilized APIs --------------- - [Unsigned `{integer}::div_ceil`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.div_ceil) - [Unsigned `{integer}::next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.next_multiple_of) - [Unsigned `{integer}::checked_next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.checked_next_multiple_of) - [`std::ffi::FromBytesUntilNulError`] (https://doc.rust-lang.org/stable/std/ffi/struct.FromBytesUntilNulError.html) - [`std::os::unix::fs::chown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.chown.html) - [`std::os::unix::fs::fchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.fchown.html) - [`std::os::unix::fs::lfchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.lchown.html) - [`LocalKey::<Cell<T>>::get`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.get) - [`LocalKey::<Cell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set) - [`LocalKey::<Cell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take) - [`LocalKey::<Cell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace) - [`LocalKey::<RefCell<T>>::with_borrow`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow) - [`LocalKey::<RefCell<T>>::with_borrow_mut`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow_mut) - [`LocalKey::<RefCell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set-1) - [`LocalKey::<RefCell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take-1) - [`LocalKey::<RefCell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace-1) These APIs are now stable in const contexts: - [`rc::Weak::new`] (https://doc.rust-lang.org/stable/alloc/rc/struct.Weak.html#method.new) - [`sync::Weak::new`] (https://doc.rust-lang.org/stable/alloc/sync/struct.Weak.html#method.new) - [`NonNull::as_ref`] (https://doc.rust-lang.org/stable/core/ptr/struct.NonNull.html#method.as_ref) Cargo ----- - [Encode URL params correctly for `SourceId` in `Cargo.lock`.] (rust-lang/cargo#12280) - [Bail out an error when using `cargo::` in custom build script.] (rust-lang/cargo#12332) Compatibility Notes ------------------- - [Update the minimum external LLVM to 15.] (rust-lang/rust#114148) - [Check for non-defining uses of return position `impl Trait`.] (rust-lang/rust#112842) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Remove LLVM pointee types, supporting only opaque pointers.] (rust-lang/rust#105545) - [Port PGO/LTO/BOLT optimized build pipeline to Rust.] (rust-lang/rust#112235) - [Replace in-tree `rustc_apfloat` with the new version of the crate.] (rust-lang/rust#113843) - [Update to LLVM 17.] (rust-lang/rust#114048) - [Add `internal_features` lint for internal unstable features.] (rust-lang/rust#108955) - [Mention style for new syntax in tracking issue template.] (rust-lang/rust#113586)

Pkgsrc changes: * Adjust patches and cargo checksums to new versions. * For an external LLVM, set dependency of llvm >= 15, in accordance with the upstream changes. * Add a patch with a backport from LLVM 17.0.3 fixing codegen for PPC, ref. rust-lang/rust#116845 Upstream changes: Version 1.73.0 (2023-10-05) ========================== Language -------- - [Uplift `clippy::fn_null_check` lint as `useless_ptr_null_checks`.] (rust-lang/rust#111717) - [Make `noop_method_call` warn by default.] (rust-lang/rust#111916) - [Support interpolated block for `try` and `async` in macros.] (rust-lang/rust#112953) - [Make `unconditional_recursion` lint detect recursive drops.] (rust-lang/rust#113902) - [Future compatibility warning for some impls being incorrectly considered not overlapping.] (rust-lang/rust#114023) - [The `invalid_reference_casting` lint is now **deny-by-default** (instead of allow-by-default)] (rust-lang/rust#112431 Compiler -------- - [Write version information in a `.comment` section like GCC/Clang.] (rust-lang/rust#97550) - [Add documentation on v0 symbol mangling.] (rust-lang/rust#97571) - [Stabilize `extern "thiscall"` and `"thiscall-unwind"` ABIs.] (rust-lang/rust#114562) - [Only check outlives goals on impl compared to trait.] (rust-lang/rust#109356) - [Infer type in irrefutable slice patterns with fixed length as array.] (rust-lang/rust#113199) - [Discard default auto trait impls if explicit ones exist.] (rust-lang/rust#113312) - Add several new tier 3 targets: - [`aarch64-unknown-teeos`] (rust-lang/rust#113480) - [`csky-unknown-linux-gnuabiv2`] (rust-lang/rust#113658) - [`riscv64-linux-android`] (rust-lang/rust#112858) - [`riscv64gc-unknown-hermit`] (rust-lang/rust#114004) - [`x86_64-unikraft-linux-musl`] (rust-lang/rust#113411) - [`x86_64-unknown-linux-ohos`] (rust-lang/rust#113061) - [Add `wasm32-wasi-preview1-threads` as a tier 2 target.] (rust-lang/rust#112922) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Add `Read`, `Write` and `Seek` impls for `Arc<File>`.] (rust-lang/rust#94748) - [Merge functionality of `io::Sink` into `io::Empty`.] (rust-lang/rust#98154) - [Implement `RefUnwindSafe` for `Backtrace`] (rust-lang/rust#100455) - [Make `ExitStatus` implement `Default`] (rust-lang/rust#106425) - [`impl SliceIndex<str> for (Bound<usize>, Bound<usize>)`] (rust-lang/rust#111081) - [Change default panic handler message format.] (rust-lang/rust#112849) - [Cleaner `assert_eq!` & `assert_ne!` panic messages.] (rust-lang/rust#111071) - [Correct the (deprecated) Android `stat` struct definitions.] (rust-lang/rust#113130) Stabilized APIs --------------- - [Unsigned `{integer}::div_ceil`] (https://doc.rust-lang.org/stable/std/primitiv e.u32.html#method.div_ceil) - [Unsigned `{integer}::next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.next_multiple_of) - [Unsigned `{integer}::checked_next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.checked_next_multiple_of) - [`std::ffi::FromBytesUntilNulError`] (https://doc.rust-lang.org/stable/std/ffi/struct.FromBytesUntilNulError.html) - [`std::os::unix::fs::chown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.chown.html) - [`std::os::unix::fs::fchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.fchown.html) - [`std::os::unix::fs::lfchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.lchown.html) - [`LocalKey::<Cell<T>>::get`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.get) - [`LocalKey::<Cell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set) - [`LocalKey::<Cell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take) - [`LocalKey::<Cell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace) - [`LocalKey::<RefCell<T>>::with_borrow`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow) - [`LocalKey::<RefCell<T>>::with_borrow_mut`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow_mut) - [`LocalKey::<RefCell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set-1) - [`LocalKey::<RefCell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take-1) - [`LocalKey::<RefCell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace-1) These APIs are now stable in const contexts: - [`rc::Weak::new`] (https://doc.rust-lang.org/stable/alloc/rc/struct.Weak.html#method.new) - [`sync::Weak::new`] (https://doc.rust-lang.org/stable/alloc/sync/struct.Weak.html#method.new) - [`NonNull::as_ref`] (https://doc.rust-lang.org/stable/core/ptr/struct.NonNull.html#method.as_ref) Cargo ----- - [Encode URL params correctly for `SourceId` in `Cargo.lock`.] (rust-lang/cargo#12280) - [Bail out an error when using `cargo::` in custom build script.] (rust-lang/cargo#12332) Misc ---- Compatibility Notes ------------------- - [Update the minimum external LLVM to 15.] (rust-lang/rust#114148) - [Check for non-defining uses of return position `impl Trait`.] (rust-lang/rust#112842) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Remove LLVM pointee types, supporting only opaque pointers.] (rust-lang/rust#105545) - [Port PGO/LTO/BOLT optimized build pipeline to Rust.] (rust-lang/rust#112235) - [Replace in-tree `rustc_apfloat` with the new version of the crate.] (rust-lang/rust#113843) - [Update to LLVM 17.] (rust-lang/rust#114048) - [Add `internal_features` lint for internal unstable features.] (rust-lang/rust#108955) - [Mention style for new syntax in tracking issue template.] (rust-lang/rust#113586)

rustbot assigned compiler-errors May 24, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 24, 2023

This comment has been minimized.

Sign in to view

erikdesjardins reviewed May 24, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the noop-method-call-warn branch from bd48f32 to 85c4aba Compare May 26, 2023 03:12

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 26, 2023

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the noop-method-call-warn branch from 85c4aba to 7a58b21 Compare May 28, 2023 05:35

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the noop-method-call-warn branch from 7a58b21 to 6c9fb8c Compare May 28, 2023 06:11

compiler-errors approved these changes May 29, 2023

View reviewed changes

compiler/rustc_lint/src/noop_method_call.rs Outdated Show resolved Hide resolved

fee1-dead force-pushed the noop-method-call-warn branch from 6c9fb8c to 6fd5a06 Compare May 29, 2023 14:09

compiler-errors added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 5, 2023

fee1-dead added T-lang Relevant to the language team, which will review and decide on the PR/issue. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jun 7, 2023

fee1-dead added 2 commits July 23, 2023 09:58

fix

626efab

add suggestion

2a76c57

fee1-dead force-pushed the noop-method-call-warn branch from 6fd5a06 to 2a76c57 Compare July 23, 2023 09:58

rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 23, 2023

fee1-dead removed the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 23, 2023

fee1-dead added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 23, 2023

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 28, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 29, 2023

bors merged commit 4734ac0 into rust-lang:master Jul 29, 2023
11 checks passed

rustbot added this to the 1.73.0 milestone Jul 29, 2023

fee1-dead deleted the noop-method-call-warn branch July 29, 2023 06:56

apiraino removed the to-announce Announce this issue on triage meeting label Aug 4, 2023

flip1995 pushed a commit to flip1995/rust that referenced this pull request Aug 11, 2023

Auto merge of rust-lang#111916 - fee1-dead-contrib:noop-method-call-w…

0c0026e

…arn, r=compiler-errors make `noop_method_call` warn by default r? `@compiler-errors`

torives mentioned this pull request Oct 24, 2023

ci: adjust dependabot settings cartesi/rollups-node#127

Merged

kalil-pelissier mentioned this pull request Jan 26, 2024

dragonball: fix noop-method-call warning kata-containers/kata-containers#8932

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make `noop_method_call` warn by default #111916

make `noop_method_call` warn by default #111916

fee1-dead commented May 24, 2023

This comment has been minimized.

erikdesjardins May 24, 2023 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

bors commented May 27, 2023

rustbot commented May 28, 2023

This comment has been minimized.

compiler-errors left a comment

fee1-dead commented May 30, 2023

RalfJung commented May 30, 2023

fee1-dead commented May 30, 2023

oli-obk commented May 30, 2023

fee1-dead commented Jun 1, 2023

fee1-dead commented Jun 6, 2023

fee1-dead commented Jun 7, 2023 •

edited

Loading

oli-obk commented Jun 8, 2023

fee1-dead commented Jul 23, 2023

rfcbot commented Jul 28, 2023

compiler-errors commented Jul 28, 2023

bors commented Jul 28, 2023

bors commented Jul 29, 2023

bors commented Jul 29, 2023

rust-timer commented Jul 29, 2023

	= 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
	= note: the type `PlainType<u32>` does not implement `Clone`, so calling `clone` on `&PlainType<u32>` clones the reference, which does not do anything and can be removed

make noop_method_call warn by default #111916

make noop_method_call warn by default #111916

Conversation

fee1-dead commented May 24, 2023

This comment has been minimized.

erikdesjardins May 24, 2023 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

bors commented May 27, 2023

rustbot commented May 28, 2023

This comment has been minimized.

compiler-errors left a comment

Choose a reason for hiding this comment

fee1-dead commented May 30, 2023

RalfJung commented May 30, 2023

fee1-dead commented May 30, 2023

oli-obk commented May 30, 2023

fee1-dead commented Jun 1, 2023

fee1-dead commented Jun 6, 2023

fee1-dead commented Jun 7, 2023 • edited Loading

oli-obk commented Jun 8, 2023

fee1-dead commented Jul 23, 2023

rfcbot commented Jul 28, 2023

compiler-errors commented Jul 28, 2023

bors commented Jul 28, 2023

bors commented Jul 29, 2023

bors commented Jul 29, 2023

rust-timer commented Jul 29, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

make `noop_method_call` warn by default #111916

make `noop_method_call` warn by default #111916

erikdesjardins May 24, 2023 •

edited

Loading

fee1-dead commented Jun 7, 2023 •

edited

Loading