when creating an AssociatedItem, read data from impl, not impl item

Before, when we created an AssociatedItem for impl item X, we would read
the impl item itself. Now we instead load up the impl I that contains X
and read the data from the `ImplItemRef` for X; actually, we do it for
all impl items in I pre-emptively.

This kills the last source of edges between a method X and a call to a
method Y defined in the same impl.

Fixes #37121

Author: nikomatsakis

src/librustc/hir/def.rs

@@ -106,7 +106,7 @@ pub type DefMap = NodeMap<PathResolution>;
 // within.
 pub type ExportMap = NodeMap<Vec<Export>>;
 
-#[derive(Copy, Clone, RustcEncodable, RustcDecodable)]
+#[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable)]
 pub struct Export {
     pub name: ast::Name, // The name of the target.
     pub def: Def, // The definition of the target.

src/librustc/traits/project.rs

@@ -1007,8 +1007,9 @@ fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx>(
                     // types, which appear not to unify -- so the
                     // overlap check succeeds, when it should
                     // fail.
-                    bug!("Tried to project an inherited associated type during \
-                          coherence checking, which is currently not supported.");
+                    span_bug!(obligation.cause.span,
+                              "Tried to project an inherited associated type during \
+                               coherence checking, which is currently not supported.");
                 };
                 candidate_set.vec.extend(new_candidate);
             }

src/librustc/ty/mod.rs

@@ -2113,69 +2113,111 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
                            .expect("missing AssociatedItem in metadata");
             }
 
+            // When the user asks for a given associated item, we
+            // always go ahead and convert all the associated items in
+            // the container. Note that we are also careful only to
+            // ever register a read on the *container* of the assoc
+            // item, not the assoc item itself. This prevents changes
+            // in the details of an item (for example, the type to
+            // which an associated type is bound) from contaminating
+            // those tasks that just need to scan the names of items
+            // and so forth.
+
             let id = self.map.as_local_node_id(def_id).unwrap();
             let parent_id = self.map.get_parent(id);
             let parent_def_id = self.map.local_def_id(parent_id);
-            match self.map.get(id) {
-                ast_map::NodeTraitItem(trait_item) => {
-                    let (kind, has_self, has_value) = match trait_item.node {
-                        hir::MethodTraitItem(ref sig, ref body) => {
-                            (AssociatedKind::Method, sig.decl.get_self().is_some(),
-                             body.is_some())
-                        }
-                        hir::ConstTraitItem(_, ref value) => {
-                            (AssociatedKind::Const, false, value.is_some())
-                        }
-                        hir::TypeTraitItem(_, ref ty) => {
-                            (AssociatedKind::Type, false, ty.is_some())
-                        }
-                    };
-
-                    AssociatedItem {
-                        name: trait_item.name,
-                        kind: kind,
-                        vis: Visibility::from_hir(&hir::Inherited, id, self),
-                        defaultness: hir::Defaultness::Default,
-                        has_value: has_value,
-                        def_id: def_id,
-                        container: TraitContainer(parent_def_id),
-                        method_has_self_argument: has_self
+            let parent_item = self.map.expect_item(parent_id);
+            match parent_item.node {
+                hir::ItemImpl(.., ref impl_trait_ref, _, ref impl_item_refs) => {
+                    for impl_item_ref in impl_item_refs {
+                        let assoc_item =
+                            self.associated_item_from_impl_item_ref(parent_def_id,
+                                                                    impl_trait_ref.is_some(),
+                                                                    impl_item_ref);
+                        self.associated_items.borrow_mut().insert(assoc_item.def_id, assoc_item);
                     }
                 }
-                ast_map::NodeImplItem(impl_item) => {
-                    let (kind, has_self) = match impl_item.node {
-                        hir::ImplItemKind::Method(ref sig, _) => {
-                            (AssociatedKind::Method, sig.decl.get_self().is_some())
-                        }
-                        hir::ImplItemKind::Const(..) => (AssociatedKind::Const, false),
-                        hir::ImplItemKind::Type(..) => (AssociatedKind::Type, false)
-                    };
-
-                    // Trait impl items are always public.
-                    let public = hir::Public;
-                    let parent_item = self.map.expect_item(parent_id);
-                    let vis = if let hir::ItemImpl(.., Some(_), _, _) = parent_item.node {
-                        &public
-                    } else {
-                        &impl_item.vis
-                    };
-
-                    AssociatedItem {
-                        name: impl_item.name,
-                        kind: kind,
-                        vis: Visibility::from_hir(vis, id, self),
-                        defaultness: impl_item.defaultness,
-                        has_value: true,
-                        def_id: def_id,
-                        container: ImplContainer(parent_def_id),
-                        method_has_self_argument: has_self
+
+                hir::ItemTrait(.., ref trait_items) => {
+                    for trait_item in trait_items {
+                        let assoc_item =
+                            self.associated_item_from_trait_item_ref(parent_def_id, trait_item);
+                        self.associated_items.borrow_mut().insert(assoc_item.def_id, assoc_item);
                     }
                 }
-                item => bug!("associated_item: {:?} not an associated item", item)
+
+                ref r => {
+                    panic!("unexpected container of associated items: {:?}", r)
+                }
             }
+
+            // memoize wants us to return something, so return
+            // the one we generated for this def-id
+            *self.associated_items.borrow().get(&def_id).unwrap()
         })
     }
 
+    fn associated_item_from_trait_item_ref(self,
+                                           parent_def_id: DefId,
+                                           trait_item: &hir::TraitItem)
+                                           -> AssociatedItem {
+        let def_id = self.map.local_def_id(trait_item.id);
+
+        let (kind, has_self, has_value) = match trait_item.node {
+            hir::MethodTraitItem(ref sig, ref body) => {
+                (AssociatedKind::Method, sig.decl.get_self().is_some(),
+                 body.is_some())
+            }
+            hir::ConstTraitItem(_, ref value) => {
+                (AssociatedKind::Const, false, value.is_some())
+            }
+            hir::TypeTraitItem(_, ref ty) => {
+                (AssociatedKind::Type, false, ty.is_some())
+            }
+        };
+
+        AssociatedItem {
+            name: trait_item.name,
+            kind: kind,
+            vis: Visibility::from_hir(&hir::Inherited, trait_item.id, self),
+            defaultness: hir::Defaultness::Default,
+            has_value: has_value,
+            def_id: def_id,
+            container: TraitContainer(parent_def_id),
+            method_has_self_argument: has_self
+        }
+    }
+
+    fn associated_item_from_impl_item_ref(self,
+                                          parent_def_id: DefId,
+                                          from_trait_impl: bool,
+                                          impl_item_ref: &hir::ImplItemRef)
+                                          -> AssociatedItem {
+        let def_id = self.map.local_def_id(impl_item_ref.id.node_id);
+        let (kind, has_self) = match impl_item_ref.kind {
+            hir::AssociatedItemKind::Const => (ty::AssociatedKind::Const, false),
+            hir::AssociatedItemKind::Method { has_self } => {
+                (ty::AssociatedKind::Method, has_self)
+            }
+            hir::AssociatedItemKind::Type => (ty::AssociatedKind::Type, false),
+        };
+
+        // Trait impl items are always public.
+        let public = hir::Public;
+        let vis = if from_trait_impl { &public } else { &impl_item_ref.vis };
+
+        ty::AssociatedItem {
+            name: impl_item_ref.name,
+            kind: kind,
+            vis: ty::Visibility::from_hir(vis, impl_item_ref.id.node_id, self),
+            defaultness: impl_item_ref.defaultness,
+            has_value: true,
+            def_id: def_id,
+            container: ImplContainer(parent_def_id),
+            method_has_self_argument: has_self
+        }
+    }
+
     pub fn associated_item_def_ids(self, def_id: DefId) -> Rc<Vec<DefId>> {
         self.associated_item_def_ids.memoize(def_id, || {
             if !def_id.is_local() {
@@ -2184,19 +2226,22 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
 
             let id = self.map.as_local_node_id(def_id).unwrap();
             let item = self.map.expect_item(id);
-            match item.node {
+            let vec: Vec<_> = match item.node {
                 hir::ItemTrait(.., ref trait_items) => {
-                    Rc::new(trait_items.iter().map(|trait_item| {
-                        self.map.local_def_id(trait_item.id)
-                    }).collect())
+                    trait_items.iter()
+                               .map(|trait_item| trait_item.id)
+                               .map(|id| self.map.local_def_id(id))
+                               .collect()
                 }
                 hir::ItemImpl(.., ref impl_item_refs) => {
-                    Rc::new(impl_item_refs.iter().map(|impl_item_ref| {
-                        self.map.local_def_id(impl_item_ref.id.node_id)
-                    }).collect())
+                    impl_item_refs.iter()
+                                  .map(|impl_item_ref| impl_item_ref.id)
+                                  .map(|id| self.map.local_def_id(id.node_id))
+                                  .collect()
                 }
                 _ => span_bug!(item.span, "associated_item_def_ids: not impl or trait")
-            }
+            };
+            Rc::new(vec)
         })
     }
 

src/librustc_typeck/collect.rs

@@ -731,11 +731,7 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) {
                       ref generics,
                       ref opt_trait_ref,
                       ref selfty,
-                      ref _impl_item_ids /* [1] */) => {
-            // [1]: We really don't want to be inspecting the details
-            // of impl-items here; it creates bad edges in the
-            // incr. comp. graph.
-
+                      _) => {
             // Create generics from the generics specified in the impl head.
             debug!("convert: ast_generics={:?}", generics);
             let def_id = ccx.tcx.map.local_def_id(it.id);

src/librustdoc/visit_ast.rs

@@ -507,15 +507,15 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                 // regardless of where they're located.
                 if !self.inlining {
                     let items = item_ids.iter()
-                                        .map(|&id| self.cx.map.impl_item(id).clone())
+                                        .map(|ii| self.cx.map.impl_item(ii.id).clone())
                                         .collect();
                     let i = Impl {
                         unsafety: unsafety,
                         polarity: polarity,
                         generics: gen.clone(),
                         trait_: tr.clone(),
                         for_: ty.clone(),
-                        items: items.clone(),
+                        items: items,
                         attrs: item.attrs.clone(),
                         id: item.id,
                         whence: item.span,

src/test/incremental/change_private_impl_method/struct_point.rs

@@ -20,9 +20,8 @@
 
 #![rustc_partition_translated(module="struct_point-point", cfg="rpass2")]
 
-// FIXME(#37121) -- the following two modules *should* be reused but are not
-#![rustc_partition_translated(module="struct_point-fn_calls_methods_in_same_impl", cfg="rpass2")]
-#![rustc_partition_translated(module="struct_point-fn_calls_methods_in_another_impl", cfg="rpass2")]
+#![rustc_partition_reused(module="struct_point-fn_calls_methods_in_same_impl", cfg="rpass2")]
+#![rustc_partition_reused(module="struct_point-fn_calls_methods_in_another_impl", cfg="rpass2")]
 #![rustc_partition_reused(module="struct_point-fn_make_struct", cfg="rpass2")]
 #![rustc_partition_reused(module="struct_point-fn_read_field", cfg="rpass2")]
 #![rustc_partition_reused(module="struct_point-fn_write_field", cfg="rpass2")]
@@ -60,8 +59,7 @@ mod point {
 mod fn_calls_methods_in_same_impl {
     use point::Point;
 
-    // FIXME(#37121) -- we should not need to typeck this again
-    #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
+    #[rustc_clean(label="TypeckItemBody", cfg="rpass2")]
     pub fn check() {
         let x = Point { x: 2.0, y: 2.0 };
         x.distance_from_origin();
@@ -72,8 +70,7 @@ mod fn_calls_methods_in_same_impl {
 mod fn_calls_methods_in_another_impl {
     use point::Point;
 
-    // FIXME(#37121) -- we should not need to typeck this again
-    #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
+    #[rustc_clean(label="TypeckItemBody", cfg="rpass2")]
     pub fn check() {
         let mut x = Point { x: 2.0, y: 2.0 };
         x.translate(3.0, 3.0);

src/test/incremental/change_private_impl_method_cc/struct_point.rs

@@ -19,20 +19,23 @@
 #![feature(stmt_expr_attributes)]
 #![allow(dead_code)]
 
-// FIXME(#37333) -- the following modules *should* be reused but are not
+#![rustc_partition_reused(module="struct_point-fn_read_field", cfg="rpass2")]
+#![rustc_partition_reused(module="struct_point-fn_write_field", cfg="rpass2")]
+
+// FIXME(#37333) the struct fields get entangled with inherent methods
+#![rustc_partition_translated(module="struct_point-fn_make_struct", cfg="rpass2")]
+
+// FIXME(#37720) these two should be reused, but data gets entangled across crates
 #![rustc_partition_translated(module="struct_point-fn_calls_methods_in_same_impl", cfg="rpass2")]
 #![rustc_partition_translated(module="struct_point-fn_calls_methods_in_another_impl", cfg="rpass2")]
-#![rustc_partition_translated(module="struct_point-fn_make_struct", cfg="rpass2")]
-#![rustc_partition_translated(module="struct_point-fn_read_field", cfg="rpass2")]
-#![rustc_partition_translated(module="struct_point-fn_write_field", cfg="rpass2")]
 
 extern crate point;
 
 /// A fn item that calls (public) methods on `Point` from the same impl which changed
 mod fn_calls_methods_in_same_impl {
     use point::Point;
 
-    // FIXME(#37333) -- we should not need to typeck this again
+    // FIXME(#37720) data gets entangled across crates
     #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
     pub fn check() {
         let x = Point { x: 2.0, y: 2.0 };
@@ -44,9 +47,9 @@ mod fn_calls_methods_in_same_impl {
 mod fn_calls_methods_in_another_impl {
     use point::Point;
 
-    // FIXME(#37333) -- we should not need to typeck this again
+    // FIXME(#37720) data gets entangled across crates
     #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
-    pub fn check() {
+    pub fn dirty() {
         let mut x = Point { x: 2.0, y: 2.0 };
         x.translate(3.0, 3.0);
     }
@@ -56,8 +59,7 @@ mod fn_calls_methods_in_another_impl {
 mod fn_make_struct {
     use point::Point;
 
-    // FIXME(#37333) -- we should not need to typeck this again
-    #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
+    #[rustc_clean(label="TypeckItemBody", cfg="rpass2")]
     pub fn make_origin() -> Point {
         Point { x: 2.0, y: 2.0 }
     }
@@ -67,8 +69,7 @@ mod fn_make_struct {
 mod fn_read_field {
     use point::Point;
 
-    // FIXME(#37333) -- we should not need to typeck this again
-    #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
+    #[rustc_clean(label="TypeckItemBody", cfg="rpass2")]
     pub fn get_x(p: Point) -> f32 {
         p.x
     }
@@ -78,8 +79,7 @@ mod fn_read_field {
 mod fn_write_field {
     use point::Point;
 
-    // FIXME(#37333) -- we should not need to typeck this again
-    #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
+    #[rustc_clean(label="TypeckItemBody", cfg="rpass2")]
     pub fn inc_x(p: &mut Point) {
         p.x += 1.0;
     }