Auto merge of rust-lang#61919 - alexreg:fix-atb-1, r=nikomatsakis

Fix for "ambiguous associated type" issue with ATBs

Fixes rust-lang#61752.

r? @nikomatsakis

CC @Centril

src/librustc/hir/mod.rs

@@ -2234,7 +2234,7 @@ pub enum UseKind {
 #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)]
 pub struct TraitRef {
     pub path: P<Path>,
-    // Don't hash the ref_id. It is tracked via the thing it is used to access
+    // Don't hash the `ref_id`. It is tracked via the thing it is used to access.
     #[stable_hasher(ignore)]
     pub hir_ref_id: HirId,
 }

src/librustc/ty/mod.rs

@@ -904,7 +904,7 @@ pub struct Generics {
     pub parent_count: usize,
     pub params: Vec<GenericParamDef>,
 
-    /// Reverse map to the `index` field of each `GenericParamDef`
+    /// Reverse map to the `index` field of each `GenericParamDef`.
     #[stable_hasher(ignore)]
     pub param_def_id_to_index: FxHashMap<DefId, u32>,
 
@@ -1252,7 +1252,7 @@ impl<'tcx> TraitPredicate<'tcx> {
 
 impl<'tcx> PolyTraitPredicate<'tcx> {
     pub fn def_id(&self) -> DefId {
-        // Ok to skip binder since trait def-ID does not care about regions.
+        // Ok to skip binder since trait `DefId` does not care about regions.
         self.skip_binder().def_id()
     }
 }
@@ -1319,7 +1319,7 @@ impl<'tcx> PolyProjectionPredicate<'tcx> {
     /// Note that this is not the `DefId` of the `TraitRef` containing this
     /// associated type, which is in `tcx.associated_item(projection_def_id()).container`.
     pub fn projection_def_id(&self) -> DefId {
-        // Ok to skip binder since trait def-ID does not care about regions.
+        // Ok to skip binder since trait `DefId` does not care about regions.
         self.skip_binder().projection_ty.item_def_id
     }
 }
@@ -1646,9 +1646,9 @@ pub type PlaceholderConst = Placeholder<BoundVar>;
 /// particular point.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
 pub struct ParamEnv<'tcx> {
-    /// Obligations that the caller must satisfy. This is basically
+    /// `Obligation`s that the caller must satisfy. This is basically
     /// the set of bounds on the in-scope type parameters, translated
-    /// into Obligations, and elaborated and normalized.
+    /// into `Obligation`s, and elaborated and normalized.
     pub caller_bounds: &'tcx List<ty::Predicate<'tcx>>,
 
     /// Typically, this is `Reveal::UserFacing`, but during codegen we
@@ -2796,7 +2796,7 @@ impl<'tcx> TyCtxt<'tcx> {
                 _ => false,
             }
         } else {
-            match self.def_kind(def_id).expect("no def for def-id") {
+            match self.def_kind(def_id).expect("no def for `DefId`") {
                 DefKind::AssocConst
                 | DefKind::Method
                 | DefKind::AssocTy => true,

src/librustc/ty/sty.rs

@@ -646,7 +646,7 @@ impl<'tcx> List<ExistentialPredicate<'tcx>> {
     ///
     /// A Rust trait object type consists (in addition to a lifetime bound)
     /// of a set of trait bounds, which are separated into any number
-    /// of auto-trait bounds, and at most 1 non-auto-trait bound. The
+    /// of auto-trait bounds, and at most one non-auto-trait bound. The
     /// non-auto-trait bound is called the "principal" of the trait
     /// object.
     ///
@@ -680,7 +680,8 @@ impl<'tcx> List<ExistentialPredicate<'tcx>> {
 
     #[inline]
     pub fn projection_bounds<'a>(&'a self) ->
-        impl Iterator<Item=ExistentialProjection<'tcx>> + 'a {
+        impl Iterator<Item = ExistentialProjection<'tcx>> + 'a
+    {
         self.iter().filter_map(|predicate| {
             match *predicate {
                 ExistentialPredicate::Projection(p) => Some(p),
@@ -690,7 +691,7 @@ impl<'tcx> List<ExistentialPredicate<'tcx>> {
     }
 
     #[inline]
-    pub fn auto_traits<'a>(&'a self) -> impl Iterator<Item=DefId> + 'a {
+    pub fn auto_traits<'a>(&'a self) -> impl Iterator<Item = DefId> + 'a {
         self.iter().filter_map(|predicate| {
             match *predicate {
                 ExistentialPredicate::AutoTrait(d) => Some(d),
@@ -711,17 +712,17 @@ impl<'tcx> Binder<&'tcx List<ExistentialPredicate<'tcx>>> {
 
     #[inline]
     pub fn projection_bounds<'a>(&'a self) ->
-        impl Iterator<Item=PolyExistentialProjection<'tcx>> + 'a {
+        impl Iterator<Item = PolyExistentialProjection<'tcx>> + 'a {
         self.skip_binder().projection_bounds().map(Binder::bind)
     }
 
     #[inline]
-    pub fn auto_traits<'a>(&'a self) -> impl Iterator<Item=DefId> + 'a {
+    pub fn auto_traits<'a>(&'a self) -> impl Iterator<Item = DefId> + 'a {
         self.skip_binder().auto_traits()
     }
 
     pub fn iter<'a>(&'a self)
-        -> impl DoubleEndedIterator<Item=Binder<ExistentialPredicate<'tcx>>> + 'tcx {
+        -> impl DoubleEndedIterator<Item = Binder<ExistentialPredicate<'tcx>>> + 'tcx {
         self.skip_binder().iter().cloned().map(Binder::bind)
     }
 }

src/librustc_codegen_llvm/debuginfo/metadata.rs

@@ -450,11 +450,11 @@ fn subroutine_type_metadata(
         false);
 }
 
-// FIXME(1563) This is all a bit of a hack because 'trait pointer' is an ill-
-// defined concept. For the case of an actual trait pointer (i.e., Box<Trait>,
-// &Trait), trait_object_type should be the whole thing (e.g, Box<Trait>) and
-// trait_type should be the actual trait (e.g., Trait). Where the trait is part
-// of a DST struct, there is no trait_object_type and the results of this
+// FIXME(1563): This is all a bit of a hack because 'trait pointer' is an ill-
+// defined concept. For the case of an actual trait pointer (i.e., `Box<Trait>`,
+// `&Trait`), `trait_object_type` should be the whole thing (e.g, `Box<Trait>`) and
+// `trait_type` should be the actual trait (e.g., `Trait`). Where the trait is part
+// of a DST struct, there is no `trait_object_type` and the results of this
 // function will be a little bit weird.
 fn trait_pointer_metadata(
     cx: &CodegenCx<'ll, 'tcx>,
@@ -464,13 +464,13 @@ fn trait_pointer_metadata(
 ) -> &'ll DIType {
     // The implementation provided here is a stub. It makes sure that the trait
     // type is assigned the correct name, size, namespace, and source location.
-    // But it does not describe the trait's methods.
+    // However, it does not describe the trait's methods.
 
     let containing_scope = match trait_type.sty {
         ty::Dynamic(ref data, ..) =>
             data.principal_def_id().map(|did| get_namespace_for_item(cx, did)),
         _ => {
-            bug!("debuginfo: Unexpected trait-object type in \
+            bug!("debuginfo: unexpected trait-object type in \
                   trait_pointer_metadata(): {:?}",
                  trait_type);
         }

src/librustc_privacy/lib.rs

@@ -1050,14 +1050,19 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
         if !self.in_body {
             // Avoid calling `hir_trait_to_predicates` in bodies, it will ICE.
             // The traits' privacy in bodies is already checked as a part of trait object types.
-            let (principal, bounds) = rustc_typeck::hir_trait_to_predicates(self.tcx, trait_ref);
-            if self.visit_trait(*principal.skip_binder()) {
-                return;
+            let bounds = rustc_typeck::hir_trait_to_predicates(self.tcx, trait_ref);
+
+            for (trait_predicate, _) in bounds.trait_bounds {
+                if self.visit_trait(*trait_predicate.skip_binder()) {
+                    return;
+                }
             }
+
             for (poly_predicate, _) in bounds.projection_bounds {
                 let tcx = self.tcx;
-                if self.visit(poly_predicate.skip_binder().ty) ||
-                   self.visit_trait(poly_predicate.skip_binder().projection_ty.trait_ref(tcx)) {
+                if self.visit(poly_predicate.skip_binder().ty)
+                    || self.visit_trait(poly_predicate.skip_binder().projection_ty.trait_ref(tcx))
+                {
                     return;
                 }
             }

src/librustc_typeck/astconv.rs

@@ -42,8 +42,17 @@ pub struct PathSeg(pub DefId, pub usize);
 pub trait AstConv<'tcx> {
     fn tcx<'a>(&'a self) -> TyCtxt<'tcx>;
 
-    /// Returns the set of bounds in scope for the type parameter with
-    /// the given id.
+    /// Returns predicates in scope of the form `X: Foo`, where `X` is
+    /// a type parameter `X` with the given id `def_id`. This is a
+    /// subset of the full set of predicates.
+    ///
+    /// This is used for one specific purpose: resolving "short-hand"
+    /// associated type references like `T::Item`. In principle, we
+    /// would do that by first getting the full set of predicates in
+    /// scope and then filtering down to find those that apply to `T`,
+    /// but this can lead to cycle errors. The problem is that we have
+    /// to do this resolution *in order to create the predicates in
+    /// the first place*. Hence, we have this "special pass".
     fn get_type_parameter_bounds(&self, span: Span, def_id: DefId)
                                  -> &'tcx ty::GenericPredicates<'tcx>;
 
@@ -775,11 +784,11 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
     /// The given trait-ref must actually be a trait.
     pub(super) fn instantiate_poly_trait_ref_inner(&self,
         trait_ref: &hir::TraitRef,
+        span: Span,
         self_ty: Ty<'tcx>,
         bounds: &mut Bounds<'tcx>,
         speculative: bool,
-    ) -> (ty::PolyTraitRef<'tcx>, Option<Vec<Span>>)
-    {
+    ) -> Option<Vec<Span>> {
         let trait_def_id = trait_ref.trait_def_id();
 
         debug!("instantiate_poly_trait_ref({:?}, def_id={:?})", trait_ref, trait_def_id);
@@ -794,6 +803,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         );
         let poly_trait_ref = ty::Binder::bind(ty::TraitRef::new(trait_def_id, substs));
 
+        bounds.trait_bounds.push((poly_trait_ref, span));
+
         let mut dup_bindings = FxHashMap::default();
         for binding in &assoc_bindings {
             // Specify type to assert that error was already reported in `Err` case.
@@ -804,14 +815,14 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                     binding,
                     bounds,
                     speculative,
-                    &mut dup_bindings
+                    &mut dup_bindings,
                 );
             // Okay to ignore `Err` because of `ErrorReported` (see above).
         }
 
         debug!("instantiate_poly_trait_ref({:?}, bounds={:?}) -> {:?}",
                trait_ref, bounds, poly_trait_ref);
-        (poly_trait_ref, potential_assoc_types)
+        potential_assoc_types
     }
 
     /// Given a trait bound like `Debug`, applies that trait bound the given self-type to construct
@@ -836,10 +847,15 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
     pub fn instantiate_poly_trait_ref(&self,
         poly_trait_ref: &hir::PolyTraitRef,
         self_ty: Ty<'tcx>,
-        bounds: &mut Bounds<'tcx>
-    ) -> (ty::PolyTraitRef<'tcx>, Option<Vec<Span>>)
-    {
-        self.instantiate_poly_trait_ref_inner(&poly_trait_ref.trait_ref, self_ty, bounds, false)
+        bounds: &mut Bounds<'tcx>,
+    ) -> Option<Vec<Span>> {
+        self.instantiate_poly_trait_ref_inner(
+            &poly_trait_ref.trait_ref,
+            poly_trait_ref.span,
+            self_ty,
+            bounds,
+            false,
+        )
     }
 
     fn ast_path_to_mono_trait_ref(&self,
@@ -983,12 +999,11 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         }
 
         for bound in trait_bounds {
-            let (poly_trait_ref, _) = self.instantiate_poly_trait_ref(
+            let _ = self.instantiate_poly_trait_ref(
                 bound,
                 param_ty,
                 bounds,
             );
-            bounds.trait_bounds.push((poly_trait_ref, bound.span))
         }
 
         bounds.region_bounds.extend(region_bounds
@@ -1172,11 +1187,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                 // Calling `skip_binder` is okay, because `add_bounds` expects the `param_ty`
                 // parameter to have a skipped binder.
                 let param_ty = tcx.mk_projection(assoc_ty.def_id, candidate.skip_binder().substs);
-                self.add_bounds(
-                    param_ty,
-                    ast_bounds,
-                    bounds,
-                );
+                self.add_bounds(param_ty, ast_bounds, bounds);
             }
         }
         Ok(())
@@ -1216,25 +1227,19 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         let mut bounds = Bounds::default();
         let mut potential_assoc_types = Vec::new();
         let dummy_self = self.tcx().types.trait_object_dummy_self;
-        // FIXME: we want to avoid collecting into a `Vec` here, but simply cloning the iterator is
-        // not straightforward due to the borrow checker.
-        let bound_trait_refs: Vec<_> = trait_bounds
-            .iter()
-            .rev()
-            .map(|trait_bound| {
-                let (trait_ref, cur_potential_assoc_types) = self.instantiate_poly_trait_ref(
-                    trait_bound,
-                    dummy_self,
-                    &mut bounds,
-                );
-                potential_assoc_types.extend(cur_potential_assoc_types.into_iter().flatten());
-                (trait_ref, trait_bound.span)
-            })
-            .collect();
+        for trait_bound in trait_bounds.iter().rev() {
+            let cur_potential_assoc_types = self.instantiate_poly_trait_ref(
+                trait_bound,
+                dummy_self,
+                &mut bounds,
+            );
+            potential_assoc_types.extend(cur_potential_assoc_types.into_iter().flatten());
+        }
 
         // Expand trait aliases recursively and check that only one regular (non-auto) trait
         // is used and no 'maybe' bounds are used.
-        let expanded_traits = traits::expand_trait_aliases(tcx, bound_trait_refs.iter().cloned());
+        let expanded_traits =
+            traits::expand_trait_aliases(tcx, bounds.trait_bounds.iter().cloned());
         let (mut auto_traits, regular_traits): (Vec<_>, Vec<_>) =
             expanded_traits.partition(|i| tcx.trait_is_auto(i.trait_ref().def_id()));
         if regular_traits.len() > 1 {
@@ -1276,7 +1281,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         // Use a `BTreeSet` to keep output in a more consistent order.
         let mut associated_types = BTreeSet::default();
 
-        let regular_traits_refs = bound_trait_refs
+        let regular_traits_refs = bounds.trait_bounds
             .into_iter()
             .filter(|(trait_ref, _)| !tcx.trait_is_auto(trait_ref.def_id()))
             .map(|(trait_ref, _)| trait_ref);
@@ -1491,7 +1496,17 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
     {
         let tcx = self.tcx();
 
+        debug!(
+            "find_bound_for_assoc_item(ty_param_def_id={:?}, assoc_name={:?}, span={:?})",
+            ty_param_def_id,
+            assoc_name,
+            span,
+        );
+
         let predicates = &self.get_type_parameter_bounds(span, ty_param_def_id).predicates;
+
+        debug!("find_bound_for_assoc_item: predicates={:#?}", predicates);
+
         let bounds = predicates.iter().filter_map(|(p, _)| p.to_opt_poly_trait_ref());
 
         // Check that there is exactly one way to find an associated type with the
@@ -1515,7 +1530,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                                    assoc_name: ast::Ident,
                                    span: Span)
         -> Result<ty::PolyTraitRef<'tcx>, ErrorReported>
-        where I: Iterator<Item=ty::PolyTraitRef<'tcx>>
+        where I: Iterator<Item = ty::PolyTraitRef<'tcx>>
     {
         let bound = match bounds.next() {
             Some(bound) => bound,
@@ -1524,13 +1539,17 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                                  "associated type `{}` not found for `{}`",
                                  assoc_name,
                                  ty_param_name)
-                  .span_label(span, format!("associated type `{}` not found", assoc_name))
-                  .emit();
+                    .span_label(span, format!("associated type `{}` not found", assoc_name))
+                    .emit();
                 return Err(ErrorReported);
             }
         };
 
+        debug!("one_bound_for_assoc_type: bound = {:?}", bound);
+
         if let Some(bound2) = bounds.next() {
+            debug!("one_bound_for_assoc_type: bound2 = {:?}", bound2);
+
             let bounds = iter::once(bound).chain(iter::once(bound2)).chain(bounds);
             let mut err = struct_span_err!(
                 self.tcx().sess, span, E0221,
@@ -1544,7 +1563,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                     item.kind == ty::AssocKind::Type &&
                         self.tcx().hygienic_eq(assoc_name, item.ident, bound.def_id())
                 })
-                .and_then(|item| self.tcx().hir().span_if_local(item.def_id));
+                    .and_then(|item| self.tcx().hir().span_if_local(item.def_id));
 
                 if let Some(span) = bound_span {
                     err.span_label(span, format!("ambiguous `{}` from `{}`",

src/librustc_typeck/check/mod.rs

@@ -2030,7 +2030,7 @@ fn report_unexpected_variant_res(tcx: TyCtxt<'_>, res: Res, span: Span, qpath: &
 impl<'a, 'tcx> AstConv<'tcx> for FnCtxt<'a, 'tcx> {
     fn tcx<'b>(&'b self) -> TyCtxt<'tcx> {
         self.tcx
-        }
+    }
 
     fn get_type_parameter_bounds(&self, _: Span, def_id: DefId)
                                  -> &'tcx ty::GenericPredicates<'tcx>