From 5a6019429f68d2fa8b92285d7dfdcd2f30fd1303 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 27 Jan 2017 17:19:02 -0500 Subject: [PATCH 1/8] don't use a mutable field where parameter passing will do --- src/librustc_typeck/check/method/probe.rs | 37 ++++++++++++----------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 300caca30fec8..201a223c15f15 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -55,7 +55,6 @@ struct ProbeContext<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { inherent_candidates: Vec>, extension_candidates: Vec>, impl_dups: FxHashSet, - import_id: Option, /// Collects near misses when the candidate functions are missing a `self` keyword and is only /// used for error reporting @@ -351,7 +350,6 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { inherent_candidates: Vec::new(), extension_candidates: Vec::new(), impl_dups: FxHashSet(), - import_id: None, steps: Rc::new(steps), opt_simplified_steps: opt_simplified_steps, static_candidates: Vec::new(), @@ -530,7 +528,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { xform_self_ty: xform_self_ty, item: item, kind: InherentImplCandidate(impl_substs, obligations), - import_id: self.import_id, + import_id: None, }); } } @@ -559,7 +557,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { xform_self_ty: xform_self_ty, item: item, kind: ObjectCandidate, - import_id: this.import_id, + import_id: None, }); }); } @@ -609,7 +607,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { xform_self_ty: xform_self_ty, item: item, kind: WhereClauseCandidate(poly_trait_ref), - import_id: this.import_id, + import_id: None, }); }); } @@ -644,9 +642,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { for trait_candidate in applicable_traits { let trait_did = trait_candidate.def_id; if duplicates.insert(trait_did) { - self.import_id = trait_candidate.import_id; - let result = self.assemble_extension_candidates_for_trait(trait_did); - self.import_id = None; + let import_id = trait_candidate.import_id; + let result = self.assemble_extension_candidates_for_trait(import_id, trait_did); result?; } } @@ -658,7 +655,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { let mut duplicates = FxHashSet(); for trait_info in suggest::all_traits(self.ccx) { if duplicates.insert(trait_info.def_id) { - self.assemble_extension_candidates_for_trait(trait_info.def_id)?; + self.assemble_extension_candidates_for_trait(None, trait_info.def_id)?; } } Ok(()) @@ -682,6 +679,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { } fn assemble_extension_candidates_for_trait(&mut self, + import_id: Option, trait_def_id: DefId) -> Result<(), MethodError<'tcx>> { debug!("assemble_extension_candidates_for_trait(trait_def_id={:?})", @@ -695,19 +693,21 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { continue; } - self.assemble_extension_candidates_for_trait_impls(trait_def_id, item.clone()); + self.assemble_extension_candidates_for_trait_impls(import_id, trait_def_id, + item.clone()); - self.assemble_closure_candidates(trait_def_id, item.clone())?; + self.assemble_closure_candidates(import_id, trait_def_id, item.clone())?; - self.assemble_projection_candidates(trait_def_id, item.clone()); + self.assemble_projection_candidates(import_id, trait_def_id, item.clone()); - self.assemble_where_clause_candidates(trait_def_id, item.clone()); + self.assemble_where_clause_candidates(import_id, trait_def_id, item.clone()); } Ok(()) } fn assemble_extension_candidates_for_trait_impls(&mut self, + import_id: Option, trait_def_id: DefId, item: ty::AssociatedItem) { let trait_def = self.tcx.lookup_trait_def(trait_def_id); @@ -751,7 +751,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { xform_self_ty: xform_self_ty, item: item.clone(), kind: ExtensionImplCandidate(impl_def_id, impl_substs, obligations), - import_id: self.import_id, + import_id: import_id, }); }); } @@ -777,6 +777,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { } fn assemble_closure_candidates(&mut self, + import_id: Option, trait_def_id: DefId, item: ty::AssociatedItem) -> Result<(), MethodError<'tcx>> { @@ -840,7 +841,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { xform_self_ty: xform_self_ty, item: item.clone(), kind: TraitCandidate, - import_id: self.import_id, + import_id: import_id, }); } @@ -848,6 +849,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { } fn assemble_projection_candidates(&mut self, + import_id: Option, trait_def_id: DefId, item: ty::AssociatedItem) { debug!("assemble_projection_candidates(\ @@ -895,7 +897,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { xform_self_ty: xform_self_ty, item: item.clone(), kind: TraitCandidate, - import_id: self.import_id, + import_id: import_id, }); } } @@ -903,6 +905,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { } fn assemble_where_clause_candidates(&mut self, + import_id: Option, trait_def_id: DefId, item: ty::AssociatedItem) { debug!("assemble_where_clause_candidates(trait_def_id={:?})", @@ -923,7 +926,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { xform_self_ty: xform_self_ty, item: item.clone(), kind: WhereClauseCandidate(poly_bound), - import_id: self.import_id, + import_id: import_id, }); } } From 93e0bc6520b8d1e1ae0e886cdf7b083073e40be8 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 28 Jan 2017 06:02:28 -0500 Subject: [PATCH 2/8] change the `used_trait_imports` map to be a `DefIdSet` --- src/librustc/ty/context.rs | 4 ++-- src/librustc_typeck/check/method/mod.rs | 6 ++++-- src/librustc_typeck/check_unused.rs | 4 +++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index ce4a6a3182635..5913ed48528f7 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -494,7 +494,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, + pub used_trait_imports: RefCell, /// The set of external nominal types whose implementations have been read. /// This is used for lazy resolution of methods. @@ -783,7 +783,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(DefIdSet()), populated_external_types: RefCell::new(DefIdSet()), populated_external_primitive_impls: RefCell::new(DefIdSet()), stability: RefCell::new(stability), diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 9b8e77301e524..f8b6fbc9e816b 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -137,7 +137,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self_ty, call_expr.id)?; if let Some(import_id) = pick.import_id { - self.tcx.used_trait_imports.borrow_mut().insert(import_id); + let import_def_id = self.tcx.hir.local_def_id(import_id); + self.tcx.used_trait_imports.borrow_mut().insert(import_def_id); } self.tcx.check_stability(pick.item.def_id, call_expr.id, span); @@ -336,7 +337,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self_ty, expr_id)?; if let Some(import_id) = pick.import_id { - self.tcx.used_trait_imports.borrow_mut().insert(import_id); + let import_def_id = self.tcx.hir.local_def_id(import_id); + self.tcx.used_trait_imports.borrow_mut().insert(import_def_id); } let def = pick.item.def(); diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs index bdda538db160e..c1ba53afad4e9 100644 --- a/src/librustc_typeck/check_unused.rs +++ b/src/librustc_typeck/check_unused.rs @@ -27,7 +27,9 @@ impl<'a, 'tcx> UnusedTraitImportVisitor<'a, 'tcx> { if !self.tcx.maybe_unused_trait_imports.contains(&id) { return; } - if self.tcx.used_trait_imports.borrow().contains(&id) { + + let import_def_id = self.tcx.hir.local_def_id(id); + if self.tcx.used_trait_imports.borrow().contains(&import_def_id) { return; } From 65b93ebcb8e2ff2d8747ce731108e8f5d9f7dddf Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 28 Jan 2017 07:01:45 -0500 Subject: [PATCH 3/8] introduce `LintTable` --- src/librustc/lint/context.rs | 20 ++++++------- src/librustc/lint/mod.rs | 3 ++ src/librustc/lint/table.rs | 56 ++++++++++++++++++++++++++++++++++++ src/librustc/session/mod.rs | 21 +++++--------- 4 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 src/librustc/lint/table.rs diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 3506a9c067ca6..edf5666a3a5f7 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -773,11 +773,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 { + debug!("LateContext::visit_id: id={:?} early_lint={:?}", id, early_lint); + self.early_lint(early_lint); } } @@ -1232,7 +1231,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 +1249,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,7 +1268,7 @@ 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); } diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 7e0da00694c4a..704e32e2d0c1a 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -43,6 +43,8 @@ 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; diff --git a/src/librustc/lint/table.rs b/src/librustc/lint/table.rs new file mode 100644 index 0000000000000..3b6d268b08fb0 --- /dev/null +++ b/src/librustc/lint/table.rs @@ -0,0 +1,56 @@ +use syntax::ast; +use syntax_pos::MultiSpan; +use util::nodemap::NodeMap; + +use super::{Lint, LintId, EarlyLint, IntoEarlyLint}; + +pub struct LintTable { + map: NodeMap> +} + +impl LintTable { + pub fn new() -> Self { + LintTable { map: NodeMap() } + } + + pub fn add_lint>(&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(&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 { + self.map.remove(&id).unwrap_or(vec![]) + } + + /// 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)> { + self.map.iter() + .filter(|&(_, v)| !v.is_empty()) + .next() + } +} + diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 36a887e062273..f10ea3544f2ca 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.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, pub working_dir: PathBuf, pub lint_store: RefCell, - pub lints: RefCell>>, + pub lints: RefCell, /// 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>(&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(&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()), From d9aaca71cc61a7c0792da2789b5e278cc294733f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 28 Jan 2017 18:13:21 -0500 Subject: [PATCH 4/8] store typeck lints in the `TypeckTables` Otherwise they are a "hidden output" --- src/Cargo.lock | 1 + src/librustc/lint/context.rs | 28 ++++++++++++++++++--- src/librustc/lint/mod.rs | 4 +-- src/librustc/lint/table.rs | 15 ++++++++++++ src/librustc/ty/context.rs | 5 ++++ src/librustc_errors/Cargo.toml | 1 + src/librustc_errors/diagnostic.rs | 4 +-- src/librustc_errors/lib.rs | 7 +++--- src/librustc_errors/snippet.rs | 2 +- src/librustc_typeck/check/cast.rs | 34 ++++++++++++++------------ src/librustc_typeck/check/mod.rs | 7 +++--- src/librustc_typeck/check/writeback.rs | 9 +++++++ src/libsyntax_pos/lib.rs | 2 +- 13 files changed, 87 insertions(+), 32 deletions(-) diff --git a/src/Cargo.lock b/src/Cargo.lock index 93bbf0f227b1b..f40d50dd59dc1 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -364,6 +364,7 @@ dependencies = [ name = "rustc_errors" version = "0.0.0" dependencies = [ + "serialize 0.0.0", "syntax_pos 0.0.0", ] diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index edf5666a3a5f7..362117d860a5c 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.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, @@ -774,7 +775,7 @@ 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) { let lints = self.sess().lints.borrow_mut().take(id); - for early_lint in lints { + 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); } @@ -1251,7 +1252,7 @@ pub fn check_ast_crate(sess: &Session, krate: &ast::Crate) { // Lints may be assigned to the whole crate. let lints = cx.sess.lints.borrow_mut().take(ast::CRATE_NODE_ID); for early_lint in lints { - cx.early_lint(early_lint); + cx.early_lint(&early_lint); } // since the root module isn't visited as an item (because it isn't an @@ -1274,3 +1275,22 @@ pub fn check_ast_crate(sess: &Session, krate: &ast::Crate) { } } } + +impl Encodable for LintId { + fn encode(&self, s: &mut S) -> Result<(), S::Error> { + s.emit_str(&self.lint.name.to_lowercase()) + } +} + +impl Decodable for LintId { + #[inline] + fn decode(d: &mut D) -> Result { + 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), + } + }) + } +} diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 704e32e2d0c1a..d12065ca86e14 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -31,13 +31,13 @@ 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, diff --git a/src/librustc/lint/table.rs b/src/librustc/lint/table.rs index 3b6d268b08fb0..f2dab25229ae4 100644 --- a/src/librustc/lint/table.rs +++ b/src/librustc/lint/table.rs @@ -1,9 +1,20 @@ +// 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 or the MIT license +// , 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> } @@ -44,6 +55,10 @@ impl LintTable { 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. diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 5913ed48528f7..a3b81586738b5 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.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, + + /// 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(), } } diff --git a/src/librustc_errors/Cargo.toml b/src/librustc_errors/Cargo.toml index 2ba1f501a63d8..78ff52b4b2371 100644 --- a/src/librustc_errors/Cargo.toml +++ b/src/librustc_errors/Cargo.toml @@ -9,4 +9,5 @@ path = "lib.rs" crate-type = ["dylib"] [dependencies] +serialize = { path = "../libserialize" } syntax_pos = { path = "../libsyntax_pos" } diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index ac39af2018998..1b77ead92deb6 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -17,7 +17,7 @@ use syntax_pos::{MultiSpan, Span}; use snippet::Style; #[must_use] -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable)] pub struct Diagnostic { pub level: Level, pub message: Vec<(String, Style)>, @@ -27,7 +27,7 @@ pub struct Diagnostic { } /// For example a note attached to an error. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable)] pub struct SubDiagnostic { pub level: Level, pub message: Vec<(String, Style)>, diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index bf5f7cde7eb06..d7bd5ed23c2b0 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -26,6 +26,7 @@ extern crate term; extern crate libc; +extern crate serialize as rustc_serialize; extern crate syntax_pos; pub use emitter::ColorConfig; @@ -49,7 +50,7 @@ mod lock; use syntax_pos::{BytePos, Loc, FileLinesResult, FileName, MultiSpan, Span, NO_EXPANSION}; use syntax_pos::MacroBacktrace; -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable)] pub enum RenderSpan { /// A FullSpan renders with both with an initial line for the /// message, prefixed by file:linenum, followed by a summary of @@ -63,7 +64,7 @@ pub enum RenderSpan { Suggestion(CodeSuggestion), } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable)] pub struct CodeSuggestion { pub msp: MultiSpan, pub substitutes: Vec, @@ -477,7 +478,7 @@ impl Handler { } -#[derive(Copy, PartialEq, Clone, Debug)] +#[derive(Copy, PartialEq, Clone, Debug, RustcEncodable, RustcDecodable)] pub enum Level { Bug, Fatal, diff --git a/src/librustc_errors/snippet.rs b/src/librustc_errors/snippet.rs index 95b03677b7285..5debbf4d37c20 100644 --- a/src/librustc_errors/snippet.rs +++ b/src/librustc_errors/snippet.rs @@ -204,7 +204,7 @@ pub struct StyledString { pub style: Style, } -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq, RustcEncodable, RustcDecodable)] pub enum Style { HeaderMsg, FileNameStyle, diff --git a/src/librustc_typeck/check/cast.rs b/src/librustc_typeck/check/cast.rs index 6215b4498dc68..441d427fe499e 100644 --- a/src/librustc_typeck/check/cast.rs +++ b/src/librustc_typeck/check/cast.rs @@ -311,23 +311,25 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> { let t_cast = self.cast_ty; let t_expr = self.expr_ty; if t_cast.is_numeric() && t_expr.is_numeric() { - fcx.tcx.sess.add_lint(lint::builtin::TRIVIAL_NUMERIC_CASTS, - self.expr.id, - self.span, - format!("trivial numeric cast: `{}` as `{}`. Cast can be \ - replaced by coercion, this might require type \ - ascription or a temporary variable", - fcx.ty_to_string(t_expr), - fcx.ty_to_string(t_cast))); + fcx.tables.borrow_mut().lints.add_lint( + lint::builtin::TRIVIAL_NUMERIC_CASTS, + self.expr.id, + self.span, + format!("trivial numeric cast: `{}` as `{}`. Cast can be \ + replaced by coercion, this might require type \ + ascription or a temporary variable", + fcx.ty_to_string(t_expr), + fcx.ty_to_string(t_cast))); } else { - fcx.tcx.sess.add_lint(lint::builtin::TRIVIAL_CASTS, - self.expr.id, - self.span, - format!("trivial cast: `{}` as `{}`. Cast can be \ - replaced by coercion, this might require type \ - ascription or a temporary variable", - fcx.ty_to_string(t_expr), - fcx.ty_to_string(t_cast))); + fcx.tables.borrow_mut().lints.add_lint( + lint::builtin::TRIVIAL_CASTS, + self.expr.id, + self.span, + format!("trivial cast: `{}` as `{}`. Cast can be \ + replaced by coercion, this might require type \ + ascription or a temporary variable", + fcx.ty_to_string(t_expr), + fcx.ty_to_string(t_cast))); } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index c435f9341253e..573cbfcc3b062 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1521,9 +1521,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if self.diverges.get() == Diverges::Always { self.diverges.set(Diverges::WarnedAlways); - self.tcx.sess.add_lint(lint::builtin::UNREACHABLE_CODE, - id, span, - format!("unreachable {}", kind)); + self.tables.borrow_mut().lints.add_lint( + lint::builtin::UNREACHABLE_CODE, + id, span, + format!("unreachable {}", kind)); } } diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 7f82d7829ce52..3a467c0296a52 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -52,6 +52,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { wbcx.visit_deferred_obligations(item_id); wbcx.visit_type_nodes(); wbcx.visit_cast_types(); + wbcx.visit_lints(); let tables = self.tcx.alloc_tables(wbcx.tables); self.tcx.tables.borrow_mut().insert(item_def_id, tables); @@ -301,6 +302,14 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> { self.fcx.tables.borrow().cast_kinds.iter().map(|(&key, &value)| (key, value))); } + fn visit_lints(&mut self) { + if self.fcx.writeback_errors.get() { + return + } + + self.fcx.tables.borrow_mut().lints.transfer(&mut self.tables.lints); + } + fn visit_anon_types(&self) { if self.fcx.writeback_errors.get() { return diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index 92fdb45caaaef..3808923e7728f 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -66,7 +66,7 @@ pub struct Span { /// the error, and would be rendered with `^^^`. /// - they can have a *label*. In this case, the label is written next /// to the mark in the snippet when we render. -#[derive(Clone, Debug, Hash, PartialEq, Eq)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, RustcEncodable, RustcDecodable)] pub struct MultiSpan { primary_spans: Vec, span_labels: Vec<(Span, String)>, From fdd7e3c74419385541e2ba244117e7aa65f72a01 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 1 Feb 2017 16:07:21 -0500 Subject: [PATCH 5/8] remove some of the ways to mutate a `DepTrackingMap` It is pretty suspect to insert an entry twice. --- src/librustc/dep_graph/dep_tracking_map.rs | 22 +++------------------ src/librustc_mir/mir_map.rs | 2 +- src/librustc_typeck/coherence/mod.rs | 23 +++++++++++++++------- src/librustc_typeck/collect.rs | 13 ++++-------- src/librustc_typeck/variance/solve.rs | 8 +++----- src/librustc_typeck/variance/terms.rs | 6 ++---- 6 files changed, 29 insertions(+), 45 deletions(-) diff --git a/src/librustc/dep_graph/dep_tracking_map.rs b/src/librustc/dep_graph/dep_tracking_map.rs index 9660758220368..103f8ef653c1e 100644 --- a/src/librustc/dep_graph/dep_tracking_map.rs +++ b/src/librustc/dep_graph/dep_tracking_map.rs @@ -61,15 +61,10 @@ impl DepTrackingMap { 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 { + 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 { @@ -80,17 +75,6 @@ impl DepTrackingMap { pub fn keys(&self) -> Vec { 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(&mut self, k: M::Key, elem: E) - where M: DepTrackingMapConfig> - { - self.write(&k); - self.map.entry(k) - .or_insert(Vec::new()) - .push(elem); - } } impl MemoizationMap for RefCell> { diff --git a/src/librustc_mir/mir_map.rs b/src/librustc_mir/mir_map.rs index c71255dcc7c61..b7f90682c7c74 100644 --- a/src/librustc_mir/mir_map.rs +++ b/src/librustc_mir/mir_map.rs @@ -139,7 +139,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BuildMir<'a, 'tcx> { let mir = tcx.alloc_mir(mir); let def_id = tcx.hir.local_def_id(src.item_id()); - assert!(tcx.mir_map.borrow_mut().insert(def_id, mir).is_none()); + tcx.mir_map.borrow_mut().insert(def_id, mir); }); let body = self.tcx.hir.body(body_id); diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index e9c710d2fec4c..4a96a5e960dbe 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -29,17 +29,19 @@ use rustc::dep_graph::DepNode; use rustc::hir::itemlikevisit::ItemLikeVisitor; use rustc::hir::{Item, ItemImpl}; use rustc::hir; +use rustc::util::nodemap::DefIdMap; mod builtin; mod orphan; mod overlap; mod unsafety; -struct CoherenceChecker<'a, 'tcx: 'a> { +struct CoherenceCollect<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, + inherent_impls: DefIdMap>, } -impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceChecker<'a, 'tcx> { +impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceCollect<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { if let ItemImpl(..) = item.node { self.check_implementation(item) @@ -53,7 +55,7 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceChecker<'a, 'tcx> { } } -impl<'a, 'tcx> CoherenceChecker<'a, 'tcx> { +impl<'a, 'tcx> CoherenceCollect<'a, 'tcx> { // Returns the def ID of the base type, if there is one. fn get_base_type_def_id(&self, span: Span, ty: Ty<'tcx>) -> Option { match ty.sty { @@ -80,9 +82,14 @@ impl<'a, 'tcx> CoherenceChecker<'a, 'tcx> { // containing the inherent methods and extension methods. It also // builds up the trait inheritance table. self.tcx.visit_all_item_likes_in_krate(DepNode::CoherenceCheckImpl, self); + + // Transfer the inherent impl lists, not that they are known, into the tcx + for (ty_def_id, impl_def_ids) in self.inherent_impls.drain() { + self.tcx.inherent_impls.borrow_mut().insert(ty_def_id, impl_def_ids); + } } - fn check_implementation(&self, item: &Item) { + fn check_implementation(&mut self, item: &Item) { let tcx = self.tcx; let impl_did = tcx.hir.local_def_id(item.id); let self_type = tcx.item_type(impl_did); @@ -119,8 +126,10 @@ impl<'a, 'tcx> CoherenceChecker<'a, 'tcx> { } } - fn add_inherent_impl(&self, base_def_id: DefId, impl_def_id: DefId) { - self.tcx.inherent_impls.borrow_mut().push(base_def_id, impl_def_id); + fn add_inherent_impl(&mut self, base_def_id: DefId, impl_def_id: DefId) { + self.inherent_impls.entry(base_def_id) + .or_insert(vec![]) + .push(impl_def_id); } fn add_trait_impl(&self, impl_trait_ref: ty::TraitRef<'tcx>, impl_def_id: DefId) { @@ -161,7 +170,7 @@ fn enforce_trait_manually_implementable(tcx: TyCtxt, sp: Span, trait_def_id: Def pub fn check_coherence(ccx: &CrateCtxt) { let _task = ccx.tcx.dep_graph.in_task(DepNode::Coherence); - CoherenceChecker { tcx: ccx.tcx }.check(); + CoherenceCollect { tcx: ccx.tcx, inherent_impls: DefIdMap() }.check(); unsafety::check(ccx.tcx); orphan::check(ccx.tcx); overlap::check(ccx.tcx); diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index bbd0c8058151f..cb0a6cccb03d4 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -634,7 +634,7 @@ fn convert_field<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, ccx.tcx.item_types.borrow_mut().insert(ty_f.did, tt); let def_id = ccx.tcx.hir.local_def_id(field.id); - ccx.tcx.item_types.borrow_mut().insert(def_id, tt); + assert_eq!(def_id, ty_f.did); ccx.tcx.generics.borrow_mut().insert(def_id, struct_generics); ccx.tcx.predicates.borrow_mut().insert(def_id, struct_predicates.clone()); } @@ -1283,9 +1283,7 @@ fn convert_trait_predicates<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, it: &hir::Item) items); trait_predicates.predicates.extend(assoc_predicates); - let prev_predicates = tcx.predicates.borrow_mut().insert(def_id, trait_predicates); - assert!(prev_predicates.is_none()); - + tcx.predicates.borrow_mut().insert(def_id, trait_predicates); return; fn predicates_for_associated_types<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, @@ -1592,9 +1590,7 @@ fn predicates_of_item<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, }; let predicates = ty_generic_predicates(ccx, generics, None, vec![], false); - let prev_predicates = ccx.tcx.predicates.borrow_mut().insert(def_id, - predicates.clone()); - assert!(prev_predicates.is_none()); + ccx.tcx.predicates.borrow_mut().insert(def_id, predicates.clone()); predicates } @@ -1617,8 +1613,7 @@ fn convert_foreign_item<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, }; let predicates = ty_generic_predicates(ccx, generics, None, vec![], false); - let prev_predicates = ccx.tcx.predicates.borrow_mut().insert(def_id, predicates); - assert!(prev_predicates.is_none()); + ccx.tcx.predicates.borrow_mut().insert(def_id, predicates); } // Is it marked with ?Sized diff --git a/src/librustc_typeck/variance/solve.rs b/src/librustc_typeck/variance/solve.rs index bdf1d0590bcc7..3ccec97d606ea 100644 --- a/src/librustc_typeck/variance/solve.rs +++ b/src/librustc_typeck/variance/solve.rs @@ -137,11 +137,9 @@ impl<'a, 'tcx> SolveContext<'a, 'tcx> { item_variances); } - let newly_added = tcx.item_variance_map - .borrow_mut() - .insert(item_def_id, Rc::new(item_variances)) - .is_none(); - assert!(newly_added); + tcx.item_variance_map + .borrow_mut() + .insert(item_def_id, Rc::new(item_variances)); } } diff --git a/src/librustc_typeck/variance/terms.rs b/src/librustc_typeck/variance/terms.rs index 0e09d34cc9bd3..253d7a25b632f 100644 --- a/src/librustc_typeck/variance/terms.rs +++ b/src/librustc_typeck/variance/terms.rs @@ -178,12 +178,10 @@ impl<'a, 'tcx> TermsContext<'a, 'tcx> { // parameters". if self.num_inferred() == inferreds_on_entry { let item_def_id = self.tcx.hir.local_def_id(item_id); - let newly_added = self.tcx + self.tcx .item_variance_map .borrow_mut() - .insert(item_def_id, self.empty_variances.clone()) - .is_none(); - assert!(newly_added); + .insert(item_def_id, self.empty_variances.clone()); } } From 78f7ac561c0b360650b36a175f39475bc237230f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 28 Jan 2017 06:34:07 -0500 Subject: [PATCH 6/8] rejigger how we handle used trait imports The previous way was not friendly to incremental compilation. The new plan is to compute, for each body, a set of trait imports used in that body (slightly subtle: for a closure, we assign the trait imports to the enclosing fn). Then we walk all bodies and union these sets. The reason we do this is that we can save the individual sets in the incremental state, and then recompute only those sets that are needed. Before we were planning to save only the final union, but in that case if some components are invalidated we have to recompute *all* of them since we don't have enough information to "partly" invalidate a result. In truth, this set probably ought to be part of the `TypeckTables`; however, I opted not to do that because I don't want to have to save/restore the entire tables in the incremental state yet (since it contains a lot of `NodeId` references, and removing those is a significant refactoring). --- src/librustc/dep_graph/dep_node.rs | 3 +++ src/librustc/ty/context.rs | 4 ++-- src/librustc/ty/maps.rs | 2 ++ src/librustc_typeck/check/method/mod.rs | 6 ++++-- src/librustc_typeck/check/mod.rs | 8 +++++++- src/librustc_typeck/check/writeback.rs | 8 +++++++- src/librustc_typeck/check_unused.rs | 27 ++++++++++++++++++++----- src/librustc_typeck/lib.rs | 1 + 8 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index df6db366df5b3..006de1c06e2d9 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -113,6 +113,7 @@ pub enum DepNode { 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 DepNode { AssociatedItemDefIds, InherentImpls, TypeckTables, + UsedTraitImports, TraitImpls, ReprHints, } @@ -230,6 +232,7 @@ impl DepNode { 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), diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index a3b81586738b5..7dcbe04caf82b 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -499,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, + pub used_trait_imports: RefCell>>, /// The set of external nominal types whose implementations have been read. /// This is used for lazy resolution of methods. @@ -788,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(DefIdSet()), + 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), diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index c0cf1d724273e..d7341d148b720 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.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> } 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 } diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index f8b6fbc9e816b..eae8989bd342e 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -138,7 +138,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(import_id) = pick.import_id { let import_def_id = self.tcx.hir.local_def_id(import_id); - self.tcx.used_trait_imports.borrow_mut().insert(import_def_id); + debug!("used_trait_import: {:?}", import_def_id); + self.used_trait_imports.borrow_mut().insert(import_def_id); } self.tcx.check_stability(pick.item.def_id, call_expr.id, span); @@ -338,7 +339,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(import_id) = pick.import_id { let import_def_id = self.tcx.hir.local_def_id(import_id); - self.tcx.used_trait_imports.borrow_mut().insert(import_def_id); + debug!("used_trait_import: {:?}", import_def_id); + self.used_trait_imports.borrow_mut().insert(import_def_id); } let def = pick.item.def(); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 573cbfcc3b062..1d9913cd96dc3 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -102,7 +102,7 @@ use CrateCtxt; use TypeAndSubsts; use lint; use util::common::{ErrorReported, indenter}; -use util::nodemap::{DefIdMap, FxHashMap, FxHashSet, NodeMap}; +use util::nodemap::{DefIdMap, DefIdSet, FxHashMap, FxHashSet, NodeMap}; use std::cell::{Cell, RefCell}; use std::cmp; @@ -179,6 +179,11 @@ pub struct Inherited<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { // Obligations which will have to be checked at the end of // type-checking, after all functions have been inferred. deferred_obligations: RefCell>>, + + // a set of trait import def-ids that we use during method + // resolution; during writeback, this is written into + // `tcx.used_trait_imports` for this item def-id + used_trait_imports: RefCell>, } impl<'a, 'gcx, 'tcx> Deref for Inherited<'a, 'gcx, 'tcx> { @@ -513,6 +518,7 @@ impl<'a, 'gcx, 'tcx> Inherited<'a, 'gcx, 'tcx> { deferred_cast_checks: RefCell::new(Vec::new()), anon_types: RefCell::new(DefIdMap()), deferred_obligations: RefCell::new(Vec::new()), + used_trait_imports: RefCell::new(DefIdSet()), } } diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 3a467c0296a52..9df0542f51fa1 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -19,9 +19,10 @@ use rustc::ty::{self, Ty, TyCtxt, MethodCall, MethodCallee}; use rustc::ty::adjustment; use rustc::ty::fold::{TypeFolder,TypeFoldable}; use rustc::infer::{InferCtxt, FixupError}; -use rustc::util::nodemap::DefIdMap; +use rustc::util::nodemap::{DefIdMap, DefIdSet}; use std::cell::Cell; +use std::mem; use syntax::ast; use syntax_pos::Span; @@ -56,6 +57,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let tables = self.tcx.alloc_tables(wbcx.tables); self.tcx.tables.borrow_mut().insert(item_def_id, tables); + + let used_trait_imports = mem::replace(&mut *self.used_trait_imports.borrow_mut(), + DefIdSet()); + debug!("used_trait_imports({:?}) = {:?}", item_def_id, used_trait_imports); + self.tcx.used_trait_imports.borrow_mut().insert(item_def_id, used_trait_imports); } } diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs index c1ba53afad4e9..6dff6d57e4fac 100644 --- a/src/librustc_typeck/check_unused.rs +++ b/src/librustc_typeck/check_unused.rs @@ -17,19 +17,21 @@ use syntax_pos::{Span, DUMMY_SP}; use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; +use rustc::util::nodemap::DefIdSet; -struct UnusedTraitImportVisitor<'a, 'tcx: 'a> { +struct CheckVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, + used_trait_imports: DefIdSet, } -impl<'a, 'tcx> UnusedTraitImportVisitor<'a, 'tcx> { +impl<'a, 'tcx> CheckVisitor<'a, 'tcx> { fn check_import(&self, id: ast::NodeId, span: Span) { if !self.tcx.maybe_unused_trait_imports.contains(&id) { return; } let import_def_id = self.tcx.hir.local_def_id(id); - if self.tcx.used_trait_imports.borrow().contains(&import_def_id) { + if self.used_trait_imports.contains(&import_def_id) { return; } @@ -42,7 +44,7 @@ impl<'a, 'tcx> UnusedTraitImportVisitor<'a, 'tcx> { } } -impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for UnusedTraitImportVisitor<'a, 'tcx> { +impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CheckVisitor<'a, 'tcx> { fn visit_item(&mut self, item: &hir::Item) { if item.vis == hir::Public || item.span == DUMMY_SP { return; @@ -61,6 +63,21 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for UnusedTraitImportVisitor<'a, 'tcx> { pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let _task = tcx.dep_graph.in_task(DepNode::UnusedTraitCheck); - let mut visitor = UnusedTraitImportVisitor { tcx: tcx }; + + let mut used_trait_imports = DefIdSet(); + for &body_id in tcx.hir.krate().bodies.keys() { + let item_id = tcx.hir.body_owner(body_id); + let item_def_id = tcx.hir.local_def_id(item_id); + + // this will have been written by the main typeck pass + if let Some(imports) = tcx.used_trait_imports.borrow().get(&item_def_id) { + debug!("GatherVisitor: item_def_id={:?} with imports {:#?}", item_def_id, imports); + used_trait_imports.extend(imports); + } else { + debug!("GatherVisitor: item_def_id={:?} with no imports", item_def_id); + } + } + + let mut visitor = CheckVisitor { tcx, used_trait_imports }; tcx.hir.krate().visit_all_item_likes(&mut visitor); } diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 4ed116b88f6d9..f19a59a5d38ae 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -77,6 +77,7 @@ This API is completely unstable and subject to change. #![feature(box_patterns)] #![feature(box_syntax)] #![feature(conservative_impl_trait)] +#![feature(field_init_shorthand)] #![feature(loop_break_value)] #![feature(quote)] #![feature(rustc_diagnostic_macros)] From 4b5613cb1d7db44a7ef5cdd937c45e7d82743021 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 2 Feb 2017 05:26:24 -0500 Subject: [PATCH 7/8] prevent multiple writes to a single entry in a `DepTrackingMap` --- src/librustc/dep_graph/dep_tracking_map.rs | 4 ++-- src/librustc_typeck/collect.rs | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/librustc/dep_graph/dep_tracking_map.rs b/src/librustc/dep_graph/dep_tracking_map.rs index 103f8ef653c1e..ad6c93bf8daac 100644 --- a/src/librustc/dep_graph/dep_tracking_map.rs +++ b/src/librustc/dep_graph/dep_tracking_map.rs @@ -63,8 +63,8 @@ impl DepTrackingMap { pub fn insert(&mut self, k: M::Key, v: M::Value) { self.write(&k); - let _old_value = self.map.insert(k, v); - // assert!(old_value.is_none()); + let old_value = self.map.insert(k, v); + assert!(old_value.is_none()); } pub fn contains_key(&self, k: &M::Key) -> bool { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index cb0a6cccb03d4..266975994ec3a 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -848,9 +848,10 @@ fn convert_trait_item(ccx: &CrateCtxt, trait_item: &hir::TraitItem) { let const_def_id = ccx.tcx.hir.local_def_id(trait_item.id); generics_of_def_id(ccx, const_def_id); let ty = ccx.icx(&trait_predicates).to_ty(&ty); - tcx.item_types.borrow_mut().insert(const_def_id, ty); - convert_associated_const(ccx, TraitContainer(trait_def_id), - trait_item.id, ty); + convert_associated_const(ccx, + TraitContainer(trait_def_id), + trait_item.id, + ty); } hir::TraitItemKind::Type(_, ref opt_ty) => { @@ -884,9 +885,10 @@ fn convert_impl_item(ccx: &CrateCtxt, impl_item: &hir::ImplItem) { let const_def_id = ccx.tcx.hir.local_def_id(impl_item.id); generics_of_def_id(ccx, const_def_id); let ty = ccx.icx(&impl_predicates).to_ty(&ty); - tcx.item_types.borrow_mut().insert(const_def_id, ty); - convert_associated_const(ccx, ImplContainer(impl_def_id), - impl_item.id, ty); + convert_associated_const(ccx, + ImplContainer(impl_def_id), + impl_item.id, + ty); } hir::ImplItemKind::Type(ref ty) => { From 2fc15868a2cd5a24cfe059457e40cbcad6da2d44 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 3 Feb 2017 21:13:59 -0500 Subject: [PATCH 8/8] go back to the older model of coherence collect --- src/librustc/dep_graph/dep_tracking_map.rs | 15 +++++++ src/librustc_typeck/coherence/mod.rs | 47 +++++++++++++--------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/src/librustc/dep_graph/dep_tracking_map.rs b/src/librustc/dep_graph/dep_tracking_map.rs index ad6c93bf8daac..2ffc3951cc94d 100644 --- a/src/librustc/dep_graph/dep_tracking_map.rs +++ b/src/librustc/dep_graph/dep_tracking_map.rs @@ -75,6 +75,21 @@ impl DepTrackingMap { pub fn keys(&self) -> Vec { 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`. + /// + /// 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(&mut self, k: M::Key, elem: E) + where M: DepTrackingMapConfig> + { + self.write(&k); + self.map.entry(k) + .or_insert(Vec::new()) + .push(elem); + } } impl MemoizationMap for RefCell> { diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 4a96a5e960dbe..b6a863fd2ed0d 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -15,8 +15,9 @@ // done by the orphan and overlap modules. Then we build up various // mappings. That mapping code resides here. +use dep_graph::DepTrackingMap; use hir::def_id::DefId; -use rustc::ty::{self, TyCtxt, TypeFoldable}; +use rustc::ty::{self, maps, TyCtxt, TypeFoldable}; use rustc::ty::{Ty, TyBool, TyChar, TyError}; use rustc::ty::{TyParam, TyRawPtr}; use rustc::ty::{TyRef, TyAdt, TyDynamic, TyNever, TyTuple}; @@ -29,7 +30,7 @@ use rustc::dep_graph::DepNode; use rustc::hir::itemlikevisit::ItemLikeVisitor; use rustc::hir::{Item, ItemImpl}; use rustc::hir; -use rustc::util::nodemap::DefIdMap; +use std::cell::RefMut; mod builtin; mod orphan; @@ -38,7 +39,7 @@ mod unsafety; struct CoherenceCollect<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, - inherent_impls: DefIdMap>, + inherent_impls: RefMut<'a, DepTrackingMap>>, } impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceCollect<'a, 'tcx> { @@ -56,6 +57,16 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceCollect<'a, 'tcx> { } impl<'a, 'tcx> CoherenceCollect<'a, 'tcx> { + fn check(tcx: TyCtxt<'a, 'tcx, 'tcx>) { + let inherent_impls = tcx.inherent_impls.borrow_mut(); + let mut this = &mut CoherenceCollect { tcx, inherent_impls }; + + // Check implementations and traits. This populates the tables + // containing the inherent methods and extension methods. It also + // builds up the trait inheritance table. + tcx.visit_all_item_likes_in_krate(DepNode::CoherenceCheckImpl, this); + } + // Returns the def ID of the base type, if there is one. fn get_base_type_def_id(&self, span: Span, ty: Ty<'tcx>) -> Option { match ty.sty { @@ -77,18 +88,6 @@ impl<'a, 'tcx> CoherenceCollect<'a, 'tcx> { } } - fn check(&mut self) { - // Check implementations and traits. This populates the tables - // containing the inherent methods and extension methods. It also - // builds up the trait inheritance table. - self.tcx.visit_all_item_likes_in_krate(DepNode::CoherenceCheckImpl, self); - - // Transfer the inherent impl lists, not that they are known, into the tcx - for (ty_def_id, impl_def_ids) in self.inherent_impls.drain() { - self.tcx.inherent_impls.borrow_mut().insert(ty_def_id, impl_def_ids); - } - } - fn check_implementation(&mut self, item: &Item) { let tcx = self.tcx; let impl_did = tcx.hir.local_def_id(item.id); @@ -127,9 +126,18 @@ impl<'a, 'tcx> CoherenceCollect<'a, 'tcx> { } fn add_inherent_impl(&mut self, base_def_id: DefId, impl_def_id: DefId) { - self.inherent_impls.entry(base_def_id) - .or_insert(vec![]) - .push(impl_def_id); + // Subtle: it'd be better to collect these into a local map + // and then write the vector only once all items are known, + // but that leads to degenerate dep-graphs. The problem is + // that the write of that big vector winds up having reads + // from *all* impls in the krate, since we've lost the + // precision basically. This would be ok in the firewall + // model so once we've made progess towards that we can modify + // the strategy here. In the meantime, using `push` is ok + // because we are doing this as a pre-pass before anyone + // actually reads from `inherent_impls` -- and we know this is + // true beacuse we hold the refcell lock. + self.inherent_impls.push(base_def_id, impl_def_id); } fn add_trait_impl(&self, impl_trait_ref: ty::TraitRef<'tcx>, impl_def_id: DefId) { @@ -169,8 +177,9 @@ fn enforce_trait_manually_implementable(tcx: TyCtxt, sp: Span, trait_def_id: Def } pub fn check_coherence(ccx: &CrateCtxt) { + CoherenceCollect::check(ccx.tcx); + let _task = ccx.tcx.dep_graph.in_task(DepNode::Coherence); - CoherenceCollect { tcx: ccx.tcx, inherent_impls: DefIdMap() }.check(); unsafety::check(ccx.tcx); orphan::check(ccx.tcx); overlap::check(ccx.tcx);