Prefer removing a redundant shared reference over reborrow

compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs

@@ -478,7 +478,13 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                             leaf_trait_predicate,
                         );
                         suggested |=
-                            self.suggest_dereferences(&obligation, &mut err, leaf_trait_predicate);
+                            self.suggest_dereferences(&obligation, &mut err, leaf_trait_predicate)
+                                || self.suggest_remove_reference(
+                                    &obligation,
+                                    &mut err,
+                                    leaf_trait_predicate,
+                                );
+
                         suggested |=
                             self.suggest_fn_call(&obligation, &mut err, leaf_trait_predicate);
                         suggested |= self.suggest_cast_to_fn_pointer(
@@ -488,11 +494,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                             main_trait_predicate,
                             span,
                         );
-                        suggested |= self.suggest_remove_reference(
-                            &obligation,
-                            &mut err,
-                            leaf_trait_predicate,
-                        );
+
                         suggested |= self.suggest_semicolon_removal(
                             &obligation,
                             &mut err,

compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs

@@ -780,6 +780,17 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                 if span.in_external_macro(self.tcx.sess.source_map()) {
                     return false;
                 }
+
+                // For a shared reference, prefer removing the outer `&` over suggesting
+                // `&*reference`. Keep the reborrow for `&mut T` and smart pointers.
+                if is_under_ref.is_some()
+                    && steps == 1
+                    && matches!(base_ty.kind(), ty::Ref(_, _, hir::Mutability::Not))
+                    && !expr.span.from_expansion()
+                    && self.suggest_remove_reference(obligation, err, real_trait_pred)
+                {
+                    return true;
+                }
                 let derefs = "*".repeat(steps);
                 let msg = "consider dereferencing here";
 

tests/ui/suggestions/redundant-shared-reference-issue-133685.fixed

@@ -0,0 +1,46 @@
+//! Regression test for https://github.com/rust-lang/rust/issues/133685.
+//! Prefer removing an extra shared borrow over reborrowing an existing shared reference.
+
+//@ run-rustfix
+
+#![allow(unused_parens)]
+
+fn consume<'a>(_: impl IntoIterator<Item = &'a i32>) {}
+
+trait Value {}
+struct Source;
+impl Value for &Source {}
+fn consume_value(_: impl Value) {}
+
+fn main() {
+    let a: Vec<i32> = Vec::new();
+    let ref_a = &a;
+    let mut b: Vec<i32> = Vec::new();
+    b.extend(ref_a);
+    //~^ ERROR is not an iterator
+
+    consume(ref_a);
+    //~^ ERROR is not an iterator
+    consume((ref_a));
+    //~^ ERROR is not an iterator
+
+    let slice = &a[..];
+    consume(slice);
+    //~^ ERROR is not an iterator
+
+    // These still need a dereference: neither operand has the required shared-reference type.
+    let boxed = Box::new(a.clone());
+    consume(&*boxed);
+    //~^ ERROR is not an iterator
+
+    let mut values = a.clone();
+    let mut_ref = &mut values;
+    consume(&*mut_ref);
+    //~^ ERROR is not an iterator
+
+    // Also cover a direct trait bound without the Iterator-to-IntoIterator blanket impl.
+    let source = Source;
+    let shared = &source;
+    consume_value(shared);
+    //~^ ERROR the trait bound
+}

tests/ui/suggestions/redundant-shared-reference-issue-133685.rs

@@ -0,0 +1,46 @@
+//! Regression test for https://github.com/rust-lang/rust/issues/133685.
+//! Prefer removing an extra shared borrow over reborrowing an existing shared reference.
+
+//@ run-rustfix
+
+#![allow(unused_parens)]
+
+fn consume<'a>(_: impl IntoIterator<Item = &'a i32>) {}
+
+trait Value {}
+struct Source;
+impl Value for &Source {}
+fn consume_value(_: impl Value) {}
+
+fn main() {
+    let a: Vec<i32> = Vec::new();
+    let ref_a = &a;
+    let mut b: Vec<i32> = Vec::new();
+    b.extend(&ref_a);
+    //~^ ERROR is not an iterator
+
+    consume(&ref_a);
+    //~^ ERROR is not an iterator
+    consume((&ref_a));
+    //~^ ERROR is not an iterator
+
+    let slice = &a[..];
+    consume(&slice);
+    //~^ ERROR is not an iterator
+
+    // These still need a dereference: neither operand has the required shared-reference type.
+    let boxed = Box::new(a.clone());
+    consume(&boxed);
+    //~^ ERROR is not an iterator
+
+    let mut values = a.clone();
+    let mut_ref = &mut values;
+    consume(&mut_ref);
+    //~^ ERROR is not an iterator
+
+    // Also cover a direct trait bound without the Iterator-to-IntoIterator blanket impl.
+    let source = Source;
+    let shared = &source;
+    consume_value(&shared);
+    //~^ ERROR the trait bound
+}

tests/ui/suggestions/redundant-shared-reference-issue-133685.stderr

@@ -0,0 +1,143 @@
+error[E0277]: `&&Vec<i32>` is not an iterator
+  --> $DIR/redundant-shared-reference-issue-133685.rs:19:14
+   |
+LL |     b.extend(&ref_a);
+   |       ------ ^^^^^^ `&&Vec<i32>` is not an iterator
+   |       |
+   |       required by a bound introduced by this call
+   |
+   = help: the trait `Iterator` is not implemented for `&&Vec<i32>`
+   = note: required for `&&Vec<i32>` to implement `IntoIterator`
+note: required by a bound in `extend`
+  --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
+help: consider removing the leading `&`-reference
+   |
+LL -     b.extend(&ref_a);
+LL +     b.extend(ref_a);
+   |
+
+error[E0277]: `&&Vec<i32>` is not an iterator
+  --> $DIR/redundant-shared-reference-issue-133685.rs:22:13
+   |
+LL |     consume(&ref_a);
+   |     ------- ^^^^^^ `&&Vec<i32>` is not an iterator
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the trait `Iterator` is not implemented for `&&Vec<i32>`
+   = note: required for `&&Vec<i32>` to implement `IntoIterator`
+note: required by a bound in `consume`
+  --> $DIR/redundant-shared-reference-issue-133685.rs:8:24
+   |
+LL | fn consume<'a>(_: impl IntoIterator<Item = &'a i32>) {}
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `consume`
+help: consider removing the leading `&`-reference
+   |
+LL -     consume(&ref_a);
+LL +     consume(ref_a);
+   |
+
+error[E0277]: `&&Vec<i32>` is not an iterator
+  --> $DIR/redundant-shared-reference-issue-133685.rs:24:13
+   |
+LL |     consume((&ref_a));
+   |     ------- ^^^^^^^^ `&&Vec<i32>` is not an iterator
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the trait `Iterator` is not implemented for `&&Vec<i32>`
+   = note: required for `&&Vec<i32>` to implement `IntoIterator`
+note: required by a bound in `consume`
+  --> $DIR/redundant-shared-reference-issue-133685.rs:8:24
+   |
+LL | fn consume<'a>(_: impl IntoIterator<Item = &'a i32>) {}
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `consume`
+help: consider removing the leading `&`-reference
+   |
+LL -     consume((&ref_a));
+LL +     consume((ref_a));
+   |
+
+error[E0277]: `&&[i32]` is not an iterator
+  --> $DIR/redundant-shared-reference-issue-133685.rs:28:13
+   |
+LL |     consume(&slice);
+   |     ------- ^^^^^^ `&&[i32]` is not an iterator
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the trait `Iterator` is not implemented for `&&[i32]`
+   = note: required for `&&[i32]` to implement `IntoIterator`
+note: required by a bound in `consume`
+  --> $DIR/redundant-shared-reference-issue-133685.rs:8:24
+   |
+LL | fn consume<'a>(_: impl IntoIterator<Item = &'a i32>) {}
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `consume`
+help: consider removing the leading `&`-reference
+   |
+LL -     consume(&slice);
+LL +     consume(slice);
+   |
+
+error[E0277]: `&Box<Vec<i32>>` is not an iterator
+  --> $DIR/redundant-shared-reference-issue-133685.rs:33:13
+   |
+LL |     consume(&boxed);
+   |     ------- ^^^^^^ `&Box<Vec<i32>>` is not an iterator
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the trait `Iterator` is not implemented for `&Box<Vec<i32>>`
+   = note: required for `&Box<Vec<i32>>` to implement `IntoIterator`
+note: required by a bound in `consume`
+  --> $DIR/redundant-shared-reference-issue-133685.rs:8:24
+   |
+LL | fn consume<'a>(_: impl IntoIterator<Item = &'a i32>) {}
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `consume`
+help: consider dereferencing here
+   |
+LL |     consume(&*boxed);
+   |              +
+
+error[E0277]: `&&mut Vec<i32>` is not an iterator
+  --> $DIR/redundant-shared-reference-issue-133685.rs:38:13
+   |
+LL |     consume(&mut_ref);
+   |     ------- ^^^^^^^^ `&&mut Vec<i32>` is not an iterator
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the trait `Iterator` is not implemented for `&&mut Vec<i32>`
+   = note: required for `&&mut Vec<i32>` to implement `IntoIterator`
+note: required by a bound in `consume`
+  --> $DIR/redundant-shared-reference-issue-133685.rs:8:24
+   |
+LL | fn consume<'a>(_: impl IntoIterator<Item = &'a i32>) {}
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `consume`
+help: consider dereferencing here
+   |
+LL |     consume(&*mut_ref);
+   |              +
+
+error[E0277]: the trait bound `&&Source: Value` is not satisfied
+  --> $DIR/redundant-shared-reference-issue-133685.rs:44:19
+   |
+LL |     consume_value(&shared);
+   |     ------------- ^^^^^^^ the trait `Value` is not implemented for `&&Source`
+   |     |
+   |     required by a bound introduced by this call
+   |
+note: required by a bound in `consume_value`
+  --> $DIR/redundant-shared-reference-issue-133685.rs:13:26
+   |
+LL | fn consume_value(_: impl Value) {}
+   |                          ^^^^^ required by this bound in `consume_value`
+help: consider removing the leading `&`-reference
+   |
+LL -     consume_value(&shared);
+LL +     consume_value(shared);
+   |
+
+error: aborting due to 7 previous errors
+
+For more information about this error, try `rustc --explain E0277`.