Auto merge of #37660 - nikomatsakis:incremental-36349, r=eddyb

Separate impl items from the parent impl

This change separates impl item bodies out of the impl itself. This gives incremental more resolution. In so doing, it refactors how the visitors work, and cleans up a bit of the collect/check logic (mostly by moving things out of collect that didn't really belong there, because they were just checking conditions).

However, this is not as effective as I expected, for a kind of frustrating reason. In particular, when invoking `foo.bar()` you still wind up with dependencies on private items. The problem is that the method resolution code scans that list for methods with the name `bar` -- and this winds up touching *all* the methods, even private ones.

I can imagine two obvious ways to fix this:

- separating fn bodies from fn sigs (#35078, currently being pursued by @flodiebold)
- a more aggressive model of incremental that @michaelwoerister has been advocating, in which we hash the intermediate results (e.g., the outputs of collect) so that we can see that the intermediate result hasn't changed, even if a particular impl item has changed.

So all in all I'm not quite sure whether to land this or not. =) It still seems like it has to be a win in some cases, but not with the test cases we have just now. I can try to gin up some test cases, but I'm not sure if they will be totally realistic. On the other hand, some of the early refactorings to the visitor trait seem worthwhile to me regardless.

cc #36349 -- well, this is basically a fix for that issue, I guess

r? @michaelwoerister

NB: Based atop of @eddyb's PR rust-lang/rust#37402; don't land until that lands.

clean/inline.rs

@@ -364,7 +364,7 @@ pub fn build_impl<'a, 'tcx>(cx: &DocContext,
     let trait_items = tcx.associated_items(did).filter_map(|item| {
         match item.kind {
             ty::AssociatedKind::Const => {
-                let default = if item.has_value {
+                let default = if item.defaultness.has_value() {
                     Some(pprust::expr_to_string(
                         lookup_const_by_id(tcx, item.def_id, None).unwrap().0))
                 } else {
@@ -407,7 +407,7 @@ pub fn build_impl<'a, 'tcx>(cx: &DocContext,
                             abi: abi
                         })
                     }
-                    _ => panic!("not a tymethod"),
+                    ref r => panic!("not a tymethod: {:?}", r),
                 };
                 Some(cleaned)
             }

clean/mod.rs

@@ -1373,9 +1373,10 @@ impl<'tcx> Clean<Item> for ty::AssociatedItem {
                         }
                     }
                 }
+
                 let provided = match self.container {
                     ty::ImplContainer(_) => false,
-                    ty::TraitContainer(_) => self.has_value
+                    ty::TraitContainer(_) => self.defaultness.has_value()
                 };
                 if provided {
                     MethodItem(Method {
@@ -1440,7 +1441,7 @@ impl<'tcx> Clean<Item> for ty::AssociatedItem {
                     None => bounds.push(TyParamBound::maybe_sized(cx)),
                 }
 
-                let ty = if self.has_value {
+                let ty = if self.defaultness.has_value() {
                     Some(cx.tcx().item_type(self.def_id))
                 } else {
                     None

visit_ast.rs

@@ -502,17 +502,20 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                 om.traits.push(t);
             },
 
-            hir::ItemImpl(unsafety, polarity, ref gen, ref tr, ref ty, ref items) => {
+            hir::ItemImpl(unsafety, polarity, ref gen, ref tr, ref ty, ref item_ids) => {
                 // Don't duplicate impls when inlining, we'll pick them up
                 // regardless of where they're located.
                 if !self.inlining {
+                    let items = item_ids.iter()
+                                        .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,