uplift `clippy::clone_double_ref` to rustc #109842

fee1-dead · 2023-04-01T18:25:53Z

This closes #109451, but is only part 1 for fixing #109429. The second part would be to run a selection of lints even after borrowck had errors.

rust/compiler/rustc_interface/src/passes.rs

Lines 815 to 822 in b9535c0

    
           // Avoid overwhelming user with errors if borrow checking failed. 
        
           // I'm not sure how helpful this is, to be honest, but it avoids a 
        
           // lot of annoying errors in the ui tests (basically, 
        
           // lint warnings and so on -- kindck used to do this abort, but 
        
           // kindck is gone now). -nmatsakis 
        
           if let Some(reported) = sess.has_errors() { 
        
               return Err(reported); 
        
           }

Note: both suggestions are pretty bad.. Why don't we just suggest dereferencing?

For Clippy:

changelog: Moves: Uplifted clippy::clone_double_ref into rustc

rustbot · 2023-04-01T18:25:58Z

r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-04-01T18:26:01Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

fee1-dead · 2023-04-01T19:51:24Z

Turns out that noop_method_call is a thing already. This should probably be merged to that.

flip1995

Clippy specific review.

src/tools/clippy/tests/ui/deprecated.rs

src/tools/clippy/tests/ui/unnecessary_clone.stderr

rustbot · 2023-04-07T14:17:49Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

bors · 2023-04-10T12:34:51Z

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

fee1-dead · 2023-04-14T05:55:09Z

@rustbot ready

compiler-errors · 2023-04-28T05:25:05Z

compiler/rustc_lint/src/noop_method_call.rs

@@ -31,18 +31,48 @@ declare_lint! {
    /// 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,
+    Warn,


I feel like this change and the associated changes to lib/compiler should be done in a separate PR.

Or at least the PR title and description need to be updated to reflect that this is also bumping NOOP_METHOD_CALL to warn.

But I would prefer the first option. Just uplifting clone_double_ref seems to me very easy to r+, having to approve both is a bit harder to disentangle (esp in case we need to unland one in the future, e.g. due to lots of false positives in some yet undiscovered case -- which I have no reason to believe may happen, but stranger things have happened before).

compiler-errors · 2023-04-28T05:27:31Z

compiler/rustc_lint/src/noop_method_call.rs

+    /// reference, instead of cloning the inner type which should be what was intended.
+    pub SUSPICIOUS_DOUBLE_REF_OP,
+    Warn,
+    "using `clone` on `&&T`"


This lint isn't Clone specific as implemented below? Wording, etc. should probably be adjusted?

compiler-errors · 2023-04-28T05:28:34Z

tests/ui/underscore-imports/cycle.rs

@@ -1,6 +1,7 @@
 // Check that cyclic glob imports are allowed with underscore imports

 // check-pass
+#![allow(noop_method_call)]


I would actually prefer if we added the WARNs to this UI test and others.

compiler-errors · 2023-04-28T05:29:17Z

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

   |
 LL |     let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
   |                                                               ^^^^^^^^ unnecessary method call
   |
   = note: the type `&PlainType<u32>` 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: using `.deref()` on a double reference, which copies `&PlainType<u32>` instead of dereferencing the inner type


Suggested change

warning: using `.deref()` on a double reference, which copies `&PlainType<u32>` instead of dereferencing the inner type

warning: using `.deref()` on a double reference, which returns `&PlainType<u32>` instead of dereferencing the inner type

…=compiler-errors uplift `clippy::clone_double_ref` as `suspicious_double_ref_op` Split from rust-lang#109842. r? `@compiler-errors`

…=compiler-errors uplift `clippy::clone_double_ref` as `suspicious_double_ref_op` Split from rust-lang#109842. r? ``@compiler-errors``

bors · 2023-05-02T10:30:26Z

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

…=compiler-errors uplift `clippy::clone_double_ref` as `suspicious_double_ref_op` Split from rust-lang#109842. r? ``@compiler-errors``

rustbot assigned wesleywiser Apr 1, 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 Apr 1, 2023

This comment has been minimized.

Sign in to view

fee1-dead added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 1, 2023

flip1995 reviewed Apr 3, 2023

View reviewed changes

src/tools/clippy/tests/ui/deprecated.rs Outdated Show resolved Hide resolved

src/tools/clippy/tests/ui/unnecessary_clone.stderr Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the uplift-clone-2x-ref branch from 3048661 to 99851d4 Compare April 8, 2023 05:53

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 Apr 8, 2023

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the uplift-clone-2x-ref branch 3 times, most recently from ebab009 to 655c9ea Compare April 8, 2023 08:22

fee1-dead added 8 commits April 14, 2023 05:53

uplift clippy::clone_double_ref to rustc

f9dde4b

clippy changes to uplift and remove lint

f89f190

fix it in librustdoc

744b241

temp

63c5d38

fix clippy

8a919eb

merge clone_double_ref with noop_method_call

0dbc32d

rename to suspicious_double_ref_op and fix CI

82fbdaa

add example, adjust

dc7f055

fee1-dead force-pushed the uplift-clone-2x-ref branch from 0284bb4 to dc7f055 Compare April 14, 2023 05:54

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 14, 2023

compiler-errors reviewed Apr 28, 2023

View reviewed changes

compiler-errors self-assigned this Apr 28, 2023

fee1-dead marked this pull request as draft April 28, 2023 17:19

fee1-dead mentioned this pull request Apr 28, 2023

uplift clippy::clone_double_ref as suspicious_double_ref_op #110955

Merged

compiler-errors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 2, 2023

fee1-dead closed this May 24, 2023

fee1-dead deleted the uplift-clone-2x-ref branch May 24, 2023 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

uplift `clippy::clone_double_ref` to rustc #109842

uplift `clippy::clone_double_ref` to rustc #109842

fee1-dead commented Apr 1, 2023 •

edited by xFrednet

Loading

rustbot commented Apr 1, 2023

rustbot commented Apr 1, 2023

This comment has been minimized.

This comment has been minimized.

fee1-dead commented Apr 1, 2023

flip1995 left a comment

rustbot commented Apr 7, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Apr 10, 2023

fee1-dead commented Apr 14, 2023

compiler-errors Apr 28, 2023 •

edited

Loading

compiler-errors Apr 28, 2023 •

edited

Loading

compiler-errors Apr 28, 2023

compiler-errors Apr 28, 2023

bors commented May 2, 2023

	// Avoid overwhelming user with errors if borrow checking failed.
	// I'm not sure how helpful this is, to be honest, but it avoids a
	// lot of annoying errors in the ui tests (basically,
	// lint warnings and so on -- kindck used to do this abort, but
	// kindck is gone now). -nmatsakis
	if let Some(reported) = sess.has_errors() {
	return Err(reported);
	}

	warning: using `.deref()` on a double reference, which copies `&PlainType<u32>` instead of dereferencing the inner type
	warning: using `.deref()` on a double reference, which returns `&PlainType<u32>` instead of dereferencing the inner type

uplift clippy::clone_double_ref to rustc #109842

uplift clippy::clone_double_ref to rustc #109842

Conversation

fee1-dead commented Apr 1, 2023 • edited by xFrednet Loading

rustbot commented Apr 1, 2023

rustbot commented Apr 1, 2023

This comment has been minimized.

This comment has been minimized.

fee1-dead commented Apr 1, 2023

flip1995 left a comment

Choose a reason for hiding this comment

rustbot commented Apr 7, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Apr 10, 2023

fee1-dead commented Apr 14, 2023

compiler-errors Apr 28, 2023 • edited Loading

Choose a reason for hiding this comment

compiler-errors Apr 28, 2023 • edited Loading

Choose a reason for hiding this comment

compiler-errors Apr 28, 2023

Choose a reason for hiding this comment

compiler-errors Apr 28, 2023

Choose a reason for hiding this comment

bors commented May 2, 2023

uplift `clippy::clone_double_ref` to rustc #109842

uplift `clippy::clone_double_ref` to rustc #109842

fee1-dead commented Apr 1, 2023 •

edited by xFrednet

Loading

compiler-errors Apr 28, 2023 •

edited

Loading

compiler-errors Apr 28, 2023 •

edited

Loading