Auto merge of #57885 - arielb1:xform-probe, r=nikomatsakis

Avoid committing to autoderef in object method probing

This fixes the "leak" introduced in #57835 (see test for details, also apparently #54252 had no tests for the "leaks" that were fixed in it, so go ahead and add one).

Maybe beta-nominating because regression, but I'm against landing things on beta we don't have to.

r? @nikomatsakis

src/librustc_typeck/check/method/probe.rs

@@ -85,6 +85,37 @@ impl<'a, 'gcx, 'tcx> Deref for ProbeContext<'a, 'gcx, 'tcx> {
 
 #[derive(Debug)]
 struct Candidate<'tcx> {
+    // Candidates are (I'm not quite sure, but they are mostly) basically
+    // some metadata on top of a `ty::AssociatedItem` (without substs).
+    //
+    // However, method probing wants to be able to evaluate the predicates
+    // for a function with the substs applied - for example, if a function
+    // has `where Self: Sized`, we don't want to consider it unless `Self`
+    // is actually `Sized`, and similarly, return-type suggestions want
+    // to consider the "actual" return type.
+    //
+    // The way this is handled is through `xform_self_ty`. It contains
+    // the receiver type of this candidate, but `xform_self_ty`,
+    // `xform_ret_ty` and `kind` (which contains the predicates) have the
+    // generic parameters of this candidate substituted with the *same set*
+    // of inference variables, which acts as some weird sort of "query".
+    //
+    // When we check out a candidate, we require `xform_self_ty` to be
+    // a subtype of the passed-in self-type, and this equates the type
+    // variables in the rest of the fields.
+    //
+    // For example, if we have this candidate:
+    // ```
+    //    trait Foo {
+    //        fn foo(&self) where Self: Sized;
+    //    }
+    // ```
+    //
+    // Then `xform_self_ty` will be `&'erased ?X` and `kind` will contain
+    // the predicate `?X: Sized`, so if we are evaluating `Foo` for a
+    // the receiver `&T`, we'll do the subtyping which will make `?X`
+    // get the right value, then when we evaluate the predicate we'll check
+    // if `T: Sized`.
     xform_self_ty: Ty<'tcx>,
     xform_ret_ty: Option<Ty<'tcx>>,
     item: ty::AssociatedItem,
@@ -506,13 +537,28 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
         match self_ty.value.value.sty {
             ty::Dynamic(ref data, ..) => {
                 if let Some(p) = data.principal() {
-                    let InferOk { value: instantiated_self_ty, obligations: _ } =
-                        self.fcx.probe_instantiate_query_response(
-                            self.span, &self.orig_steps_var_values, self_ty)
-                        .unwrap_or_else(|_| {
-                            span_bug!(self.span, "{:?} was applicable but now isn't?", self_ty)
-                        });
-                    self.assemble_inherent_candidates_from_object(instantiated_self_ty);
+                    // Subtle: we can't use `instantiate_query_response` here: using it will
+                    // commit to all of the type equalities assumed by inference going through
+                    // autoderef (see the `method-probe-no-guessing` test).
+                    //
+                    // However, in this code, it is OK if we end up with an object type that is
+                    // "more general" than the object type that we are evaluating. For *every*
+                    // object type `MY_OBJECT`, a function call that goes through a trait-ref
+                    // of the form `<MY_OBJECT as SuperTraitOf(MY_OBJECT)>::func` is a valid
+                    // `ObjectCandidate`, and it should be discoverable "exactly" through one
+                    // of the iterations in the autoderef loop, so there is no problem with it
+                    // being discoverable in another one of these iterations.
+                    //
+                    // Using `instantiate_canonical_with_fresh_inference_vars` on our
+                    // `Canonical<QueryResponse<Ty<'tcx>>>` and then *throwing away* the
+                    // `CanonicalVarValues` will exactly give us such a generalization - it
+                    // will still match the original object type, but it won't pollute our
+                    // type variables in any form, so just do that!
+                    let (QueryResponse { value: generalized_self_ty, .. }, _ignored_var_values) =
+                        self.fcx.instantiate_canonical_with_fresh_inference_vars(
+                            self.span, &self_ty);
+
+                    self.assemble_inherent_candidates_from_object(generalized_self_ty);
                     self.assemble_inherent_impl_candidates_for_type(p.def_id());
                 }
             }

src/test/run-pass/methods/method-probe-no-guessing-dyn-trait.rs

@@ -0,0 +1,59 @@
+// Check that method matching does not make "guesses" depending on
+// Deref impls that don't eventually end up being picked.
+
+use std::ops::Deref;
+
+// An impl with less derefs will get called over an impl with more derefs,
+// so `(t: Foo<_>).my_fn()` will use `<Foo<u32> as MyTrait1>::my_fn(t)`,
+// and does *not* force the `_` to equal `()`, because the Deref impl
+// was *not* used.
+
+trait MyTrait1 {
+    fn my_fn(&self) {}
+}
+
+impl MyTrait1 for Foo<u32> {}
+
+struct Foo<T>(T);
+
+impl Deref for Foo<()> {
+    type Target = dyn MyTrait1 + 'static;
+    fn deref(&self) -> &(dyn MyTrait1 + 'static) {
+        panic!()
+    }
+}
+
+// ...but if there is no impl with less derefs, the "guess" will be
+// forced, so `(t: Bar<_>).my_fn2()` is `<dyn MyTrait2 as MyTrait2>::my_fn2(*t)`,
+// and because the deref impl is used, the `_` is forced to equal `u8`.
+
+trait MyTrait2 {
+    fn my_fn2(&self) {}
+}
+
+impl MyTrait2 for u32 {}
+struct Bar<T>(T, u32);
+impl Deref for Bar<u8> {
+    type Target = dyn MyTrait2 + 'static;
+    fn deref(&self) -> &(dyn MyTrait2 + 'static) {
+        &self.1
+    }
+}
+
+// actually invoke things
+
+fn main() {
+    let mut foo: Option<Foo<_>> = None;
+    let mut bar: Option<Bar<_>> = None;
+    let mut first_iter = true;
+    loop {
+        if !first_iter {
+            foo.as_ref().unwrap().my_fn();
+            bar.as_ref().unwrap().my_fn2();
+            break;
+        }
+        foo = Some(Foo(0));
+        bar = Some(Bar(Default::default(), 0));
+        first_iter = false;
+    }
+}

src/test/ui/methods/method-deref-to-same-trait-object-with-separate-params.rs

@@ -0,0 +1,177 @@
+#![feature(arbitrary_self_types, coerce_unsized, dispatch_from_dyn, unsize, unsized_locals)]
+
+// This tests a few edge-cases around `arbitrary_self_types`. Most specifically,
+// it checks that the `ObjectCandidate` you get from method matching can't
+// match a trait with the same DefId as a supertrait but a bad type parameter.
+
+use std::marker::PhantomData;
+
+mod internal {
+    use std::ops::{CoerceUnsized, Deref, DispatchFromDyn};
+    use std::marker::{PhantomData, Unsize};
+
+    pub struct Smaht<T: ?Sized, MISC>(pub Box<T>, pub PhantomData<MISC>);
+
+    impl<T: ?Sized, MISC> Deref for Smaht<T, MISC> {
+        type Target = T;
+
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl<T: ?Sized + Unsize<U>, U: ?Sized, MISC> CoerceUnsized<Smaht<U, MISC>>
+        for Smaht<T, MISC>
+    {}
+    impl<T: ?Sized + Unsize<U>, U: ?Sized, MISC> DispatchFromDyn<Smaht<U, MISC>>
+        for Smaht<T, MISC>
+    {}
+
+    pub trait Foo: X<u32> {}
+    pub trait X<T> {
+        fn foo(self: Smaht<Self, T>) -> T;
+    }
+
+    impl X<u32> for () {
+        fn foo(self: Smaht<Self, u32>) -> u32 {
+            0
+        }
+    }
+
+    pub trait Marker {}
+    impl Marker for dyn Foo {}
+    impl<T: Marker + ?Sized> X<u64> for T {
+        fn foo(self: Smaht<Self, u64>) -> u64 {
+            1
+        }
+    }
+
+    impl Deref for dyn Foo {
+        type Target = ();
+        fn deref(&self) -> &() { &() }
+    }
+
+    impl Foo for () {}
+}
+
+pub trait FinalFoo {
+    fn foo(&self) -> u8;
+}
+
+impl FinalFoo for () {
+    fn foo(&self) -> u8 { 0 }
+}
+
+mod nuisance_foo {
+    pub trait NuisanceFoo {
+        fn foo(self);
+    }
+
+    impl<T: ?Sized> NuisanceFoo for T {
+        fn foo(self) {}
+    }
+}
+
+
+fn objectcandidate_impl() {
+    let x: internal::Smaht<(), u32> = internal::Smaht(Box::new(()), PhantomData);
+    let x: internal::Smaht<dyn internal::Foo, u32> = x;
+
+    // This picks `<dyn internal::Foo as X<u32>>::foo` via `ObjectCandidate`.
+    //
+    // The `TraitCandidate` is not relevant because `X` is not in scope.
+    let z = x.foo();
+
+    // Observe the type of `z` is `u32`
+    let _seetype: () = z; //~ ERROR mismatched types
+    //~| expected (), found u32
+}
+
+fn traitcandidate_impl() {
+    use internal::X;
+
+    let x: internal::Smaht<(), u64> = internal::Smaht(Box::new(()), PhantomData);
+    let x: internal::Smaht<dyn internal::Foo, u64> = x;
+
+    // This picks `<dyn internal::Foo as X<u64>>::foo` via `TraitCandidate`.
+    //
+    // The `ObjectCandidate` does not apply, as it only applies to
+    // `X<u32>` (and not `X<u64>`).
+    let z = x.foo();
+
+    // Observe the type of `z` is `u64`
+    let _seetype: () = z; //~ ERROR mismatched types
+    //~| expected (), found u64
+}
+
+fn traitcandidate_impl_with_nuisance() {
+    use internal::X;
+    use nuisance_foo::NuisanceFoo;
+
+    let x: internal::Smaht<(), u64> = internal::Smaht(Box::new(()), PhantomData);
+    let x: internal::Smaht<dyn internal::Foo, u64> = x;
+
+    // This picks `<dyn internal::Foo as X<u64>>::foo` via `TraitCandidate`.
+    //
+    // The `ObjectCandidate` does not apply, as it only applies to
+    // `X<u32>` (and not `X<u64>`).
+    //
+    // The NuisanceFoo impl has the same priority as the `X` impl,
+    // so we get a conflict.
+    let z = x.foo(); //~ ERROR multiple applicable items in scope
+}
+
+
+fn neither_impl() {
+    let x: internal::Smaht<(), u64> = internal::Smaht(Box::new(()), PhantomData);
+    let x: internal::Smaht<dyn internal::Foo, u64> = x;
+
+    // This can't pick the `TraitCandidate` impl, because `Foo` is not
+    // imported. However, this also can't pick the `ObjectCandidate`
+    // impl, because it only applies to `X<u32>` (and not `X<u64>`).
+    //
+    // Therefore, neither of the candidates is applicable, and we pick
+    // the `FinalFoo` impl after another deref, which will return `u8`.
+    let z = x.foo();
+
+    // Observe the type of `z` is `u8`
+    let _seetype: () = z; //~ ERROR mismatched types
+    //~| expected (), found u8
+}
+
+fn both_impls() {
+    use internal::X;
+
+    let x: internal::Smaht<(), u32> = internal::Smaht(Box::new(()), PhantomData);
+    let x: internal::Smaht<dyn internal::Foo, u32> = x;
+
+    // This can pick both the `TraitCandidate` and the `ObjectCandidate` impl.
+    //
+    // However, the `ObjectCandidate` is considered an "inherent candidate",
+    // and therefore has priority over both the `TraitCandidate` as well as
+    // any other "nuisance" candidate" (if present).
+    let z = x.foo();
+
+    // Observe the type of `z` is `u32`
+    let _seetype: () = z; //~ ERROR mismatched types
+    //~| expected (), found u32
+}
+
+
+fn both_impls_with_nuisance() {
+    // Similar to the `both_impls` example, except with a nuisance impl to
+    // make sure the `ObjectCandidate` indeed has a higher priority.
+
+    use internal::X;
+    use nuisance_foo::NuisanceFoo;
+
+    let x: internal::Smaht<(), u32> = internal::Smaht(Box::new(()), PhantomData);
+    let x: internal::Smaht<dyn internal::Foo, u32> = x;
+    let z = x.foo();
+
+    // Observe the type of `z` is `u32`
+    let _seetype: () = z; //~ ERROR mismatched types
+    //~| expected (), found u32
+}
+
+fn main() {
+}

src/test/ui/methods/method-deref-to-same-trait-object-with-separate-params.stderr

@@ -0,0 +1,72 @@
+error[E0308]: mismatched types
+  --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:85:24
+   |
+LL |     let _seetype: () = z; //~ ERROR mismatched types
+   |                        ^ expected (), found u32
+   |
+   = note: expected type `()`
+              found type `u32`
+
+error[E0308]: mismatched types
+  --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:102:24
+   |
+LL |     let _seetype: () = z; //~ ERROR mismatched types
+   |                        ^ expected (), found u64
+   |
+   = note: expected type `()`
+              found type `u64`
+
+error[E0034]: multiple applicable items in scope
+  --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:120:15
+   |
+LL |     let z = x.foo(); //~ ERROR multiple applicable items in scope
+   |               ^^^ multiple `foo` found
+   |
+note: candidate #1 is defined in an impl of the trait `internal::X` for the type `_`
+  --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:43:9
+   |
+LL |         fn foo(self: Smaht<Self, u64>) -> u64 {
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: candidate #2 is defined in an impl of the trait `nuisance_foo::NuisanceFoo` for the type `_`
+  --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:70:9
+   |
+LL |         fn foo(self) {}
+   |         ^^^^^^^^^^^^
+note: candidate #3 is defined in the trait `FinalFoo`
+  --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:57:5
+   |
+LL |     fn foo(&self) -> u8;
+   |     ^^^^^^^^^^^^^^^^^^^^
+   = help: to disambiguate the method call, write `FinalFoo::foo(x)` instead
+
+error[E0308]: mismatched types
+  --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:137:24
+   |
+LL |     let _seetype: () = z; //~ ERROR mismatched types
+   |                        ^ expected (), found u8
+   |
+   = note: expected type `()`
+              found type `u8`
+
+error[E0308]: mismatched types
+  --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:155:24
+   |
+LL |     let _seetype: () = z; //~ ERROR mismatched types
+   |                        ^ expected (), found u32
+   |
+   = note: expected type `()`
+              found type `u32`
+
+error[E0308]: mismatched types
+  --> $DIR/method-deref-to-same-trait-object-with-separate-params.rs:172:24
+   |
+LL |     let _seetype: () = z; //~ ERROR mismatched types
+   |                        ^ expected (), found u32
+   |
+   = note: expected type `()`
+              found type `u32`
+
+error: aborting due to 6 previous errors
+
+Some errors occurred: E0034, E0308.
+For more information about an error, try `rustc --explain E0034`.