Arbitrary self types v2: detect shadowing problems.

This builds on the previous commits by actually adding checks for cases
where a new method shadows an older method.

Author: adetaylor

compiler/rustc_hir_typeck/src/method/probe.rs

@@ -1386,16 +1386,22 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         // the shadowing, because the autoderef-based maths wouldn't line up.
         // This is a niche case and we can live without generating an error
         // in the case of such shadowing.
-        let _potentially_shadowed_pick = self.pick_autorefd_method(
+        let potentially_shadowed_pick = self.pick_autorefd_method(
             step,
             self_ty,
             mutbl,
             &mut pick_diag_hints,
             Some(&pick_constraints),
         );
-
-        // At the moment, this function does no checks. A future
-        // commit will fill out the body here.
+        // Look for actual pairs of shadower/shadowed which are
+        // the sort of shadowing case we want to avoid. Specifically...
+        if let Some(Ok(possible_shadowed)) = potentially_shadowed_pick.as_ref() {
+            let sources = [possible_shadower, possible_shadowed]
+                .into_iter()
+                .map(|p| self.candidate_source_from_pick(p))
+                .collect();
+            return Err(MethodError::Ambiguity(sources));
+        }
         Ok(())
     }
 
@@ -1765,6 +1771,15 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         }
     }
 
+    fn candidate_source_from_pick(&self, pick: &Pick<'tcx>) -> CandidateSource {
+        match pick.kind {
+            InherentImplPick => CandidateSource::Impl(pick.item.container_id(self.tcx)),
+            ObjectPick | WhereClausePick(_) | TraitPick => {
+                CandidateSource::Trait(pick.item.container_id(self.tcx))
+            }
+        }
+    }
+
     #[instrument(level = "trace", skip(self, possibly_unsatisfied_predicates), ret)]
     fn consider_probe(
         &self,

tests/ui/self/arbitrary_self_types_by_value_reborrow.rs

@@ -0,0 +1,69 @@
+//@ run-pass
+
+#![feature(arbitrary_self_types)]
+#![allow(dead_code)]
+
+// With arbitrary self types v2, we show an error if there are
+// multiple contenders for a method call in an inner and outer type.
+// The goal is to avoid any possibility of confusion by a new
+// 'shadowing' method calling a 'shadowed' method.
+// However, there are niche circumstances where this
+// algorithm doesn't quite work, due to reborrows to get a different
+// lifetime. The test below explicitly tests those circumstances to ensure
+// the behavior is as expected, even if it's not 100% desirable. They're
+// very niche circumstances.
+
+#[derive(Debug, PartialEq)]
+enum Callee {
+    INNER,
+    OUTER
+}
+
+struct MyNonNull<T>(T);
+
+impl<T> std::ops::Receiver for MyNonNull<T> {
+    type Target = T;
+}
+
+struct A;
+impl A {
+    fn foo(self: MyNonNull<A>) -> Callee {
+        Callee::INNER
+    }
+
+    fn bar(self: &MyNonNull<A>) -> Callee {
+        Callee::INNER
+    }
+
+    fn baz(self: &&MyNonNull<A>) -> Callee {
+        // note this is by DOUBLE reference
+        Callee::INNER
+    }
+}
+
+impl<T> MyNonNull<T> {
+    fn foo(&self) -> Callee{
+        Callee::OUTER
+    }
+
+    fn bar(&self) -> Callee{
+        Callee::OUTER
+    }
+
+    fn baz(&self) -> Callee{
+        Callee::OUTER
+    }
+}
+
+fn main() {
+    // The normal deshadowing case. Does not compile.
+    // assert_eq!(MyNonNull(A).foo(), Callee::INNER);
+
+    // Similarly, does not compile.
+    //assert_eq!(MyNonNull(A).bar(), Callee::INNER);
+
+    // The double-reference case.
+    // We call the newly-added outer type method.
+    // Not ideal but very niche so we accept it.
+    assert_eq!(MyNonNull(A).baz(), Callee::OUTER);
+}

tests/ui/self/arbitrary_self_types_shadowing_val_constptr.rs

@@ -0,0 +1,33 @@
+//@ run-pass
+
+#![feature(arbitrary_self_types)]
+#![feature(arbitrary_self_types_pointers)]
+
+pub struct A;
+
+impl A {
+    pub fn f(self: *const MyNonNull<Self>) -> i32 { 1 }
+}
+
+pub struct MyNonNull<T>(T);
+
+impl<T> core::ops::Receiver for MyNonNull<T> {
+    type Target = T;
+}
+
+impl<T> MyNonNull<T> {
+    // Imagine this a NEW method in B<T> shadowing an EXISTING
+    // method in A.
+    pub fn f(self: *mut Self) -> i32 {
+        2
+    }
+}
+
+fn main() {
+    let mut b = MyNonNull(A);
+    let b = &mut b;
+    let b = b as *mut MyNonNull<A>;
+    // We actually allow the shadowing in the case of const vs mut raw
+    // pointer receivers.
+    assert_eq!(b.f(), 2);
+}

tests/ui/self/arbitrary_self_types_unshadowing.rs

@@ -0,0 +1,55 @@
+#![feature(arbitrary_self_types)]
+
+pub struct A;
+
+// The receiver of the potentially shadowed method
+// precisely matches that of the shadower
+impl A {
+    pub fn f(self: Wrapper<Self>) -> i32 { 1 }
+    pub fn g(self: &Wrapper<Self>) -> i32 { 2 }
+    pub fn h(self: &mut Wrapper<Self>) -> i32 { 3 }
+}
+
+// The receiver of the potentially shadowed method is a reference
+pub struct B;
+
+impl B {
+    pub fn f(self: &Wrapper<Self>) -> i32 { 9 }
+}
+
+// The receiver of the potentially shadowed method is a mut reference
+
+pub struct C;
+
+impl C {
+    pub fn f(self: &mut Wrapper<Self>) -> i32 { 10 }
+    pub fn g(self: &mut Wrapper<Self>) -> i32 { 11 }
+}
+
+pub struct Wrapper<T>(T);
+
+impl<T> core::ops::Receiver for Wrapper<T> {
+    type Target = T;
+}
+
+impl<T> Wrapper<T> {
+    pub fn f(self) -> i32 { 5 }
+    pub fn g(&self) -> i32 { 6 }
+    pub fn h(&mut self) -> i32 { 7 }
+}
+
+fn main() {
+    assert_eq!(Wrapper(A).f(), 1);
+    //~^ ERROR: multiple applicable items in scope
+    assert_eq!(Wrapper(A).g(), 2);
+    //~^ ERROR: multiple applicable items in scope
+    assert_eq!(Wrapper(A).h(), 3);
+    //~^ ERROR: multiple applicable items in scope
+    let a = Wrapper(A);
+    assert_eq!(Wrapper(B).f(), 9);
+    //~^ ERROR: multiple applicable items in scope
+    assert_eq!(Wrapper(C).f(), 10);
+    //~^ ERROR: multiple applicable items in scope
+    assert_eq!(Wrapper(C).g(), 11);
+    //~^ ERROR: multiple applicable items in scope
+}

tests/ui/self/arbitrary_self_types_unshadowing.stderr

@@ -0,0 +1,105 @@
+error[E0034]: multiple applicable items in scope
+  --> $DIR/arbitrary_self_types_unshadowing.rs:42:27
+   |
+LL |     assert_eq!(Wrapper(A).f(), 1);
+   |                           ^ multiple `f` found
+   |
+note: candidate #1 is defined in an impl for the type `A`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:8:5
+   |
+LL |     pub fn f(self: Wrapper<Self>) -> i32 { 1 }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: candidate #2 is defined in an impl for the type `Wrapper<T>`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:36:5
+   |
+LL |     pub fn f(self) -> i32 { 5 }
+   |     ^^^^^^^^^^^^^^^^^^^^^
+
+error[E0034]: multiple applicable items in scope
+  --> $DIR/arbitrary_self_types_unshadowing.rs:44:27
+   |
+LL |     assert_eq!(Wrapper(A).g(), 2);
+   |                           ^ multiple `g` found
+   |
+note: candidate #1 is defined in an impl for the type `A`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:9:5
+   |
+LL |     pub fn g(self: &Wrapper<Self>) -> i32 { 2 }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: candidate #2 is defined in an impl for the type `Wrapper<T>`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:37:5
+   |
+LL |     pub fn g(&self) -> i32 { 6 }
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
+error[E0034]: multiple applicable items in scope
+  --> $DIR/arbitrary_self_types_unshadowing.rs:46:27
+   |
+LL |     assert_eq!(Wrapper(A).h(), 3);
+   |                           ^ multiple `h` found
+   |
+note: candidate #1 is defined in an impl for the type `A`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:10:5
+   |
+LL |     pub fn h(self: &mut Wrapper<Self>) -> i32 { 3 }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: candidate #2 is defined in an impl for the type `Wrapper<T>`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:38:5
+   |
+LL |     pub fn h(&mut self) -> i32 { 7 }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error[E0034]: multiple applicable items in scope
+  --> $DIR/arbitrary_self_types_unshadowing.rs:49:27
+   |
+LL |     assert_eq!(Wrapper(B).f(), 9);
+   |                           ^ multiple `f` found
+   |
+note: candidate #1 is defined in an impl for the type `B`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:17:5
+   |
+LL |     pub fn f(self: &Wrapper<Self>) -> i32 { 9 }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: candidate #2 is defined in an impl for the type `Wrapper<T>`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:36:5
+   |
+LL |     pub fn f(self) -> i32 { 5 }
+   |     ^^^^^^^^^^^^^^^^^^^^^
+
+error[E0034]: multiple applicable items in scope
+  --> $DIR/arbitrary_self_types_unshadowing.rs:51:27
+   |
+LL |     assert_eq!(Wrapper(C).f(), 10);
+   |                           ^ multiple `f` found
+   |
+note: candidate #1 is defined in an impl for the type `C`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:25:5
+   |
+LL |     pub fn f(self: &mut Wrapper<Self>) -> i32 { 10 }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: candidate #2 is defined in an impl for the type `Wrapper<T>`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:36:5
+   |
+LL |     pub fn f(self) -> i32 { 5 }
+   |     ^^^^^^^^^^^^^^^^^^^^^
+
+error[E0034]: multiple applicable items in scope
+  --> $DIR/arbitrary_self_types_unshadowing.rs:53:27
+   |
+LL |     assert_eq!(Wrapper(C).g(), 11);
+   |                           ^ multiple `g` found
+   |
+note: candidate #1 is defined in an impl for the type `C`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:26:5
+   |
+LL |     pub fn g(self: &mut Wrapper<Self>) -> i32 { 11 }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: candidate #2 is defined in an impl for the type `Wrapper<T>`
+  --> $DIR/arbitrary_self_types_unshadowing.rs:37:5
+   |
+LL |     pub fn g(&self) -> i32 { 6 }
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 6 previous errors
+
+For more information about this error, try `rustc --explain E0034`.

tests/ui/self/arbitrary_self_types_unshadowing_ptrs.rs

@@ -0,0 +1,61 @@
+#![feature(arbitrary_self_types_pointers)]
+#![feature(arbitrary_self_types)]
+
+pub struct A;
+
+// The receiver of the potentially shadowed method
+// precisely matches that of the shadower
+impl A {
+    pub fn f(self: Wrapper<Self>) -> i32 { 1 }
+    pub fn g(self: &Wrapper<Self>) -> i32 { 2 }
+    pub fn h(self: &mut Wrapper<Self>) -> i32 { 3 }
+    pub fn i(self: *const Wrapper<Self>) -> i32 { 4 }
+}
+
+// The receiver of the potentially shadowed method is a reference
+pub struct B;
+
+impl B {
+    pub fn f(self: &Wrapper<Self>) -> i32 { 9 }
+}
+
+// The receiver of the potentially shadowed method is a mut reference
+
+pub struct C;
+
+impl C {
+    pub fn f(self: &mut Wrapper<Self>) -> i32 { 10 }
+    pub fn g(self: &mut Wrapper<Self>) -> i32 { 11 }
+}
+
+pub struct Wrapper<T>(T);
+
+impl<T> core::ops::Receiver for Wrapper<T> {
+    type Target = T;
+}
+
+impl<T> Wrapper<T> {
+    pub fn f(self) -> i32 { 5 }
+    pub fn g(&self) -> i32 { 6 }
+    pub fn h(&mut self) -> i32 { 7 }
+    pub fn i(self: *const Self) -> i32 { 8 }
+}
+
+fn main() {
+    assert_eq!(Wrapper(A).f(), 1);
+    //~^ ERROR: multiple applicable items in scope
+    assert_eq!(Wrapper(A).g(), 2);
+    //~^ ERROR: multiple applicable items in scope
+    assert_eq!(Wrapper(A).h(), 3);
+    //~^ ERROR: multiple applicable items in scope
+    let a = Wrapper(A);
+    let a_ptr = &a as *const Wrapper<A>;
+    assert_eq!(a_ptr.i(), 4);
+    //~^ ERROR: multiple applicable items in scope
+    assert_eq!(Wrapper(B).f(), 9);
+    //~^ ERROR: multiple applicable items in scope
+    assert_eq!(Wrapper(C).f(), 10);
+    //~^ ERROR: multiple applicable items in scope
+    assert_eq!(Wrapper(C).g(), 11);
+    //~^ ERROR: multiple applicable items in scope
+}