write to inherent_impls during the visitor

The goal here is to avoid writing to the `inherent_impls` map from
within the general `Coherence` task, and instead write to it as we
visit. Writing to it from the Coherence task is actually an information
leak; it happened to be safe because Coherence read from
`DepNode::Krate`, but that was very coarse.

I removed the `Rc` here because, upon manual inspection, nobody clones
the data in this table, and it meant that we can accumulate the data in
place. That said, the pattern that is used for the inherent impls map
is *generally* an anti-pattern (that is, holding the borrow lock for the
duration of using the contents), so it'd probably be better to
clone (and I doubt that would be expensive -- how many inherent impls
does a typical type have?).

Author: nikomatsakis

src/librustc/dep_graph/dep_tracking_map.rs

@@ -80,6 +80,17 @@ impl<M: DepTrackingMapConfig> DepTrackingMap<M> {
     pub fn keys(&self) -> Vec<M::Key> {
         self.map.keys().cloned().collect()
     }
+
+    /// Append `elem` to the vector stored for `k`, creating a new vector if needed.
+    /// This is considered a write to `k`.
+    pub fn push<E: Clone>(&mut self, k: M::Key, elem: E)
+        where M: DepTrackingMapConfig<Value=Vec<E>>
+    {
+        self.write(&k);
+        self.map.entry(k)
+                .or_insert(Vec::new())
+                .push(elem);
+    }
 }
 
 impl<M: DepTrackingMapConfig> MemoizationMap for RefCell<DepTrackingMap<M>> {

src/librustc/ty/maps.rs

@@ -39,7 +39,7 @@ dep_map_ty! { ImplTraitRefs: ItemSignature(DefId) -> Option<ty::TraitRef<'tcx>>
 dep_map_ty! { TraitDefs: ItemSignature(DefId) -> &'tcx ty::TraitDef<'tcx> }
 dep_map_ty! { AdtDefs: ItemSignature(DefId) -> ty::AdtDefMaster<'tcx> }
 dep_map_ty! { ItemVariances: ItemSignature(DefId) -> Rc<Vec<ty::Variance>> }
-dep_map_ty! { InherentImpls: InherentImpls(DefId) -> Rc<Vec<DefId>> }
+dep_map_ty! { InherentImpls: InherentImpls(DefId) -> Vec<DefId> }
 dep_map_ty! { ImplItems: ImplItems(DefId) -> Vec<ty::ImplOrTraitItemId> }
 dep_map_ty! { TraitItems: TraitItems(DefId) -> Rc<Vec<ty::ImplOrTraitItem<'tcx>>> }
 dep_map_ty! { ReprHints: ReprHints(DefId) -> Rc<Vec<attr::ReprAttr>> }

src/librustc/ty/mod.rs

@@ -2665,7 +2665,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             self.impl_items.borrow_mut().insert(impl_def_id, impl_items);
         }
 
-        self.inherent_impls.borrow_mut().insert(type_id, Rc::new(inherent_impls));
+        self.inherent_impls.borrow_mut().insert(type_id, inherent_impls);
         self.populated_external_types.borrow_mut().insert(type_id);
     }
 

src/librustc_typeck/coherence/mod.rs

@@ -32,10 +32,7 @@ use rustc::ty::util::CopyImplementationError;
 use middle::free_region::FreeRegionMap;
 use CrateCtxt;
 use rustc::infer::{self, InferCtxt, TypeOrigin};
-use std::cell::RefCell;
-use std::rc::Rc;
 use syntax_pos::Span;
-use util::nodemap::{DefIdMap, FnvHashMap};
 use rustc::dep_graph::DepNode;
 use rustc::hir::map as hir_map;
 use rustc::hir::intravisit;
@@ -49,7 +46,6 @@ mod unsafety;
 struct CoherenceChecker<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     crate_context: &'a CrateCtxt<'a, 'gcx>,
     inference_context: InferCtxt<'a, 'gcx, 'tcx>,
-    inherent_impls: RefCell<DefIdMap<Rc<RefCell<Vec<DefId>>>>>,
 }
 
 struct CoherenceCheckVisitor<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
@@ -109,15 +105,6 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> {
             DepNode::CoherenceCheckImpl,
             &mut CoherenceCheckVisitor { cc: self });
 
-        // Copy over the inherent impls we gathered up during the walk into
-        // the tcx.
-        let mut tcx_inherent_impls =
-            self.crate_context.tcx.inherent_impls.borrow_mut();
-        for (k, v) in self.inherent_impls.borrow().iter() {
-            tcx_inherent_impls.insert((*k).clone(),
-                                      Rc::new((*v.borrow()).clone()));
-        }
-
         // Populate the table of destructors. It might seem a bit strange to
         // do this here, but it's actually the most convenient place, since
         // the coherence tables contain the trait -> type mappings.
@@ -175,14 +162,8 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> {
     }
 
     fn add_inherent_impl(&self, base_def_id: DefId, impl_def_id: DefId) {
-        if let Some(implementation_list) = self.inherent_impls.borrow().get(&base_def_id) {
-            implementation_list.borrow_mut().push(impl_def_id);
-            return;
-        }
-
-        self.inherent_impls.borrow_mut().insert(
-            base_def_id,
-            Rc::new(RefCell::new(vec!(impl_def_id))));
+        let tcx = self.crate_context.tcx;
+        tcx.inherent_impls.borrow_mut().push(base_def_id, impl_def_id);
     }
 
     fn add_trait_impl(&self, impl_trait_ref: ty::TraitRef<'gcx>, impl_def_id: DefId) {
@@ -556,7 +537,6 @@ pub fn check_coherence(ccx: &CrateCtxt) {
         CoherenceChecker {
             crate_context: ccx,
             inference_context: infcx,
-            inherent_impls: RefCell::new(FnvHashMap()),
         }.check();
     });
     unsafety::check(ccx.tcx);

src/test/incremental/krate-inherent.rs

@@ -0,0 +1,34 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// revisions: rpass1 rpass2
+// compile-flags: -Z query-dep-graph
+
+#![allow(warnings)]
+#![feature(rustc_attrs)]
+#![rustc_partition_reused(module="krate_inherent-x", cfg="rpass2")]
+
+fn main() { }
+
+mod x {
+    struct Foo;
+    impl Foo {
+        fn foo(&self) { }
+    }
+
+    fn method() {
+        let x: Foo = Foo;
+        x.foo(); // inherent methods used to add an edge from Krate
+    }
+}
+
+#[cfg(rpass1)]
+fn bar() { } // remove this unrelated fn in rpass2, which should not affect `x::method`
+