Modify turbofish suggestion probing logic

estebank · estebank · commit af13f34ffe9f · 2020-10-23T13:02:16.000-07:00
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
@@ -1462,8 +1462,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
         let bound_predicate = predicate.bound_atom();
         let mut err = match bound_predicate.skip_binder() {
             ty::PredicateAtom::Trait(data, _) => {
-                let trait_ref = bound_predicate.rebind(data.trait_ref);
-                debug!("trait_ref {:?}", trait_ref);
+                let self_ty = data.trait_ref.self_ty();
 
                 if predicate.references_error() {
                     return;
@@ -1504,7 +1503,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
                 // avoid inundating the user with unnecessary errors, but we now
                 // check upstream for type errors and don't add the obligations to
                 // begin with in those cases.
-                if self.tcx.lang_items().sized_trait() == Some(trait_ref.def_id()) {
+                if self.tcx.lang_items().sized_trait() == Some(data.trait_ref.def_id) {
                     self.emit_inference_failure_err(body_id, span, subst, ErrorCode::E0282, &[])
                         .emit();
                     return;
@@ -1523,7 +1522,12 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
                 err.note(&format!("cannot satisfy `{}`", predicate));
 
                 if let ObligationCauseCode::ItemObligation(def_id) = obligation.cause.code {
-                    self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id());
+                    self.suggest_fully_qualified_path(
+                        &mut err,
+                        def_id,
+                        span,
+                        data.trait_ref.def_id,
+                    );
                 } else if let (
                     Ok(ref snippet),
                     ObligationCauseCode::BindingObligation(ref def_id, _),
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -4,7 +4,7 @@ use super::{
 };
 
 use crate::autoderef::Autoderef;
-use crate::infer::{InferCtxt, InferOk};
+use crate::infer::InferCtxt;
 use crate::traits::{self, normalize_projection_type};
 
 use rustc_data_structures::fx::FxHashSet;
@@ -16,12 +16,11 @@ use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::lang_items::LangItem;
 use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node};
-use rustc_middle::ty::subst::{GenericArgKind, Subst};
+use rustc_middle::ty::subst::InternalSubsts;
 use rustc_middle::ty::{
     self, suggest_constraining_type_param, AdtKind, DefIdTree, Infer, InferTy, ToPredicate, Ty,
-    TyCtxt, TypeFoldable, WithConstness,
+    TyCtxt, TypeAndMut, TypeFoldable, TypeckResults, WithConstness,
 };
-use rustc_middle::ty::{TypeAndMut, TypeckResults};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{MultiSpan, Span, DUMMY_SP};
 use rustc_target::spec::abi;
@@ -338,67 +337,45 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         let mut turbofish_suggestions = FxHashSet::default();
         self.tcx.for_each_relevant_impl(data.trait_ref.def_id, self_ty, |impl_def_id| {
             self.probe(|_| {
-                let impl_substs = self.fresh_substs_for_item(DUMMY_SP, impl_def_id);
+                let trait_args = InternalSubsts::identity_for_item(self.tcx, data.trait_ref.def_id);
+                let impl_args = traits::specialize::fulfill_implication(
+                    self,
+                    obligation.param_env,
+                    data.trait_ref,
+                    impl_def_id,
+                );
+                let substs = match impl_args {
+                    Ok(impl_args) => trait_args.rebase_onto(self.tcx, impl_def_id, impl_args),
+                    _ => return,
+                };
                 let trait_ref = self.tcx.impl_trait_ref(impl_def_id).unwrap();
-                let trait_ref = trait_ref.subst(self.tcx, impl_substs);
 
-                // Require the type the impl is implemented on to match
-                // our type, and ignore the impl if there was a mismatch.
                 let cause = traits::ObligationCause::dummy();
-                let eq_result =
-                    self.at(&cause, obligation.param_env).eq(trait_ref.self_ty(), self_ty);
-                if let Ok(InferOk { value: (), obligations: _ }) = eq_result {
-                    if let Ok(eval_result) = self.evaluate_obligation(&Obligation::new(
+                let mut selcx = SelectionContext::new(&self);
+                let param_env = obligation.param_env.clone();
+                let (target_trait_ref, obligations) = traits::util::impl_trait_ref_and_oblig(
+                    &mut selcx,
+                    param_env,
+                    impl_def_id,
+                    substs,
+                );
+                self.resolve_vars_if_possible(&target_trait_ref);
+                if let Ok(eval_result) = selcx.evaluate_predicates(
+                    Some(Obligation::new(
                         cause.clone(),
-                        obligation.param_env,
-                        trait_ref.without_const().to_predicate(self.tcx),
-                    )) {
-                        // FIXME: We will also suggest cases where `eval_result` is
-                        // `EvaluatedToAmbig`, we just put them at the end of the sugg list.
-                        // Ideally we would only suggest types that would always apply cleanly.
-                        // This means that for `collect` we will suggest `PathBuf` as a valid type
-                        // whereas that `impl` has an extra requirement `T: AsRef<Path>` which we
-                        // don't evaluate.
-                        if eval_result.may_apply()
-                            && data.trait_ref.substs.iter().zip(trait_ref.substs.iter()).all(
-                                // Here we'll have something like `[_, i32]` coming from our code
-                                // and `[Vec<_>, _]` from the probe. For cases with
-                                // inference variables we are left with ambiguous cases due to
-                                // trait bounds we couldn't evaluate, but we *can* filter out cases
-                                // like `[std::string::String, char]`, where we can `collect` a
-                                // `String` only if we have an `IntoIterator<Item = char>`, which
-                                // won't match the `i32` we have.
-                                |(l, r)| {
-                                    // FIXME: ideally we would use `can_coerce` here instead, but `typeck`
-                                    // comes *after* in the dependency graph.
-                                    match (l.unpack(), r.unpack()) {
-                                        (
-                                            GenericArgKind::Type(left_ty),
-                                            GenericArgKind::Type(right_ty),
-                                        ) => match (
-                                            &left_ty.peel_refs().kind(),
-                                            &right_ty.peel_refs().kind(),
-                                        ) {
-                                            (Infer(_), _) | (_, Infer(_)) => true,
-                                            (left_kind, right_kind) => left_kind == right_kind,
-                                        },
-                                        (
-                                            GenericArgKind::Lifetime(_),
-                                            GenericArgKind::Lifetime(_),
-                                        )
-                                        | (GenericArgKind::Const(_), GenericArgKind::Const(_)) => {
-                                            true
-                                        }
-                                        _ => false,
-                                    }
-                                },
-                            )
-                            && !matches!(trait_ref.self_ty().kind(), ty::Infer(_))
-                        {
-                            turbofish_suggestions.insert(trait_ref.self_ty());
-                        }
-                    };
-                }
+                        param_env,
+                        target_trait_ref.without_const().to_predicate(self.tcx),
+                    ))
+                    .into_iter()
+                    .chain(obligations),
+                ) {
+                    debug!("get_turbofish_suggestions result {:?}", eval_result);
+                    if eval_result.must_apply_modulo_regions()
+                        && !matches!(trait_ref.self_ty().kind(), ty::Infer(_))
+                    {
+                        turbofish_suggestions.insert(trait_ref.self_ty());
+                    }
+                };
             })
         });
         // Sort types by always suggesting `Vec<_>` and `String` first, as they are the
diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs
@@ -403,20 +403,27 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         })
     }
 
+    crate fn evaluate_predicates(
+        &mut self,
+        predicates: impl Iterator<Item = PredicateObligation<'tcx>>,
+    ) -> Result<EvaluationResult, OverflowError> {
+        self.evaluate_predicates_recursively(
+            TraitObligationStackList::empty(&ProvisionalEvaluationCache::default()),
+            predicates,
+        )
+    }
+
     /// Evaluates the predicates in `predicates` recursively. Note that
     /// this applies projections in the predicates, and therefore
     /// is run within an inference probe.
-    fn evaluate_predicates_recursively<'o, I>(
+    fn evaluate_predicates_recursively<'o>(
         &mut self,
         stack: TraitObligationStackList<'o, 'tcx>,
-        predicates: I,
-    ) -> Result<EvaluationResult, OverflowError>
-    where
-        I: IntoIterator<Item = PredicateObligation<'tcx>> + std::fmt::Debug,
-    {
+        predicates: impl Iterator<Item = PredicateObligation<'tcx>>,
+    ) -> Result<EvaluationResult, OverflowError> {
         let mut result = EvaluatedToOk;
-        debug!(?predicates, "evaluate_predicates_recursively");
         for obligation in predicates {
+            debug!(?obligation, "evaluate_predicates_recursively obligation");
             let eval = self.evaluate_predicate_recursively(stack, obligation.clone())?;
             if let EvaluatedToErr = eval {
                 // fast-path - EvaluatedToErr is the top of the lattice,
@@ -484,7 +491,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 ) {
                     Some(mut obligations) => {
                         self.add_depth(obligations.iter_mut(), obligation.recursion_depth);
-                        self.evaluate_predicates_recursively(previous_stack, obligations)
+                        self.evaluate_predicates_recursively(
+                            previous_stack,
+                            obligations.into_iter(),
+                        )
                     }
                     None => Ok(EvaluatedToAmbig),
                 },
@@ -508,8 +518,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     match project::poly_project_and_unify_type(self, &project_obligation) {
                         Ok(Ok(Some(mut subobligations))) => {
                             self.add_depth(subobligations.iter_mut(), obligation.recursion_depth);
-                            let result = self
-                                .evaluate_predicates_recursively(previous_stack, subobligations);
+                            let result = self.evaluate_predicates_recursively(
+                                previous_stack,
+                                subobligations.into_iter(),
+                            );
                             if let Some(key) =
                                 ProjectionCacheKey::from_poly_projection_predicate(self, data)
                             {
@@ -1245,7 +1257,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     ) -> Result<EvaluationResult, OverflowError> {
         self.evaluation_probe(|this| {
             match this.match_where_clause_trait_ref(stack.obligation, where_clause_trait_ref) {
-                Ok(obligations) => this.evaluate_predicates_recursively(stack.list(), obligations),
+                Ok(obligations) => {
+                    this.evaluate_predicates_recursively(stack.list(), obligations.into_iter())
+                }
                 Err(()) => Ok(EvaluatedToErr),
             }
         })
diff --git a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs
@@ -176,7 +176,7 @@ pub(super) fn specializes(tcx: TyCtxt<'_>, (impl1_def_id, impl2_def_id): (DefId,
 /// generics of `target_impl`, including both those needed to unify with
 /// `source_trait_ref` and those whose identity is determined via a where
 /// clause in the impl.
-fn fulfill_implication<'a, 'tcx>(
+crate fn fulfill_implication<'a, 'tcx>(
     infcx: &InferCtxt<'a, 'tcx>,
     param_env: ty::ParamEnv<'tcx>,
     source_trait_ref: ty::TraitRef<'tcx>,
diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs
@@ -131,7 +131,11 @@ pub(super) fn check_fn<'a, 'tcx>(
         // for simple cases like `fn foo(x: Trait)`,
         // where we would error once on the parameter as a whole, and once on the binding `x`.
         if param.pat.simple_ident().is_none() && !tcx.features().unsized_locals {
-            fcx.require_type_is_sized(param_ty, param.pat.span, traits::SizedArgumentType(ty_span));
+            fcx.require_type_is_sized_deferred(
+                param_ty,
+                param.pat.span,
+                traits::SizedArgumentType(ty_span),
+            );
         }
 
         fcx.write_ty(param.hir_id, param_ty);
diff --git a/src/test/ui/closures/issue-41366.stderr b/src/test/ui/closures/issue-41366.stderr
@@ -1,14 +1,3 @@
-error[E0631]: type mismatch in closure arguments
-  --> $DIR/issue-41366.rs:10:5
-   |
-LL |     (&|_| ()) as &dyn for<'x> Fn(<u32 as T<'x>>::V);
-   |     ^^------^
-   |     | |
-   |     | found signature of `fn(u16) -> _`
-   |     expected signature of `fn(<u32 as T<'x>>::V) -> _`
-   |
-   = note: required for the cast to the object type `dyn for<'x> Fn(<u32 as T<'x>>::V)`
-
 error[E0277]: the size for values of type `<u32 as T<'_>>::V` cannot be known at compilation time
   --> $DIR/issue-41366.rs:10:8
    |
@@ -26,6 +15,17 @@ help: function arguments must have a statically known size, borrowed types alway
 LL |     (&|&_| ()) as &dyn for<'x> Fn(<u32 as T<'x>>::V);
    |        ^
 
+error[E0631]: type mismatch in closure arguments
+  --> $DIR/issue-41366.rs:10:5
+   |
+LL |     (&|_| ()) as &dyn for<'x> Fn(<u32 as T<'x>>::V);
+   |     ^^------^
+   |     | |
+   |     | found signature of `fn(u16) -> _`
+   |     expected signature of `fn(<u32 as T<'x>>::V) -> _`
+   |
+   = note: required for the cast to the object type `dyn for<'x> Fn(<u32 as T<'x>>::V)`
+
 error: aborting due to 2 previous errors
 
 Some errors have detailed explanations: E0277, E0631.
diff --git a/src/test/ui/question-mark-type-infer.stderr b/src/test/ui/question-mark-type-infer.stderr
@@ -8,15 +8,8 @@ LL |     l.iter().map(f).collect()?
    = note: required by `into_result`
 help: consider specifying the type argument in the method call
    |
-LL |     l.iter().map(f).collect::<ControlFlow<_, _>>()?
-   |                            ^^^^^^^^^^^^^^^^^^^^^
-LL |     l.iter().map(f).collect::<Option<_>>()?
-   |                            ^^^^^^^^^^^^^
-LL |     l.iter().map(f).collect::<Poll<Option<std::result::Result<_, _>>>>()?
-   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-LL |     l.iter().map(f).collect::<Poll<std::result::Result<_, _>>>()?
-   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-     and 1 other candidate
+LL |     l.iter().map(f).collect::<B>()?
+   |                            ^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/suggestions/appropriate-type-param-turbofish.rs b/src/test/ui/suggestions/appropriate-type-param-turbofish.rs
@@ -36,6 +36,7 @@ mod c {
 trait T: Sized {
     fn new() -> Self;
 }
+
 fn x<X: T>() -> X {
     T::new()
 }
@@ -46,6 +47,14 @@ impl T for S {
     }
 }
 
+struct AAA<X> { x: X, }
+
+impl<Z: std::fmt::Display> std::iter::FromIterator<Z> for AAA<Z> {
+    fn from_iter<I: IntoIterator<Item = Z>>(_iter: I) -> AAA<Z> {
+        panic!()
+    }
+}
+
 fn foo() {
     x(); //~ ERROR type annotations needed
 }
@@ -54,4 +63,34 @@ fn bar() {
     let _ = x(); //~ ERROR type annotations needed
 }
 
-fn main() {}
+fn qux() {
+    let x: Vec<&std::path::Path> = vec![];
+    let y = x.into_iter().collect(); //~ ERROR type annotations needed
+}
+
+trait Foo {
+    fn foo<T: Bar<K>, K>(&self) -> T {
+        panic!()
+    }
+}
+
+trait Bar<X> {}
+
+struct R<X>(X);
+impl<X> Bar<X> for R<X> {}
+struct K<X>(X);
+impl Bar<i32> for K<i32> {}
+
+struct I;
+
+impl Foo for I {}
+
+// These two cases do not have good suggestions yet:
+fn bat() {
+    let _ = I.foo(); //~ ERROR type annotations needed
+}
+fn bak<T: Into<String>>() {}
+
+fn main() {
+    bak(); //~ ERROR type annotations needed
+}
diff --git a/src/test/ui/suggestions/appropriate-type-param-turbofish.stderr b/src/test/ui/suggestions/appropriate-type-param-turbofish.stderr
diff --git a/src/test/ui/typeck/typeck_type_placeholder_item.stderr b/src/test/ui/typeck/typeck_type_placeholder_item.stderr
