Auto merge of rust-lang#39434 - nikomatsakis:incr-comp-skip-typeck-2, r=eddyb

Miscellaneous refactors around how lints and typeck interact

This is preparation for making incr. comp. skip typeck. The main gist of is trying to rationalize the outputs from typeck that are not part of tables:

- one bit of output is the `used_trait_imports` set, which becomes something we track for dependencies
- the other big of output are various lints; we used to store these into a table on sess, but this work stores them into the`TypeckTables`, and then makes the lint pass consult that
    - I think it probably makes sense to handle errors similarly, eventually, but that's not necessary now

r? @eddyb

Fixes rust-lang#39495

Author: bors

src/Cargo.lock

@@ -113,6 +113,7 @@ pub enum DepNode<D: Clone + Debug> {
     AssociatedItemDefIds(D),
     InherentImpls(D),
     TypeckTables(D),
+    UsedTraitImports(D),
 
     // The set of impls for a given trait. Ultimately, it would be
     // nice to get more fine-grained here (e.g., to include a
@@ -162,6 +163,7 @@ impl<D: Clone + Debug> DepNode<D> {
             AssociatedItemDefIds,
             InherentImpls,
             TypeckTables,
+            UsedTraitImports,
             TraitImpls,
             ReprHints,
         }
@@ -230,6 +232,7 @@ impl<D: Clone + Debug> DepNode<D> {
             AssociatedItemDefIds(ref d) => op(d).map(AssociatedItemDefIds),
             InherentImpls(ref d) => op(d).map(InherentImpls),
             TypeckTables(ref d) => op(d).map(TypeckTables),
+            UsedTraitImports(ref d) => op(d).map(UsedTraitImports),
             TraitImpls(ref d) => op(d).map(TraitImpls),
             TraitItems(ref d) => op(d).map(TraitItems),
             ReprHints(ref d) => op(d).map(ReprHints),

src/librustc/dep_graph/dep_node.rs

@@ -61,15 +61,10 @@ impl<M: DepTrackingMapConfig> DepTrackingMap<M> {
         self.map.get(k)
     }
 
-    pub fn get_mut(&mut self, k: &M::Key) -> Option<&mut M::Value> {
-        self.read(k);
-        self.write(k);
-        self.map.get_mut(k)
-    }
-
-    pub fn insert(&mut self, k: M::Key, v: M::Value) -> Option<M::Value> {
+    pub fn insert(&mut self, k: M::Key, v: M::Value) {
         self.write(&k);
-        self.map.insert(k, v)
+        let old_value = self.map.insert(k, v);
+        assert!(old_value.is_none());
     }
 
     pub fn contains_key(&self, k: &M::Key) -> bool {
@@ -83,6 +78,10 @@ impl<M: DepTrackingMapConfig> DepTrackingMap<M> {
 
     /// Append `elem` to the vector stored for `k`, creating a new vector if needed.
     /// This is considered a write to `k`.
+    ///
+    /// NOTE: Caution is required when using this method. You should
+    /// be sure that nobody is **reading from the vector** while you
+    /// are writing to it. Eventually, it'd be nice to remove this.
     pub fn push<E: Clone>(&mut self, k: M::Key, elem: E)
         where M: DepTrackingMapConfig<Value=Vec<E>>
     {

src/librustc/dep_graph/dep_tracking_map.rs

@@ -33,6 +33,7 @@ use lint::{Level, LevelSource, Lint, LintId, LintPass, LintSource};
 use lint::{EarlyLintPassObject, LateLintPassObject};
 use lint::{Default, CommandLine, Node, Allow, Warn, Deny, Forbid};
 use lint::builtin;
+use rustc_serialize::{Decoder, Decodable, Encoder, Encodable};
 use util::nodemap::FxHashMap;
 
 use std::cmp;
@@ -82,7 +83,7 @@ pub struct LintStore {
 
 /// When you call `add_lint` on the session, you wind up storing one
 /// of these, which records a "potential lint" at a particular point.
-#[derive(PartialEq)]
+#[derive(PartialEq, RustcEncodable, RustcDecodable)]
 pub struct EarlyLint {
     /// what lint is this? (e.g., `dead_code`)
     pub id: LintId,
@@ -558,7 +559,7 @@ pub trait LintContext<'tcx>: Sized {
         self.lookup_and_emit(lint, Some(span), msg);
     }
 
-    fn early_lint(&self, early_lint: EarlyLint) {
+    fn early_lint(&self, early_lint: &EarlyLint) {
         let span = early_lint.diagnostic.span.primary_span().expect("early lint w/o primary span");
         let mut err = self.struct_span_lint(early_lint.id.lint,
                                             span,
@@ -773,11 +774,10 @@ impl<'a, 'tcx> hir_visit::Visitor<'tcx> for LateContext<'a, 'tcx> {
 
     // Output any lints that were previously added to the session.
     fn visit_id(&mut self, id: ast::NodeId) {
-        if let Some(lints) = self.sess().lints.borrow_mut().remove(&id) {
-            debug!("LateContext::visit_id: id={:?} lints={:?}", id, lints);
-            for early_lint in lints {
-                self.early_lint(early_lint);
-            }
+        let lints = self.sess().lints.borrow_mut().take(id);
+        for early_lint in lints.iter().chain(self.tables.lints.get(id)) {
+            debug!("LateContext::visit_id: id={:?} early_lint={:?}", id, early_lint);
+            self.early_lint(early_lint);
         }
     }
 
@@ -1232,7 +1232,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 
     // If we missed any lints added to the session, then there's a bug somewhere
     // in the iteration code.
-    for (id, v) in tcx.sess.lints.borrow().iter() {
+    if let Some((id, v)) = tcx.sess.lints.borrow().get_any() {
         for early_lint in v {
             span_bug!(early_lint.diagnostic.span.clone(),
                       "unprocessed lint {:?} at {}",
@@ -1250,10 +1250,9 @@ pub fn check_ast_crate(sess: &Session, krate: &ast::Crate) {
     // Visit the whole crate.
     cx.with_lint_attrs(&krate.attrs, |cx| {
         // Lints may be assigned to the whole crate.
-        if let Some(lints) = cx.sess.lints.borrow_mut().remove(&ast::CRATE_NODE_ID) {
-            for early_lint in lints {
-                cx.early_lint(early_lint);
-            }
+        let lints = cx.sess.lints.borrow_mut().take(ast::CRATE_NODE_ID);
+        for early_lint in lints {
+            cx.early_lint(&early_lint);
         }
 
         // since the root module isn't visited as an item (because it isn't an
@@ -1270,9 +1269,28 @@ pub fn check_ast_crate(sess: &Session, krate: &ast::Crate) {
 
     // If we missed any lints added to the session, then there's a bug somewhere
     // in the iteration code.
-    for (_, v) in sess.lints.borrow().iter() {
+    for (_, v) in sess.lints.borrow().get_any() {
         for early_lint in v {
             span_bug!(early_lint.diagnostic.span.clone(), "unprocessed lint {:?}", early_lint);
         }
     }
 }
+
+impl Encodable for LintId {
+    fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
+        s.emit_str(&self.lint.name.to_lowercase())
+    }
+}
+
+impl Decodable for LintId {
+    #[inline]
+    fn decode<D: Decoder>(d: &mut D) -> Result<LintId, D::Error> {
+        let s = d.read_str()?;
+        ty::tls::with(|tcx| {
+            match tcx.sess.lint_store.borrow().find_lint(&s, tcx.sess, None) {
+                Ok(id) => Ok(id),
+                Err(_) => panic!("invalid lint-id `{}`", s),
+            }
+        })
+    }
+}

src/librustc/lint/context.rs

@@ -31,18 +31,20 @@
 pub use self::Level::*;
 pub use self::LintSource::*;
 
+use hir;
+use hir::intravisit::FnKind;
 use std::hash;
 use std::ascii::AsciiExt;
 use syntax_pos::Span;
-use hir::intravisit::FnKind;
 use syntax::visit as ast_visit;
 use syntax::ast;
-use hir;
 
 pub use lint::context::{LateContext, EarlyContext, LintContext, LintStore,
                         raw_emit_lint, check_crate, check_ast_crate, gather_attrs,
                         raw_struct_lint, FutureIncompatibleInfo, EarlyLint, IntoEarlyLint};
 
+pub use lint::table::LintTable;
+
 /// Specification of a single lint.
 #[derive(Copy, Clone, Debug)]
 pub struct Lint {
@@ -346,3 +348,4 @@ pub type LevelSource = (Level, LintSource);
 
 pub mod builtin;
 mod context;
+mod table;

src/librustc/lint/mod.rs

@@ -0,0 +1,71 @@
+// Copyright 2012-2015 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.
+
+use syntax::ast;
+use syntax_pos::MultiSpan;
+use util::nodemap::NodeMap;
+
+use super::{Lint, LintId, EarlyLint, IntoEarlyLint};
+
+#[derive(RustcEncodable, RustcDecodable)]
+pub struct LintTable {
+    map: NodeMap<Vec<EarlyLint>>
+}
+
+impl LintTable {
+    pub fn new() -> Self {
+        LintTable { map: NodeMap() }
+    }
+
+    pub fn add_lint<S: Into<MultiSpan>>(&mut self,
+                                        lint: &'static Lint,
+                                        id: ast::NodeId,
+                                        sp: S,
+                                        msg: String)
+    {
+        self.add_lint_diagnostic(lint, id, (sp, &msg[..]))
+    }
+
+    pub fn add_lint_diagnostic<M>(&mut self,
+                                  lint: &'static Lint,
+                                  id: ast::NodeId,
+                                  msg: M)
+        where M: IntoEarlyLint,
+    {
+        let lint_id = LintId::of(lint);
+        let early_lint = msg.into_early_lint(lint_id);
+        let arr = self.map.entry(id).or_insert(vec![]);
+        if !arr.contains(&early_lint) {
+            arr.push(early_lint);
+        }
+    }
+
+    pub fn get(&self, id: ast::NodeId) -> &[EarlyLint] {
+        self.map.get(&id).map(|v| &v[..]).unwrap_or(&[])
+    }
+
+    pub fn take(&mut self, id: ast::NodeId) -> Vec<EarlyLint> {
+        self.map.remove(&id).unwrap_or(vec![])
+    }
+
+    pub fn transfer(&mut self, into: &mut LintTable) {
+        into.map.extend(self.map.drain());
+    }
+
+    /// Returns the first (id, lint) pair that is non-empty. Used to
+    /// implement a sanity check in lints that all node-ids are
+    /// visited.
+    pub fn get_any(&self) -> Option<(&ast::NodeId, &Vec<EarlyLint>)> {
+        self.map.iter()
+                .filter(|&(_, v)| !v.is_empty())
+                .next()
+    }
+}
+

src/librustc/lint/table.rs

@@ -20,7 +20,7 @@ use middle::dependency_format;
 use session::search_paths::PathKind;
 use session::config::DebugInfoLevel;
 use ty::tls;
-use util::nodemap::{NodeMap, FxHashMap, FxHashSet};
+use util::nodemap::{FxHashMap, FxHashSet};
 use util::common::duration_to_secs_str;
 use mir::transform as mir_pass;
 
@@ -78,7 +78,7 @@ pub struct Session {
     pub local_crate_source_file: Option<PathBuf>,
     pub working_dir: PathBuf,
     pub lint_store: RefCell<lint::LintStore>,
-    pub lints: RefCell<NodeMap<Vec<lint::EarlyLint>>>,
+    pub lints: RefCell<lint::LintTable>,
     /// Set of (LintId, span, message) tuples tracking lint (sub)diagnostics
     /// that have been set once, but should not be set again, in order to avoid
     /// redundantly verbose output (Issue #24690).
@@ -270,13 +270,14 @@ impl Session {
     pub fn unimpl(&self, msg: &str) -> ! {
         self.diagnostic().unimpl(msg)
     }
+
     pub fn add_lint<S: Into<MultiSpan>>(&self,
                                         lint: &'static lint::Lint,
                                         id: ast::NodeId,
                                         sp: S,
                                         msg: String)
     {
-        self.add_lint_diagnostic(lint, id, (sp, &msg[..]))
+        self.lints.borrow_mut().add_lint(lint, id, sp, msg);
     }
 
     pub fn add_lint_diagnostic<M>(&self,
@@ -285,17 +286,9 @@ impl Session {
                                   msg: M)
         where M: lint::IntoEarlyLint,
     {
-        let lint_id = lint::LintId::of(lint);
-        let mut lints = self.lints.borrow_mut();
-        let early_lint = msg.into_early_lint(lint_id);
-        if let Some(arr) = lints.get_mut(&id) {
-            if !arr.contains(&early_lint) {
-                arr.push(early_lint);
-            }
-            return;
-        }
-        lints.insert(id, vec![early_lint]);
+        self.lints.borrow_mut().add_lint_diagnostic(lint, id, msg);
     }
+
     pub fn reserve_node_ids(&self, count: usize) -> ast::NodeId {
         let id = self.next_node_id.get();
 
@@ -617,7 +610,7 @@ pub fn build_session_(sopts: config::Options,
         local_crate_source_file: local_crate_source_file,
         working_dir: env::current_dir().unwrap(),
         lint_store: RefCell::new(lint::LintStore::new()),
-        lints: RefCell::new(NodeMap()),
+        lints: RefCell::new(lint::LintTable::new()),
         one_time_diagnostics: RefCell::new(FxHashSet()),
         plugin_llvm_passes: RefCell::new(Vec::new()),
         mir_passes: RefCell::new(mir_pass::Passes::new()),

src/librustc/session/mod.rs

@@ -12,6 +12,7 @@
 
 use dep_graph::{DepGraph, DepTrackingMap};
 use session::Session;
+use lint;
 use middle;
 use hir::TraitMap;
 use hir::def::Def;
@@ -237,6 +238,9 @@ pub struct TypeckTables<'tcx> {
     /// Maps a cast expression to its kind. This is keyed on the
     /// *from* expression of the cast, not the cast itself.
     pub cast_kinds: NodeMap<ty::cast::CastKind>,
+
+    /// Lints for the body of this fn generated by typeck.
+    pub lints: lint::LintTable,
 }
 
 impl<'tcx> TypeckTables<'tcx> {
@@ -253,6 +257,7 @@ impl<'tcx> TypeckTables<'tcx> {
             liberated_fn_sigs: NodeMap(),
             fru_field_types: NodeMap(),
             cast_kinds: NodeMap(),
+            lints: lint::LintTable::new(),
         }
     }
 
@@ -494,7 +499,7 @@ pub struct GlobalCtxt<'tcx> {
 
     /// Set of trait imports actually used in the method resolution.
     /// This is used for warning unused imports.
-    pub used_trait_imports: RefCell<NodeSet>,
+    pub used_trait_imports: RefCell<DepTrackingMap<maps::UsedTraitImports<'tcx>>>,
 
     /// The set of external nominal types whose implementations have been read.
     /// This is used for lazy resolution of methods.
@@ -783,7 +788,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             inherent_impls: RefCell::new(DepTrackingMap::new(dep_graph.clone())),
             used_unsafe: RefCell::new(NodeSet()),
             used_mut_nodes: RefCell::new(NodeSet()),
-            used_trait_imports: RefCell::new(NodeSet()),
+            used_trait_imports: RefCell::new(DepTrackingMap::new(dep_graph.clone())),
             populated_external_types: RefCell::new(DefIdSet()),
             populated_external_primitive_impls: RefCell::new(DefIdSet()),
             stability: RefCell::new(stability),

src/librustc/ty/context.rs

@@ -12,6 +12,7 @@ use dep_graph::{DepNode, DepTrackingMapConfig};
 use hir::def_id::DefId;
 use mir;
 use ty::{self, Ty};
+use util::nodemap::DefIdSet;
 
 use std::cell::RefCell;
 use std::marker::PhantomData;
@@ -49,3 +50,4 @@ dep_map_ty! { Mir: Mir(DefId) -> &'tcx RefCell<mir::Mir<'tcx>> }
 dep_map_ty! { ClosureKinds: ItemSignature(DefId) -> ty::ClosureKind }
 dep_map_ty! { ClosureTypes: ItemSignature(DefId) -> ty::ClosureTy<'tcx> }
 dep_map_ty! { TypeckTables: TypeckTables(DefId) -> &'tcx ty::TypeckTables<'tcx> }
+dep_map_ty! { UsedTraitImports: UsedTraitImports(DefId) -> DefIdSet }

src/librustc/ty/maps.rs

@@ -9,4 +9,5 @@ path = "lib.rs"
 crate-type = ["dylib"]
 
 [dependencies]
+serialize = { path = "../libserialize" }
 syntax_pos = { path = "../libsyntax_pos" }