From dd65cb223a3cd1a0fa8d98e9402f8725d606f6b2 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 23 Aug 2016 13:23:58 -0400 Subject: [PATCH 01/14] Add some infrastructure for timing things where time_passes can't be used. --- src/librustc/session/config.rs | 2 + src/librustc/session/mod.rs | 33 +++++++++++++++ src/librustc/util/common.rs | 35 +++++++++++----- src/librustc_driver/driver.rs | 4 ++ src/librustc_trans/back/symbol_names.rs | 53 +++++++++++++------------ 5 files changed, 93 insertions(+), 34 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 562dce6a1b129..79e14212db426 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -908,6 +908,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "dump MIR state at various points in translation"), dump_mir_dir: Option = (None, parse_opt_string, [UNTRACKED], "the directory the MIR is dumped into"), + perf_stats: bool = (false, parse_bool, [UNTRACKED], + "print some performance-related statistics"), } pub fn default_lib_output() -> CrateType { diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 338c656379959..cc115cbeb85bc 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -18,6 +18,7 @@ use session::search_paths::PathKind; use session::config::{DebugInfoLevel, PanicStrategy}; use ty::tls; use util::nodemap::{NodeMap, FnvHashMap}; +use util::common::duration_to_secs_str; use mir::transform as mir_pass; use syntax::ast::{NodeId, Name}; @@ -43,6 +44,7 @@ use std::env; use std::ffi::CString; use std::rc::Rc; use std::fmt; +use std::time::Duration; use libc::c_int; pub mod config; @@ -104,9 +106,23 @@ pub struct Session { incr_comp_session: RefCell, + /// Some measurements that are being gathered during compilation. + pub perf_stats: PerfStats, + next_node_id: Cell, } +pub struct PerfStats { + // The accumulated time needed for computing the SVH of the crate + pub svh_time: Cell, + // The accumulated time spent on computing incr. comp. hashes + pub incr_comp_hashes_time: Cell, + // The number of incr. comp. hash computations performed + pub incr_comp_hashes_count: Cell, + // The accumulated time spent on computing symbol hashes + pub symbol_hash_time: Cell, +} + impl Session { pub fn local_crate_disambiguator(&self) -> token::InternedString { self.crate_disambiguator.borrow().clone() @@ -404,6 +420,17 @@ impl Session { None } } + + pub fn print_perf_stats(&self) { + println!("Total time spent computing SVHs: {}", + duration_to_secs_str(self.perf_stats.svh_time.get())); + println!("Total time spent computing incr. comp. hashes: {}", + duration_to_secs_str(self.perf_stats.incr_comp_hashes_time.get())); + println!("Total number of incr. comp. hashes computed: {}", + self.perf_stats.incr_comp_hashes_count.get()); + println!("Total time spent computing symbol hashes: {}", + duration_to_secs_str(self.perf_stats.symbol_hash_time.get())); + } } pub fn build_session(sopts: config::Options, @@ -520,6 +547,12 @@ pub fn build_session_(sopts: config::Options, available_macros: RefCell::new(HashSet::new()), imported_macro_spans: RefCell::new(HashMap::new()), incr_comp_session: RefCell::new(IncrCompSession::NotInitialized), + perf_stats: PerfStats { + svh_time: Cell::new(Duration::from_secs(0)), + incr_comp_hashes_time: Cell::new(Duration::from_secs(0)), + incr_comp_hashes_count: Cell::new(0), + symbol_hash_time: Cell::new(Duration::from_secs(0)), + } }; init_llvm(&sess); diff --git a/src/librustc/util/common.rs b/src/librustc/util/common.rs index bdfb97549d5d5..78f20b77f3185 100644 --- a/src/librustc/util/common.rs +++ b/src/librustc/util/common.rs @@ -17,7 +17,7 @@ use std::fmt::Debug; use std::hash::{Hash, BuildHasher}; use std::iter::repeat; use std::path::Path; -use std::time::Instant; +use std::time::{Duration, Instant}; use hir; use hir::intravisit; @@ -47,12 +47,6 @@ pub fn time(do_it: bool, what: &str, f: F) -> T where let rv = f(); let dur = start.elapsed(); - // Hack up our own formatting for the duration to make it easier for scripts - // to parse (always use the same number of decimal places and the same unit). - const NANOS_PER_SEC: f64 = 1_000_000_000.0; - let secs = dur.as_secs() as f64; - let secs = secs + dur.subsec_nanos() as f64 / NANOS_PER_SEC; - let mem_string = match get_resident() { Some(n) => { let mb = n as f64 / 1_000_000.0; @@ -60,14 +54,37 @@ pub fn time(do_it: bool, what: &str, f: F) -> T where } None => "".to_owned(), }; - println!("{}time: {:.3}{}\t{}", repeat(" ").take(old).collect::(), - secs, mem_string, what); + println!("{}time: {}{}\t{}", + repeat(" ").take(old).collect::(), + duration_to_secs_str(dur), + mem_string, + what); DEPTH.with(|slot| slot.set(old)); rv } +// Hack up our own formatting for the duration to make it easier for scripts +// to parse (always use the same number of decimal places and the same unit). +pub fn duration_to_secs_str(dur: Duration) -> String { + const NANOS_PER_SEC: f64 = 1_000_000_000.0; + let secs = dur.as_secs() as f64 + + dur.subsec_nanos() as f64 / NANOS_PER_SEC; + + format!("{:.3}", secs) +} + +pub fn record_time(accu: &Cell, f: F) -> T where + F: FnOnce() -> T, +{ + let start = Instant::now(); + let rv = f(); + let duration = start.elapsed(); + accu.set(duration + accu.get()); + rv +} + // Like std::macros::try!, but for Option<>. macro_rules! option_try( ($e:expr) => (match $e { Some(e) => e, None => return None }) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 94092be4922b5..e8137430a0646 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -233,6 +233,10 @@ pub fn compile_input(sess: &Session, // any more, we can finalize it (which involves renaming it) rustc_incremental::finalize_session_directory(sess, trans.link.crate_hash); + if sess.opts.debugging_opts.perf_stats { + sess.print_perf_stats(); + } + controller_entry_point!(compilation_done, sess, CompileState::state_when_compilation_done(input, sess, outdir, output), diff --git a/src/librustc_trans/back/symbol_names.rs b/src/librustc_trans/back/symbol_names.rs index 9b02cbe6721f3..00f29b7412ed2 100644 --- a/src/librustc_trans/back/symbol_names.rs +++ b/src/librustc_trans/back/symbol_names.rs @@ -108,6 +108,7 @@ use rustc::ty::{Ty, TyCtxt, TypeFoldable}; use rustc::ty::item_path::{self, ItemPathBuffer, RootMode}; use rustc::ty::subst::Substs; use rustc::hir::map::definitions::{DefPath, DefPathData}; +use rustc::util::common::record_time; use syntax::attr; use syntax::parse::token::{self, InternedString}; @@ -138,33 +139,35 @@ fn get_symbol_hash<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, let tcx = scx.tcx(); - let mut hash_state = scx.symbol_hasher().borrow_mut(); - - hash_state.reset(); - - // the main symbol name is not necessarily unique; hash in the - // compiler's internal def-path, guaranteeing each symbol has a - // truly unique path - hash_state.input_str(&def_path.to_string(tcx)); - - // Include the main item-type. Note that, in this case, the - // assertions about `needs_subst` may not hold, but this item-type - // ought to be the same for every reference anyway. - assert!(!item_type.has_erasable_regions()); - let encoded_item_type = tcx.sess.cstore.encode_type(tcx, item_type, def_id_to_string); - hash_state.input(&encoded_item_type[..]); - - // also include any type parameters (for generic items) - if let Some(substs) = substs { - for t in substs.types() { - assert!(!t.has_erasable_regions()); - assert!(!t.needs_subst()); - let encoded_type = tcx.sess.cstore.encode_type(tcx, t, def_id_to_string); - hash_state.input(&encoded_type[..]); + return record_time(&tcx.sess.perf_stats.symbol_hash_time, || { + let mut hash_state = scx.symbol_hasher().borrow_mut(); + + hash_state.reset(); + + // the main symbol name is not necessarily unique; hash in the + // compiler's internal def-path, guaranteeing each symbol has a + // truly unique path + hash_state.input_str(&def_path.to_string(tcx)); + + // Include the main item-type. Note that, in this case, the + // assertions about `needs_subst` may not hold, but this item-type + // ought to be the same for every reference anyway. + assert!(!item_type.has_erasable_regions()); + let encoded_item_type = tcx.sess.cstore.encode_type(tcx, item_type, def_id_to_string); + hash_state.input(&encoded_item_type[..]); + + // also include any type parameters (for generic items) + if let Some(substs) = substs { + for t in substs.types() { + assert!(!t.has_erasable_regions()); + assert!(!t.needs_subst()); + let encoded_type = tcx.sess.cstore.encode_type(tcx, t, def_id_to_string); + hash_state.input(&encoded_type[..]); + } } - } - return format!("h{}", truncated_hash_result(&mut *hash_state)); + format!("h{}", truncated_hash_result(&mut *hash_state)) + }); fn truncated_hash_result(symbol_hasher: &mut Sha256) -> String { let output = symbol_hasher.result_bytes(); From 8e4f4810dcc7cc21aec13d421d211a94f29e413f Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 23 Aug 2016 13:54:02 -0400 Subject: [PATCH 02/14] Fill some holes in SVH/ICH computation, making it more strict. --- src/librustc_incremental/calculate_svh/mod.rs | 34 +- .../calculate_svh/svh_visitor.rs | 321 +++++++++++++++--- src/librustc_incremental/lib.rs | 1 + 3 files changed, 291 insertions(+), 65 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/mod.rs b/src/librustc_incremental/calculate_svh/mod.rs index b14c20ae8d46e..6ad93d8f4733c 100644 --- a/src/librustc_incremental/calculate_svh/mod.rs +++ b/src/librustc_incremental/calculate_svh/mod.rs @@ -35,6 +35,8 @@ use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId}; use rustc::hir::intravisit as visit; use rustc::ty::TyCtxt; use rustc_data_structures::fnv::FnvHashMap; +use rustc::util::common::record_time; +use rustc::session::config::DebugInfoLevel::NoDebugInfo; use self::def_path_hash::DefPathHashes; use self::svh_visitor::StrictVersionHashVisitor; @@ -48,12 +50,19 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> IncrementalHashesMap { let _ignore = tcx.dep_graph.in_ignore(); let krate = tcx.map.krate(); - let mut visitor = HashItemsVisitor { tcx: tcx, - hashes: FnvHashMap(), - def_path_hashes: DefPathHashes::new(tcx) }; - visitor.calculate_def_id(DefId::local(CRATE_DEF_INDEX), |v| visit::walk_crate(v, krate)); - krate.visit_all_items(&mut visitor); - visitor.compute_crate_hash(); + let hash_spans = tcx.sess.opts.debuginfo != NoDebugInfo; + let mut visitor = HashItemsVisitor { + tcx: tcx, + hashes: FnvHashMap(), + def_path_hashes: DefPathHashes::new(tcx), + hash_spans: hash_spans + }; + record_time(&tcx.sess.perf_stats.incr_comp_hashes_time, || { + visitor.calculate_def_id(DefId::local(CRATE_DEF_INDEX), + |v| visit::walk_crate(v, krate)); + krate.visit_all_items(&mut visitor); + }); + record_time(&tcx.sess.perf_stats.svh_time, || visitor.compute_crate_hash()); visitor.hashes } @@ -61,6 +70,7 @@ struct HashItemsVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, def_path_hashes: DefPathHashes<'a, 'tcx>, hashes: IncrementalHashesMap, + hash_spans: bool, } impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> { @@ -81,7 +91,8 @@ impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> { let mut state = SipHasher::new(); walk_op(&mut StrictVersionHashVisitor::new(&mut state, self.tcx, - &mut self.def_path_hashes)); + &mut self.def_path_hashes, + self.hash_spans)); let item_hash = state.finish(); self.hashes.insert(DepNode::Hir(def_id), item_hash); debug!("calculate_item_hash: def_id={:?} hash={:?}", def_id, item_hash); @@ -117,9 +128,12 @@ impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> { item_hashes.hash(&mut crate_state); } - for attr in &krate.attrs { - debug!("krate attr {:?}", attr); - attr.meta().hash(&mut crate_state); + { + let mut visitor = StrictVersionHashVisitor::new(&mut crate_state, + self.tcx, + &mut self.def_path_hashes, + self.hash_spans); + visitor.hash_attributes(&krate.attrs); } let crate_hash = crate_state.finish(); diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index c1158dc2d5fe9..de286d68fe980 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -13,10 +13,9 @@ // hash computation, but for many kinds of items the order of // declaration should be irrelevant to the ABI. -pub use self::SawExprComponent::*; -pub use self::SawStmtComponent::*; +use self::SawExprComponent::*; use self::SawAbiComponent::*; -use syntax::ast::{self, Name, NodeId}; +use syntax::ast::{self, Name, NodeId, Attribute}; use syntax::parse::token; use syntax_pos::Span; use rustc::hir; @@ -24,7 +23,6 @@ use rustc::hir::*; use rustc::hir::def::{Def, PathResolution}; use rustc::hir::def_id::DefId; use rustc::hir::intravisit as visit; -use rustc::hir::intravisit::{Visitor, FnKind}; use rustc::ty::TyCtxt; use std::hash::{Hash, SipHasher}; @@ -34,22 +32,41 @@ use super::def_path_hash::DefPathHashes; pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> { pub tcx: TyCtxt<'hash, 'tcx, 'tcx>, pub st: &'a mut SipHasher, - // collect a deterministic hash of def-ids that we have seen def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>, + hash_spans: bool, } impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { pub fn new(st: &'a mut SipHasher, tcx: TyCtxt<'hash, 'tcx, 'tcx>, - def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>) + def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>, + hash_spans: bool) -> Self { - StrictVersionHashVisitor { st: st, tcx: tcx, def_path_hashes: def_path_hashes } + StrictVersionHashVisitor { + st: st, + tcx: tcx, + def_path_hashes: def_path_hashes, + hash_spans: hash_spans, + } } fn compute_def_id_hash(&mut self, def_id: DefId) -> u64 { self.def_path_hashes.hash(def_id) } + + #[inline] + fn hash_span(&mut self, span: Span) { + if self.hash_spans { + let _ = span; + } + } + + fn hash_discriminant(&mut self, v: &T) { + unsafe { + ::std::intrinsics::discriminant_value(&v).hash(self.st); + } + } } // To off-load the bulk of the hash-computation on #[derive(Hash)], @@ -80,26 +97,35 @@ enum SawAbiComponent<'a> { SawIdent(token::InternedString), SawStructDef(token::InternedString), - SawLifetime(token::InternedString), - SawLifetimeDef(token::InternedString), + SawLifetime, + SawLifetimeDef(usize), SawMod, SawForeignItem, SawItem, SawTy, SawGenerics, - SawFn, SawTraitItem, SawImplItem, SawStructField, SawVariant, - SawPath, + SawPath(bool), + SawPathSegment, + SawPathParameters, + SawPathListItem, SawBlock, SawPat, SawLocal, SawArm, SawExpr(SawExprComponent<'a>), - SawStmt(SawStmtComponent), + SawStmt, + SawVis, + SawWherePredicate, + SawTyParamBound, + SawPolyTraitRef, + SawAssocTypeBinding, + SawAttribute(ast::AttrStyle, bool), + SawMacroDef, } /// SawExprComponent carries all of the information that we want @@ -117,7 +143,7 @@ enum SawAbiComponent<'a> { /// guarantee of collision-freedom, hash collisions are just /// (hopefully) unlikely.) #[derive(Hash)] -pub enum SawExprComponent<'a> { +enum SawExprComponent<'a> { SawExprLoop(Option), SawExprField(token::InternedString), @@ -185,31 +211,39 @@ fn saw_expr<'a>(node: &'a Expr_) -> SawExprComponent<'a> { } } -/// SawStmtComponent is analogous to SawExprComponent, but for statements. -#[derive(Hash)] -pub enum SawStmtComponent { - SawStmtExpr, - SawStmtSemi, +macro_rules! hash_attrs { + ($visitor:expr, $attrs:expr) => ({ + let attrs = $attrs; + if attrs.len() > 0 { + $visitor.hash_attributes(attrs); + } + }) } -impl<'a, 'hash, 'tcx> Visitor<'tcx> for StrictVersionHashVisitor<'a, 'hash, 'tcx> { +impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'hash, 'tcx> { fn visit_nested_item(&mut self, _: ItemId) { // Each item is hashed independently; ignore nested items. } - fn visit_variant_data(&mut self, s: &'tcx VariantData, name: Name, - g: &'tcx Generics, _: NodeId, _: Span) { + fn visit_variant_data(&mut self, + s: &'tcx VariantData, + name: Name, + _: &'tcx Generics, + _: NodeId, + span: Span) { debug!("visit_variant_data: st={:?}", self.st); SawStructDef(name.as_str()).hash(self.st); - visit::walk_generics(self, g); - visit::walk_struct_def(self, s) + self.hash_span(span); + visit::walk_struct_def(self, s); } - fn visit_variant(&mut self, v: &'tcx Variant, g: &'tcx Generics, item_id: NodeId) { + fn visit_variant(&mut self, + v: &'tcx Variant, + g: &'tcx Generics, + item_id: NodeId) { debug!("visit_variant: st={:?}", self.st); SawVariant.hash(self.st); - // walk_variant does not call walk_generics, so do it here. - visit::walk_generics(self, g); + hash_attrs!(self, &v.node.attrs); visit::walk_variant(self, v, g, item_id) } @@ -227,19 +261,22 @@ impl<'a, 'hash, 'tcx> Visitor<'tcx> for StrictVersionHashVisitor<'a, 'hash, 'tcx // (If you edit a method such that it deviates from the // pattern, please move that method up above this comment.) - fn visit_name(&mut self, _: Span, name: Name) { + fn visit_name(&mut self, span: Span, name: Name) { debug!("visit_name: st={:?}", self.st); SawIdent(name.as_str()).hash(self.st); + self.hash_span(span); } fn visit_lifetime(&mut self, l: &'tcx Lifetime) { debug!("visit_lifetime: st={:?}", self.st); - SawLifetime(l.name.as_str()).hash(self.st); + SawLifetime.hash(self.st); + visit::walk_lifetime(self, l); } fn visit_lifetime_def(&mut self, l: &'tcx LifetimeDef) { debug!("visit_lifetime_def: st={:?}", self.st); - SawLifetimeDef(l.lifetime.name.as_str()).hash(self.st); + SawLifetimeDef(l.bounds.len()).hash(self.st); + visit::walk_lifetime_def(self, l); } // We do recursively walk the bodies of functions/methods @@ -249,7 +286,12 @@ impl<'a, 'hash, 'tcx> Visitor<'tcx> for StrictVersionHashVisitor<'a, 'hash, 'tcx // crates to be recompiled. fn visit_expr(&mut self, ex: &'tcx Expr) { debug!("visit_expr: st={:?}", self.st); - SawExpr(saw_expr(&ex.node)).hash(self.st); visit::walk_expr(self, ex) + SawExpr(saw_expr(&ex.node)).hash(self.st); + // No need to explicitly hash the discriminant here, since we are + // implicitly hashing the discriminant of SawExprComponent. + self.hash_span(ex.span); + hash_attrs!(self, &ex.attrs); + visit::walk_expr(self, ex) } fn visit_stmt(&mut self, s: &'tcx Stmt) { @@ -263,8 +305,16 @@ impl<'a, 'hash, 'tcx> Visitor<'tcx> for StrictVersionHashVisitor<'a, 'hash, 'tcx // rules). match s.node { StmtDecl(..) => (), - StmtExpr(..) => SawStmt(SawStmtExpr).hash(self.st), - StmtSemi(..) => SawStmt(SawStmtSemi).hash(self.st), + StmtExpr(..) => { + SawStmt.hash(self.st); + self.hash_discriminant(&s.node); + self.hash_span(s.span); + } + StmtSemi(..) => { + SawStmt.hash(self.st); + self.hash_discriminant(&s.node); + self.hash_span(s.span); + } } visit::walk_stmt(self, s) @@ -277,17 +327,21 @@ impl<'a, 'hash, 'tcx> Visitor<'tcx> for StrictVersionHashVisitor<'a, 'hash, 'tcx // perhaps reachability) somewhere here, so foreign items // that do not leak into downstream crates would not be // part of the ABI. - SawForeignItem.hash(self.st); visit::walk_foreign_item(self, i) + SawForeignItem.hash(self.st); + self.hash_span(i.span); + hash_attrs!(self, &i.attrs); + visit::walk_foreign_item(self, i) } fn visit_item(&mut self, i: &'tcx Item) { debug!("visit_item: {:?} st={:?}", i, self.st); - // FIXME (#14132) ideally would incorporate reachability - // analysis somewhere here, so items that never leak into - // downstream crates (e.g. via monomorphisation or - // inlining) would not be part of the ABI. - SawItem.hash(self.st); visit::walk_item(self, i) + SawItem.hash(self.st); + // Hash the value of the discriminant of the Item variant. + self.hash_discriminant(&i.node); + self.hash_span(i.span); + hash_attrs!(self, &i.attrs); + visit::walk_item(self, i) } fn visit_mod(&mut self, m: &'tcx Mod, _s: Span, n: NodeId) { @@ -297,63 +351,159 @@ impl<'a, 'hash, 'tcx> Visitor<'tcx> for StrictVersionHashVisitor<'a, 'hash, 'tcx fn visit_ty(&mut self, t: &'tcx Ty) { debug!("visit_ty: st={:?}", self.st); - SawTy.hash(self.st); visit::walk_ty(self, t) + SawTy.hash(self.st); + self.hash_span(t.span); + visit::walk_ty(self, t) } fn visit_generics(&mut self, g: &'tcx Generics) { debug!("visit_generics: st={:?}", self.st); - SawGenerics.hash(self.st); visit::walk_generics(self, g) - } - - fn visit_fn(&mut self, fk: FnKind<'tcx>, fd: &'tcx FnDecl, - b: &'tcx Block, s: Span, n: NodeId) { - debug!("visit_fn: st={:?}", self.st); - SawFn.hash(self.st); visit::walk_fn(self, fk, fd, b, s, n) + SawGenerics.hash(self.st); + // FIXME: nested stuff + visit::walk_generics(self, g) } fn visit_trait_item(&mut self, ti: &'tcx TraitItem) { debug!("visit_trait_item: st={:?}", self.st); - SawTraitItem.hash(self.st); visit::walk_trait_item(self, ti) + SawTraitItem.hash(self.st); + self.hash_discriminant(&ti.node); + self.hash_span(ti.span); + hash_attrs!(self, &ti.attrs); + visit::walk_trait_item(self, ti) } fn visit_impl_item(&mut self, ii: &'tcx ImplItem) { debug!("visit_impl_item: st={:?}", self.st); - SawImplItem.hash(self.st); visit::walk_impl_item(self, ii) + SawImplItem.hash(self.st); + self.hash_discriminant(&ii.node); + self.hash_span(ii.span); + hash_attrs!(self, &ii.attrs); + visit::walk_impl_item(self, ii) } fn visit_struct_field(&mut self, s: &'tcx StructField) { debug!("visit_struct_field: st={:?}", self.st); - SawStructField.hash(self.st); visit::walk_struct_field(self, s) + SawStructField.hash(self.st); + self.hash_span(s.span); + hash_attrs!(self, &s.attrs); + visit::walk_struct_field(self, s) } fn visit_path(&mut self, path: &'tcx Path, _: ast::NodeId) { debug!("visit_path: st={:?}", self.st); - SawPath.hash(self.st); visit::walk_path(self, path) + SawPath(path.global).hash(self.st); + self.hash_span(path.span); + visit::walk_path(self, path) } fn visit_block(&mut self, b: &'tcx Block) { debug!("visit_block: st={:?}", self.st); - SawBlock.hash(self.st); visit::walk_block(self, b) + SawBlock.hash(self.st); + self.hash_span(b.span); + visit::walk_block(self, b) } fn visit_pat(&mut self, p: &'tcx Pat) { debug!("visit_pat: st={:?}", self.st); - SawPat.hash(self.st); visit::walk_pat(self, p) + SawPat.hash(self.st); + self.hash_discriminant(&p.node); + self.hash_span(p.span); + visit::walk_pat(self, p) } fn visit_local(&mut self, l: &'tcx Local) { debug!("visit_local: st={:?}", self.st); - SawLocal.hash(self.st); visit::walk_local(self, l) + SawLocal.hash(self.st); + hash_attrs!(self, &l.attrs); + visit::walk_local(self, l) + // No need to hash span, we are hashing all component spans } fn visit_arm(&mut self, a: &'tcx Arm) { debug!("visit_arm: st={:?}", self.st); - SawArm.hash(self.st); visit::walk_arm(self, a) + SawArm.hash(self.st); + hash_attrs!(self, &a.attrs); + visit::walk_arm(self, a) } fn visit_id(&mut self, id: NodeId) { debug!("visit_id: id={} st={:?}", id, self.st); - self.hash_resolve(id); + self.hash_resolve(id) + } + + fn visit_vis(&mut self, v: &'tcx Visibility) { + debug!("visit_vis: st={:?}", self.st); + SawVis.hash(self.st); + self.hash_discriminant(&v); + visit::walk_vis(self, v) + } + + fn visit_where_predicate(&mut self, predicate: &'tcx WherePredicate) { + debug!("visit_where_predicate: st={:?}", self.st); + SawWherePredicate.hash(self.st); + self.hash_discriminant(predicate); + // Ignoring span. Any important nested components should be visited. + visit::walk_where_predicate(self, predicate) + } + + fn visit_ty_param_bound(&mut self, bounds: &'tcx TyParamBound) { + debug!("visit_ty_param_bound: st={:?}", self.st); + SawTyParamBound.hash(self.st); + self.hash_discriminant(bounds); + // The TraitBoundModifier in TraitTyParamBound will be hash in + // visit_poly_trait_ref() + visit::walk_ty_param_bound(self, bounds) + } + + fn visit_poly_trait_ref(&mut self, t: &'tcx PolyTraitRef, m: &'tcx TraitBoundModifier) { + debug!("visit_poly_trait_ref: st={:?}", self.st); + SawPolyTraitRef.hash(self.st); + m.hash(self.st); + visit::walk_poly_trait_ref(self, t, m) + } + + fn visit_path_list_item(&mut self, prefix: &'tcx Path, item: &'tcx PathListItem) { + debug!("visit_path_list_item: st={:?}", self.st); + SawPathListItem.hash(self.st); + self.hash_discriminant(&item.node); + self.hash_span(item.span); + visit::walk_path_list_item(self, prefix, item) + } + + fn visit_path_segment(&mut self, path_span: Span, path_segment: &'tcx PathSegment) { + debug!("visit_path_segment: st={:?}", self.st); + SawPathSegment.hash(self.st); + visit::walk_path_segment(self, path_span, path_segment) + } + + fn visit_path_parameters(&mut self, path_span: Span, path_parameters: &'tcx PathParameters) { + debug!("visit_path_parameters: st={:?}", self.st); + SawPathParameters.hash(self.st); + self.hash_discriminant(path_parameters); + visit::walk_path_parameters(self, path_span, path_parameters) + } + + fn visit_assoc_type_binding(&mut self, type_binding: &'tcx TypeBinding) { + debug!("visit_assoc_type_binding: st={:?}", self.st); + SawAssocTypeBinding.hash(self.st); + self.hash_span(type_binding.span); + visit::walk_assoc_type_binding(self, type_binding) + } + + fn visit_attribute(&mut self, _: &Attribute) { + // We explicitly do not use this method, since doing that would + // implicitly impose an order on the attributes being hashed, while we + // explicitly don't want their order to matter + } + + fn visit_macro_def(&mut self, macro_def: &'tcx MacroDef) { + debug!("visit_macro_def: st={:?}", self.st); + if macro_def.export { + SawMacroDef.hash(self.st); + hash_attrs!(self, ¯o_def.attrs); + visit::walk_macro_def(self, macro_def) + // FIXME: We should hash the body of the macro too. + } } } @@ -450,4 +600,65 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { } } } + + fn hash_meta_item(&mut self, meta_item: &ast::MetaItem) { + // ignoring span information, it doesn't matter here + match meta_item.node { + ast::MetaItemKind::Word(ref s) => { + "Word".hash(self.st); + s.len().hash(self.st); + s.hash(self.st); + } + ast::MetaItemKind::NameValue(ref s, ref lit) => { + "NameValue".hash(self.st); + s.len().hash(self.st); + s.hash(self.st); + lit.node.hash(self.st); + } + ast::MetaItemKind::List(ref s, ref items) => { + "List".hash(self.st); + s.len().hash(self.st); + s.hash(self.st); + // Sort subitems so the hash does not depend on their order + let indices = self.indices_sorted_by(&items, |p| { + meta_item_sort_key(&*p) + }); + items.len().hash(self.st); + for (index, &item_index) in indices.iter().enumerate() { + index.hash(self.st); + self.hash_meta_item(&items[item_index]); + } + } + } + } + + pub fn hash_attributes(&mut self, attributes: &[Attribute]) { + let indices = self.indices_sorted_by(attributes, |attr| { + meta_item_sort_key(&attr.node.value) + }); + + for i in indices { + let attr = &attributes[i].node; + SawAttribute(attr.style, attr.is_sugared_doc).hash(self.st); + self.hash_meta_item(&*attr.value); + } + } + + fn indices_sorted_by(&mut self, items: &[T], get_key: F) -> Vec + where K: Ord, + F: Fn(&T) -> K + { + let mut indices = Vec::with_capacity(items.len()); + indices.extend(0 .. items.len()); + indices.sort_by_key(|index| get_key(&items[*index])); + indices + } +} + +fn meta_item_sort_key(item: &ast::MetaItem) -> token::InternedString { + match item.node { + ast::MetaItemKind::Word(ref s) | + ast::MetaItemKind::NameValue(ref s, _) | + ast::MetaItemKind::List(ref s, _) => s.clone() + } } diff --git a/src/librustc_incremental/lib.rs b/src/librustc_incremental/lib.rs index 511ba8ec19cc7..feacfdc96731f 100644 --- a/src/librustc_incremental/lib.rs +++ b/src/librustc_incremental/lib.rs @@ -23,6 +23,7 @@ #![feature(rustc_private)] #![feature(staged_api)] #![feature(rand)] +#![feature(core_intrinsics)] extern crate graphviz; extern crate rbml; From e355ec1c6a24f8a597a08809b9dad394498dc3dd Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 24 Aug 2016 17:06:31 -0400 Subject: [PATCH 03/14] incr.comp.: Add stable hashing of HIR spans to ICH. --- .../calculate_svh/svh_visitor.rs | 207 +++++++++++++----- src/libsyntax/codemap.rs | 24 +- src/libsyntax_pos/lib.rs | 64 ++++++ 3 files changed, 226 insertions(+), 69 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index de286d68fe980..554a0e0a97cd3 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -17,24 +17,99 @@ use self::SawExprComponent::*; use self::SawAbiComponent::*; use syntax::ast::{self, Name, NodeId, Attribute}; use syntax::parse::token; -use syntax_pos::Span; +use syntax::codemap::CodeMap; +use syntax_pos::{Span, NO_EXPANSION, COMMAND_LINE_EXPN, BytePos, FileMap}; use rustc::hir; use rustc::hir::*; use rustc::hir::def::{Def, PathResolution}; use rustc::hir::def_id::DefId; use rustc::hir::intravisit as visit; use rustc::ty::TyCtxt; - +use std::rc::Rc; use std::hash::{Hash, SipHasher}; use super::def_path_hash::DefPathHashes; +const IGNORED_ATTRIBUTES: &'static [&'static str] = &["cfg", + "rustc_clean", + "rustc_dirty"]; + pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> { pub tcx: TyCtxt<'hash, 'tcx, 'tcx>, pub st: &'a mut SipHasher, // collect a deterministic hash of def-ids that we have seen def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>, hash_spans: bool, + codemap: CachedCodemapView<'tcx>, +} + +struct CachedCodemapView<'tcx> { + codemap: &'tcx CodeMap, + // Format: (line number, line-start, line_end, file) + line_cache: [(usize, BytePos, BytePos, Rc); 4], + eviction_index: usize, +} + +impl<'tcx> CachedCodemapView<'tcx> { + fn new<'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CachedCodemapView<'tcx> { + let codemap = tcx.sess.codemap(); + let first_file = codemap.files.borrow()[0].clone(); + + CachedCodemapView { + codemap: codemap, + line_cache: [(0, BytePos(0), BytePos(0), first_file.clone()), + (0, BytePos(0), BytePos(0), first_file.clone()), + (0, BytePos(0), BytePos(0), first_file.clone()), + (0, BytePos(0), BytePos(0), first_file.clone())], + eviction_index: 0, + } + } + + fn byte_pos_to_line_and_col(&mut self, + pos: BytePos) + -> (Rc, usize, BytePos) { + // Check if the position is in one of the cached lines + for &(line, start, end, ref file) in self.line_cache.iter() { + if pos >= start && pos < end { + return (file.clone(), line, pos - start); + } + } + + // Check whether we have a cached line in the correct file, so we can + // overwrite it without having to look up the file again. + for &mut (ref mut line, + ref mut start, + ref mut end, + ref file) in self.line_cache.iter_mut() { + if pos >= file.start_pos && pos < file.end_pos { + let line_index = file.lookup_line(pos).unwrap(); + let (line_start, line_end) = file.line_bounds(line_index); + + // Update the cache entry in place + *line = line_index + 1; + *start = line_start; + *end = line_end; + + return (file.clone(), line_index + 1, pos - line_start); + } + } + + // No cache hit ... + let file_index = self.codemap.lookup_filemap_idx(pos); + let file = self.codemap.files.borrow()[file_index].clone(); + let line_index = file.lookup_line(pos).unwrap(); + let (line_start, line_end) = file.line_bounds(line_index); + + // Just overwrite some cache entry. If we got this for, all of them + // point to the wrong file. + self.line_cache[self.eviction_index] = (line_index + 1, + line_start, + line_end, + file.clone()); + self.eviction_index = (self.eviction_index + 1) % self.line_cache.len(); + + return (file, line_index + 1, pos - line_start); + } } impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { @@ -48,6 +123,7 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { tcx: tcx, def_path_hashes: def_path_hashes, hash_spans: hash_spans, + codemap: CachedCodemapView::new(tcx), } } @@ -55,10 +131,46 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { self.def_path_hashes.hash(def_id) } - #[inline] + // Hash a span in a stable way. If we would just hash the spans BytePos + // fields that would be similar hashing pointers since those or just offsets + // into the CodeMap. Instead, we hash the (file name, line, column) triple, + // which stays the same even if the containing FileMap has moved within the + // CodeMap. + // Also note that we are hashing byte offsets for the column, not unicode + // codepoint offsets. For the purpose of the hash that's sufficient. fn hash_span(&mut self, span: Span) { - if self.hash_spans { - let _ = span; + debug_assert!(self.hash_spans); + debug!("hash_span: st={:?}", self.st); + + // If this is not an empty or invalid span, we want to hash the last + // position that belongs to it, as opposed to hashing the first + // position past it. + let span_hi = if span.hi > span.lo { + // We might end up in the middle of a multibyte character here, + // but that's OK, since we are not trying to decode anything at + // this position. + span.hi - BytePos(1) + } else { + span.hi + }; + + let (file1, line1, col1) = self.codemap.byte_pos_to_line_and_col(span.lo); + let (file2, line2, col2) = self.codemap.byte_pos_to_line_and_col(span_hi); + + let expansion_kind = match span.expn_id { + NO_EXPANSION => SawSpanExpnKind::NoExpansion, + COMMAND_LINE_EXPN => SawSpanExpnKind::CommandLine, + _ => SawSpanExpnKind::SomeExpansion, + }; + + expansion_kind.hash(self.st); + + SawSpan(&file1.name[..], line1, col1, + &file2.name[..], line2, col2, + expansion_kind).hash(self.st); + + if expansion_kind == SawSpanExpnKind::SomeExpansion { + self.hash_span(self.codemap.codemap.source_callsite(span)); } } @@ -126,6 +238,7 @@ enum SawAbiComponent<'a> { SawAssocTypeBinding, SawAttribute(ast::AttrStyle, bool), SawMacroDef, + SawSpan(&'a str, usize, BytePos, &'a str, usize, BytePos, SawSpanExpnKind), } /// SawExprComponent carries all of the information that we want @@ -211,6 +324,13 @@ fn saw_expr<'a>(node: &'a Expr_) -> SawExprComponent<'a> { } } +#[derive(Clone, Copy, Hash, Eq, PartialEq)] +enum SawSpanExpnKind { + NoExpansion, + CommandLine, + SomeExpansion, +} + macro_rules! hash_attrs { ($visitor:expr, $attrs:expr) => ({ let attrs = $attrs; @@ -220,6 +340,14 @@ macro_rules! hash_attrs { }) } +macro_rules! hash_span { + ($visitor:expr, $span:expr) => ({ + if $visitor.hash_spans { + $visitor.hash_span($span); + } + }) +} + impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'hash, 'tcx> { fn visit_nested_item(&mut self, _: ItemId) { // Each item is hashed independently; ignore nested items. @@ -233,7 +361,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has span: Span) { debug!("visit_variant_data: st={:?}", self.st); SawStructDef(name.as_str()).hash(self.st); - self.hash_span(span); + hash_span!(self, span); visit::walk_struct_def(self, s); } @@ -247,24 +375,10 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has visit::walk_variant(self, v, g, item_id) } - // All of the remaining methods just record (in the hash - // SipHasher) that the visitor saw that particular variant - // (with its payload), and continue walking as the default - // visitor would. - // - // Some of the implementations have some notes as to how one - // might try to make their SVH computation less discerning - // (e.g. by incorporating reachability analysis). But - // currently all of their implementations are uniform and - // uninteresting. - // - // (If you edit a method such that it deviates from the - // pattern, please move that method up above this comment.) - fn visit_name(&mut self, span: Span, name: Name) { debug!("visit_name: st={:?}", self.st); SawIdent(name.as_str()).hash(self.st); - self.hash_span(span); + hash_span!(self, span); } fn visit_lifetime(&mut self, l: &'tcx Lifetime) { @@ -279,17 +393,12 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has visit::walk_lifetime_def(self, l); } - // We do recursively walk the bodies of functions/methods - // (rather than omitting their bodies from the hash) since - // monomorphization and cross-crate inlining generally implies - // that a change to a crate body will require downstream - // crates to be recompiled. fn visit_expr(&mut self, ex: &'tcx Expr) { debug!("visit_expr: st={:?}", self.st); SawExpr(saw_expr(&ex.node)).hash(self.st); // No need to explicitly hash the discriminant here, since we are // implicitly hashing the discriminant of SawExprComponent. - self.hash_span(ex.span); + hash_span!(self, ex.span); hash_attrs!(self, &ex.attrs); visit::walk_expr(self, ex) } @@ -308,12 +417,12 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has StmtExpr(..) => { SawStmt.hash(self.st); self.hash_discriminant(&s.node); - self.hash_span(s.span); + hash_span!(self, s.span); } StmtSemi(..) => { SawStmt.hash(self.st); self.hash_discriminant(&s.node); - self.hash_span(s.span); + hash_span!(self, s.span); } } @@ -323,12 +432,8 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has fn visit_foreign_item(&mut self, i: &'tcx ForeignItem) { debug!("visit_foreign_item: st={:?}", self.st); - // FIXME (#14132) ideally we would incorporate privacy (or - // perhaps reachability) somewhere here, so foreign items - // that do not leak into downstream crates would not be - // part of the ABI. SawForeignItem.hash(self.st); - self.hash_span(i.span); + hash_span!(self, i.span); hash_attrs!(self, &i.attrs); visit::walk_foreign_item(self, i) } @@ -339,7 +444,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has SawItem.hash(self.st); // Hash the value of the discriminant of the Item variant. self.hash_discriminant(&i.node); - self.hash_span(i.span); + hash_span!(self, i.span); hash_attrs!(self, &i.attrs); visit::walk_item(self, i) } @@ -352,7 +457,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has fn visit_ty(&mut self, t: &'tcx Ty) { debug!("visit_ty: st={:?}", self.st); SawTy.hash(self.st); - self.hash_span(t.span); + hash_span!(self, t.span); visit::walk_ty(self, t) } @@ -367,7 +472,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has debug!("visit_trait_item: st={:?}", self.st); SawTraitItem.hash(self.st); self.hash_discriminant(&ti.node); - self.hash_span(ti.span); + hash_span!(self, ti.span); hash_attrs!(self, &ti.attrs); visit::walk_trait_item(self, ti) } @@ -376,7 +481,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has debug!("visit_impl_item: st={:?}", self.st); SawImplItem.hash(self.st); self.hash_discriminant(&ii.node); - self.hash_span(ii.span); + hash_span!(self, ii.span); hash_attrs!(self, &ii.attrs); visit::walk_impl_item(self, ii) } @@ -384,7 +489,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has fn visit_struct_field(&mut self, s: &'tcx StructField) { debug!("visit_struct_field: st={:?}", self.st); SawStructField.hash(self.st); - self.hash_span(s.span); + hash_span!(self, s.span); hash_attrs!(self, &s.attrs); visit::walk_struct_field(self, s) } @@ -392,14 +497,14 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has fn visit_path(&mut self, path: &'tcx Path, _: ast::NodeId) { debug!("visit_path: st={:?}", self.st); SawPath(path.global).hash(self.st); - self.hash_span(path.span); + hash_span!(self, path.span); visit::walk_path(self, path) } fn visit_block(&mut self, b: &'tcx Block) { debug!("visit_block: st={:?}", self.st); SawBlock.hash(self.st); - self.hash_span(b.span); + hash_span!(self, b.span); visit::walk_block(self, b) } @@ -407,7 +512,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has debug!("visit_pat: st={:?}", self.st); SawPat.hash(self.st); self.hash_discriminant(&p.node); - self.hash_span(p.span); + hash_span!(self, p.span); visit::walk_pat(self, p) } @@ -466,7 +571,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has debug!("visit_path_list_item: st={:?}", self.st); SawPathListItem.hash(self.st); self.hash_discriminant(&item.node); - self.hash_span(item.span); + hash_span!(self, item.span); visit::walk_path_list_item(self, prefix, item) } @@ -486,7 +591,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has fn visit_assoc_type_binding(&mut self, type_binding: &'tcx TypeBinding) { debug!("visit_assoc_type_binding: st={:?}", self.st); SawAssocTypeBinding.hash(self.st); - self.hash_span(type_binding.span); + hash_span!(self, type_binding.span); visit::walk_assoc_type_binding(self, type_binding) } @@ -602,21 +707,21 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { } fn hash_meta_item(&mut self, meta_item: &ast::MetaItem) { + debug!("hash_meta_item: st={:?}", self.st); + // ignoring span information, it doesn't matter here + self.hash_discriminant(&meta_item.node); match meta_item.node { ast::MetaItemKind::Word(ref s) => { - "Word".hash(self.st); s.len().hash(self.st); s.hash(self.st); } ast::MetaItemKind::NameValue(ref s, ref lit) => { - "NameValue".hash(self.st); s.len().hash(self.st); s.hash(self.st); lit.node.hash(self.st); } ast::MetaItemKind::List(ref s, ref items) => { - "List".hash(self.st); s.len().hash(self.st); s.hash(self.st); // Sort subitems so the hash does not depend on their order @@ -633,14 +738,18 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { } pub fn hash_attributes(&mut self, attributes: &[Attribute]) { + debug!("hash_attributes: st={:?}", self.st); let indices = self.indices_sorted_by(attributes, |attr| { meta_item_sort_key(&attr.node.value) }); for i in indices { let attr = &attributes[i].node; - SawAttribute(attr.style, attr.is_sugared_doc).hash(self.st); - self.hash_meta_item(&*attr.value); + + if !IGNORED_ATTRIBUTES.contains(&&*meta_item_sort_key(&attr.value)) { + SawAttribute(attr.style, attr.is_sugared_doc).hash(self.st); + self.hash_meta_item(&*attr.value); + } } } diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index b176b8fefc612..cd6f2874954b8 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -348,26 +348,10 @@ impl CodeMap { let files = self.files.borrow(); let f = (*files)[idx].clone(); - let len = f.lines.borrow().len(); - if len == 0 { - return Err(f); + match f.lookup_line(pos) { + Some(line) => Ok(FileMapAndLine { fm: f, line: line }), + None => Err(f) } - - let mut a = 0; - { - let lines = f.lines.borrow(); - let mut b = lines.len(); - while b - a > 1 { - let m = (a + b) / 2; - if (*lines)[m] > pos { - b = m; - } else { - a = m; - } - } - assert!(a <= lines.len()); - } - Ok(FileMapAndLine { fm: f, line: a }) } pub fn lookup_char_pos_adj(&self, pos: BytePos) -> LocWithOpt { @@ -691,7 +675,7 @@ impl CodeMap { } // Return the index of the filemap (in self.files) which contains pos. - fn lookup_filemap_idx(&self, pos: BytePos) -> usize { + pub fn lookup_filemap_idx(&self, pos: BytePos) -> usize { let files = self.files.borrow(); let files = &*files; let count = files.len(); diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index b11bbea84abce..d835f8058fa0e 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -507,6 +507,39 @@ impl FileMap { pub fn count_lines(&self) -> usize { self.lines.borrow().len() } + + /// Find the line containing the given position. The return value is the + /// index into the `lines` array of this FileMap, not the 1-based line + /// number. If the filemap is empty or the position is located before the + /// first line, None is returned. + pub fn lookup_line(&self, pos: BytePos) -> Option { + let lines = self.lines.borrow(); + if lines.len() == 0 { + return None; + } + + let line_index = lookup_line(&lines[..], pos); + assert!(line_index < lines.len() as isize); + if line_index >= 0 { + Some(line_index as usize) + } else { + None + } + } + + pub fn line_bounds(&self, line_index: usize) -> (BytePos, BytePos) { + if self.start_pos == self.end_pos { + return (self.start_pos, self.end_pos); + } + + let lines = self.lines.borrow(); + assert!(line_index < lines.len()); + if line_index == (lines.len() - 1) { + (lines[line_index], self.end_pos) + } else { + (lines[line_index], lines[line_index + 1]) + } + } } // _____________________________________________________________________________ @@ -688,3 +721,34 @@ pub struct MalformedCodemapPositions { pub end_pos: BytePos } +// Given a slice of line start positions and a position, returns the index of +// the line the position is on. Returns -1 if the position is located before +// the first line. +fn lookup_line(lines: &[BytePos], pos: BytePos) -> isize { + match lines.binary_search(&pos) { + Ok(line) => line as isize, + Err(line) => line as isize - 1 + } +} + +#[cfg(test)] +mod tests { + use super::{lookup_line, BytePos}; + + #[test] + fn test_lookup_line() { + + let lines = &[BytePos(3), BytePos(17), BytePos(28)]; + + assert_eq!(lookup_line(lines, BytePos(0)), -1); + assert_eq!(lookup_line(lines, BytePos(3)), 0); + assert_eq!(lookup_line(lines, BytePos(4)), 0); + + assert_eq!(lookup_line(lines, BytePos(16)), 0); + assert_eq!(lookup_line(lines, BytePos(17)), 1); + assert_eq!(lookup_line(lines, BytePos(18)), 1); + + assert_eq!(lookup_line(lines, BytePos(28)), 2); + assert_eq!(lookup_line(lines, BytePos(29)), 2); + } +} From 8b67ad69a7fb943d79d74b588057b5071c406060 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 26 Aug 2016 16:31:02 -0400 Subject: [PATCH 04/14] incr.comp. Add tests for stable span hashing. --- src/test/incremental/source_loc_macros.rs | 63 +++++++++++++++++++ .../span_hash_stable/auxiliary/mod.rs | 17 +++++ .../span_hash_stable/auxiliary/sub1.rs | 15 +++++ .../span_hash_stable/auxiliary/sub2.rs | 15 +++++ src/test/incremental/span_hash_stable/main.rs | 34 ++++++++++ .../spans_insignificant_w_o_debuginfo.rs | 25 ++++++++ .../spans_significant_w_debuginfo.rs | 25 ++++++++ 7 files changed, 194 insertions(+) create mode 100644 src/test/incremental/source_loc_macros.rs create mode 100644 src/test/incremental/span_hash_stable/auxiliary/mod.rs create mode 100644 src/test/incremental/span_hash_stable/auxiliary/sub1.rs create mode 100644 src/test/incremental/span_hash_stable/auxiliary/sub2.rs create mode 100644 src/test/incremental/span_hash_stable/main.rs create mode 100644 src/test/incremental/spans_insignificant_w_o_debuginfo.rs create mode 100644 src/test/incremental/spans_significant_w_debuginfo.rs diff --git a/src/test/incremental/source_loc_macros.rs b/src/test/incremental/source_loc_macros.rs new file mode 100644 index 0000000000000..f922ac0da41b1 --- /dev/null +++ b/src/test/incremental/source_loc_macros.rs @@ -0,0 +1,63 @@ +// Copyright 2016 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. + +// This test makes sure that different expansions of the file!(), line!(), +// column!() macros get picked up by the incr. comp. hash. + +// revisions:rpass1 rpass2 + +// compile-flags: -Z query-dep-graph + +#![feature(rustc_attrs)] + +#[rustc_clean(label="Hir", cfg="rpass2")] +fn line_same() { + let _ = line!(); +} + +#[rustc_clean(label="Hir", cfg="rpass2")] +fn col_same() { + let _ = column!(); +} + +#[rustc_clean(label="Hir", cfg="rpass2")] +fn file_same() { + let _ = file!(); +} + +#[cfg(rpass1)] +fn line_different() { + let _ = line!(); +} + +#[cfg(rpass2)] +#[rustc_dirty(label="Hir", cfg="rpass2")] +fn line_different() { + let _ = line!(); +} + +#[cfg(rpass1)] +fn col_different() { + let _ = column!(); +} + +#[cfg(rpass2)] +#[rustc_dirty(label="Hir", cfg="rpass2")] +fn col_different() { + let _ = column!(); +} + +fn main() { + line_same(); + line_different(); + col_same(); + col_different(); + file_same(); +} diff --git a/src/test/incremental/span_hash_stable/auxiliary/mod.rs b/src/test/incremental/span_hash_stable/auxiliary/mod.rs new file mode 100644 index 0000000000000..dfd2a6610f259 --- /dev/null +++ b/src/test/incremental/span_hash_stable/auxiliary/mod.rs @@ -0,0 +1,17 @@ +// Copyright 2016 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. + +#[cfg(rpass1)] +pub mod sub2; + +pub mod sub1; + +#[cfg(rpass2)] +pub mod sub2; diff --git a/src/test/incremental/span_hash_stable/auxiliary/sub1.rs b/src/test/incremental/span_hash_stable/auxiliary/sub1.rs new file mode 100644 index 0000000000000..2d042c316833e --- /dev/null +++ b/src/test/incremental/span_hash_stable/auxiliary/sub1.rs @@ -0,0 +1,15 @@ +// Copyright 2016 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. + +#[rustc_clean(label="Hir", cfg="rpass2")] +pub struct SomeType { + pub x: u32, + pub y: i64, +} diff --git a/src/test/incremental/span_hash_stable/auxiliary/sub2.rs b/src/test/incremental/span_hash_stable/auxiliary/sub2.rs new file mode 100644 index 0000000000000..df7d2f0267d02 --- /dev/null +++ b/src/test/incremental/span_hash_stable/auxiliary/sub2.rs @@ -0,0 +1,15 @@ +// Copyright 2016 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. + +#[rustc_clean(label="Hir", cfg="rpass2")] +pub struct SomeOtherType { + pub a: i32, + pub b: u64, +} diff --git a/src/test/incremental/span_hash_stable/main.rs b/src/test/incremental/span_hash_stable/main.rs new file mode 100644 index 0000000000000..1512c5dc53788 --- /dev/null +++ b/src/test/incremental/span_hash_stable/main.rs @@ -0,0 +1,34 @@ +// Copyright 2016 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. + +// This test makes sure that it doesn't make a difference in which order we are +// adding source files to the codemap. The order affects the BytePos values of +// the spans and this test makes sure that we handle them correctly by hashing +// file:line:column instead of raw byte offset. + +// revisions:rpass1 rpass2 +// compile-flags: -g -Z query-dep-graph + +#![feature(rustc_attrs)] + +mod auxiliary; + +fn main() { + let _ = auxiliary::sub1::SomeType { + x: 0, + y: 1, + }; + + let _ = auxiliary::sub2::SomeOtherType { + a: 2, + b: 3, + }; +} + diff --git a/src/test/incremental/spans_insignificant_w_o_debuginfo.rs b/src/test/incremental/spans_insignificant_w_o_debuginfo.rs new file mode 100644 index 0000000000000..9c8b8552498c4 --- /dev/null +++ b/src/test/incremental/spans_insignificant_w_o_debuginfo.rs @@ -0,0 +1,25 @@ +// Copyright 2016 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. + +// This test makes sure that just changing a definition's location in the +// source file does *not* change its incr. comp. hash, if debuginfo is disabled. + +// revisions:rpass1 rpass2 + +// compile-flags: -Z query-dep-graph + +#![feature(rustc_attrs)] + +#[cfg(rpass1)] +pub fn main() {} + +#[cfg(rpass2)] +#[rustc_clean(label="Hir", cfg="rpass2")] +pub fn main() {} diff --git a/src/test/incremental/spans_significant_w_debuginfo.rs b/src/test/incremental/spans_significant_w_debuginfo.rs new file mode 100644 index 0000000000000..b0920aa1fa510 --- /dev/null +++ b/src/test/incremental/spans_significant_w_debuginfo.rs @@ -0,0 +1,25 @@ +// Copyright 2016 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. + +// This test makes sure that just changing a definition's location in the +// source file also changes its incr. comp. hash, if debuginfo is enabled. + +// revisions:rpass1 rpass2 + +// compile-flags: -g -Z query-dep-graph + +#![feature(rustc_attrs)] + +#[cfg(rpass1)] +pub fn main() {} + +#[cfg(rpass2)] +#[rustc_dirty(label="Hir", cfg="rpass2")] +pub fn main() {} From 1cfd7c36542bfbd68a9808a6fc295e1ffe98a749 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 26 Aug 2016 18:29:13 -0400 Subject: [PATCH 05/14] incr.comp.: Ignore doc-comments when computing the ICH. --- src/librustc_incremental/calculate_svh/svh_visitor.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index 554a0e0a97cd3..7bc4d0ac46337 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -236,7 +236,7 @@ enum SawAbiComponent<'a> { SawTyParamBound, SawPolyTraitRef, SawAssocTypeBinding, - SawAttribute(ast::AttrStyle, bool), + SawAttribute(ast::AttrStyle), SawMacroDef, SawSpan(&'a str, usize, BytePos, &'a str, usize, BytePos, SawSpanExpnKind), } @@ -746,8 +746,9 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { for i in indices { let attr = &attributes[i].node; - if !IGNORED_ATTRIBUTES.contains(&&*meta_item_sort_key(&attr.value)) { - SawAttribute(attr.style, attr.is_sugared_doc).hash(self.st); + if !attr.is_sugared_doc && + !IGNORED_ATTRIBUTES.contains(&&*meta_item_sort_key(&attr.value)) { + SawAttribute(attr.style).hash(self.st); self.hash_meta_item(&*attr.value); } } From 5dd36bd48630e925ec617e40db8d23b593814cef Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 29 Aug 2016 15:28:53 -0400 Subject: [PATCH 06/14] Rename CacheCodemapView to CachingCodemapView. --- .../calculate_svh/svh_visitor.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index 7bc4d0ac46337..84f4ac4b74570 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -40,22 +40,22 @@ pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> { // collect a deterministic hash of def-ids that we have seen def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>, hash_spans: bool, - codemap: CachedCodemapView<'tcx>, + codemap: CachingCodemapView<'tcx>, } -struct CachedCodemapView<'tcx> { +struct CachingCodemapView<'tcx> { codemap: &'tcx CodeMap, // Format: (line number, line-start, line_end, file) line_cache: [(usize, BytePos, BytePos, Rc); 4], eviction_index: usize, } -impl<'tcx> CachedCodemapView<'tcx> { - fn new<'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CachedCodemapView<'tcx> { +impl<'tcx> CachingCodemapView<'tcx> { + fn new<'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CachingCodemapView<'tcx> { let codemap = tcx.sess.codemap(); let first_file = codemap.files.borrow()[0].clone(); - CachedCodemapView { + CachingCodemapView { codemap: codemap, line_cache: [(0, BytePos(0), BytePos(0), first_file.clone()), (0, BytePos(0), BytePos(0), first_file.clone()), @@ -123,7 +123,7 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { tcx: tcx, def_path_hashes: def_path_hashes, hash_spans: hash_spans, - codemap: CachedCodemapView::new(tcx), + codemap: CachingCodemapView::new(tcx), } } From 6785256557d6a4c21d53806637042871babc7302 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 29 Aug 2016 15:29:50 -0400 Subject: [PATCH 07/14] ICH: Don't hash span expansion kind twice. --- src/librustc_incremental/calculate_svh/svh_visitor.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index 84f4ac4b74570..db062d6dca96d 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -163,8 +163,6 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { _ => SawSpanExpnKind::SomeExpansion, }; - expansion_kind.hash(self.st); - SawSpan(&file1.name[..], line1, col1, &file2.name[..], line2, col2, expansion_kind).hash(self.st); From 500ab357c263eaa8ae55ed1323daa946def5b7b6 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 29 Aug 2016 15:47:44 -0400 Subject: [PATCH 08/14] ICH: Cleanup some comments. --- .../calculate_svh/svh_visitor.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index db062d6dca96d..702eed9d96fc5 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -45,7 +45,7 @@ pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> { struct CachingCodemapView<'tcx> { codemap: &'tcx CodeMap, - // Format: (line number, line-start, line_end, file) + // Format: (line number, line-start, line-end, file) line_cache: [(usize, BytePos, BytePos, Rc); 4], eviction_index: usize, } @@ -100,7 +100,7 @@ impl<'tcx> CachingCodemapView<'tcx> { let line_index = file.lookup_line(pos).unwrap(); let (line_start, line_end) = file.line_bounds(line_index); - // Just overwrite some cache entry. If we got this for, all of them + // Just overwrite some cache entry. If we got this far, all of them // point to the wrong file. self.line_cache[self.eviction_index] = (line_index + 1, line_start, @@ -131,11 +131,11 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { self.def_path_hashes.hash(def_id) } - // Hash a span in a stable way. If we would just hash the spans BytePos - // fields that would be similar hashing pointers since those or just offsets - // into the CodeMap. Instead, we hash the (file name, line, column) triple, - // which stays the same even if the containing FileMap has moved within the - // CodeMap. + // Hash a span in a stable way. We can't directly hash the span's BytePos + // fields (that would be similar to hashing pointers, since those are just + // offsets into the CodeMap). Instead, we hash the (file name, line, column) + // triple, which stays the same even if the containing FileMap has moved + // within the CodeMap. // Also note that we are hashing byte offsets for the column, not unicode // codepoint offsets. For the purpose of the hash that's sufficient. fn hash_span(&mut self, span: Span) { @@ -462,7 +462,6 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has fn visit_generics(&mut self, g: &'tcx Generics) { debug!("visit_generics: st={:?}", self.st); SawGenerics.hash(self.st); - // FIXME: nested stuff visit::walk_generics(self, g) } @@ -605,7 +604,8 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has SawMacroDef.hash(self.st); hash_attrs!(self, ¯o_def.attrs); visit::walk_macro_def(self, macro_def) - // FIXME: We should hash the body of the macro too. + // FIXME(mw): We should hash the body of the macro too but we don't + // have a stable way of doing so yet. } } } From 0310e3444b6b24da1b542b7c6b2999a1220259c0 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 29 Aug 2016 16:45:34 -0400 Subject: [PATCH 09/14] ICH: Take CaptureClause of closure expressions into account. --- src/librustc_incremental/calculate_svh/svh_visitor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index 702eed9d96fc5..23fb211d7c7e4 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -275,7 +275,7 @@ enum SawExprComponent<'a> { SawExprIf, SawExprWhile, SawExprMatch, - SawExprClosure, + SawExprClosure(CaptureClause), SawExprBlock, SawExprAssign, SawExprAssignOp(hir::BinOp_), @@ -304,7 +304,7 @@ fn saw_expr<'a>(node: &'a Expr_) -> SawExprComponent<'a> { ExprWhile(..) => SawExprWhile, ExprLoop(_, id) => SawExprLoop(id.map(|id| id.node.as_str())), ExprMatch(..) => SawExprMatch, - ExprClosure(..) => SawExprClosure, + ExprClosure(cc, _, _, _) => SawExprClosure(cc), ExprBlock(..) => SawExprBlock, ExprAssign(..) => SawExprAssign, ExprAssignOp(op, _, _) => SawExprAssignOp(op.node), From a142d2ff025466a69662854255c959f60a31fd8c Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 30 Aug 2016 12:11:43 -0400 Subject: [PATCH 10/14] ICH: Fix bug in hash_discriminant() and visit_vis(). --- src/librustc_incremental/calculate_svh/svh_visitor.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index 23fb211d7c7e4..edfde33b3a15a 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -174,7 +174,9 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { fn hash_discriminant(&mut self, v: &T) { unsafe { - ::std::intrinsics::discriminant_value(&v).hash(self.st); + let disr = ::std::intrinsics::discriminant_value(v); + debug!("hash_discriminant: disr={}, st={:?}", disr, self.st); + disr.hash(self.st); } } } @@ -536,7 +538,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has fn visit_vis(&mut self, v: &'tcx Visibility) { debug!("visit_vis: st={:?}", self.st); SawVis.hash(self.st); - self.hash_discriminant(&v); + self.hash_discriminant(v); visit::walk_vis(self, v) } From 2faca22bd31b0deeb38f35dc8d0916fb6ace95c2 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 31 Aug 2016 16:29:04 -0400 Subject: [PATCH 11/14] ICH: Fix codemap lookup caching. --- .../calculate_svh/svh_visitor.rs | 94 ++++++++++--------- 1 file changed, 52 insertions(+), 42 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index edfde33b3a15a..e871dd5bcbffe 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -43,72 +43,82 @@ pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> { codemap: CachingCodemapView<'tcx>, } +#[derive(Clone)] +struct CacheEntry { + time_stamp: usize, + line_number: usize, + line_start: BytePos, + line_end: BytePos, + file: Rc, +} + struct CachingCodemapView<'tcx> { codemap: &'tcx CodeMap, - // Format: (line number, line-start, line-end, file) - line_cache: [(usize, BytePos, BytePos, Rc); 4], - eviction_index: usize, + line_cache: [CacheEntry; 3], + time_stamp: usize, } impl<'tcx> CachingCodemapView<'tcx> { fn new<'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CachingCodemapView<'tcx> { let codemap = tcx.sess.codemap(); let first_file = codemap.files.borrow()[0].clone(); + let entry = CacheEntry { + time_stamp: 0, + line_number: 0, + line_start: BytePos(0), + line_end: BytePos(0), + file: first_file, + }; CachingCodemapView { codemap: codemap, - line_cache: [(0, BytePos(0), BytePos(0), first_file.clone()), - (0, BytePos(0), BytePos(0), first_file.clone()), - (0, BytePos(0), BytePos(0), first_file.clone()), - (0, BytePos(0), BytePos(0), first_file.clone())], - eviction_index: 0, + line_cache: [entry.clone(), entry.clone(), entry.clone()], + time_stamp: 0, } } fn byte_pos_to_line_and_col(&mut self, pos: BytePos) -> (Rc, usize, BytePos) { + self.time_stamp += 1; + // Check if the position is in one of the cached lines - for &(line, start, end, ref file) in self.line_cache.iter() { - if pos >= start && pos < end { - return (file.clone(), line, pos - start); + for cache_entry in self.line_cache.iter_mut() { + if pos >= cache_entry.line_start && pos < cache_entry.line_end { + cache_entry.time_stamp = self.time_stamp; + return (cache_entry.file.clone(), + cache_entry.line_number, + pos - cache_entry.line_start); } } - // Check whether we have a cached line in the correct file, so we can - // overwrite it without having to look up the file again. - for &mut (ref mut line, - ref mut start, - ref mut end, - ref file) in self.line_cache.iter_mut() { - if pos >= file.start_pos && pos < file.end_pos { - let line_index = file.lookup_line(pos).unwrap(); - let (line_start, line_end) = file.line_bounds(line_index); - - // Update the cache entry in place - *line = line_index + 1; - *start = line_start; - *end = line_end; - - return (file.clone(), line_index + 1, pos - line_start); + // No cache hit ... + let mut oldest = 0; + for index in 1 .. self.line_cache.len() { + if self.line_cache[index].time_stamp < self.line_cache[oldest].time_stamp { + oldest = index; } } - // No cache hit ... - let file_index = self.codemap.lookup_filemap_idx(pos); - let file = self.codemap.files.borrow()[file_index].clone(); - let line_index = file.lookup_line(pos).unwrap(); - let (line_start, line_end) = file.line_bounds(line_index); - - // Just overwrite some cache entry. If we got this far, all of them - // point to the wrong file. - self.line_cache[self.eviction_index] = (line_index + 1, - line_start, - line_end, - file.clone()); - self.eviction_index = (self.eviction_index + 1) % self.line_cache.len(); - - return (file, line_index + 1, pos - line_start); + let cache_entry = &mut self.line_cache[oldest]; + + // If the entry doesn't point to the correct file, fix it up + if pos < cache_entry.file.start_pos || pos >= cache_entry.file.end_pos { + let file_index = self.codemap.lookup_filemap_idx(pos); + cache_entry.file = self.codemap.files.borrow()[file_index].clone(); + } + + let line_index = cache_entry.file.lookup_line(pos).unwrap(); + let line_bounds = cache_entry.file.line_bounds(line_index); + + cache_entry.line_number = line_index + 1; + cache_entry.line_start = line_bounds.0; + cache_entry.line_end = line_bounds.1; + cache_entry.time_stamp = self.time_stamp; + + return (cache_entry.file.clone(), + cache_entry.line_number, + pos - cache_entry.line_start); } } From 8cbd6fe33155fde25146d2cf03aa26e450086106 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 31 Aug 2016 16:51:24 -0400 Subject: [PATCH 12/14] ICH: Share codemap cache between subsequent runs of the ICH visitor. --- .../calculate_svh/caching_codemap_view.rs | 97 +++++++++++++++++++ src/librustc_incremental/calculate_svh/mod.rs | 8 +- .../calculate_svh/svh_visitor.rs | 92 ++---------------- 3 files changed, 111 insertions(+), 86 deletions(-) create mode 100644 src/librustc_incremental/calculate_svh/caching_codemap_view.rs diff --git a/src/librustc_incremental/calculate_svh/caching_codemap_view.rs b/src/librustc_incremental/calculate_svh/caching_codemap_view.rs new file mode 100644 index 0000000000000..32aa5a4272871 --- /dev/null +++ b/src/librustc_incremental/calculate_svh/caching_codemap_view.rs @@ -0,0 +1,97 @@ +// Copyright 2016 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 rustc::ty::TyCtxt; +use std::rc::Rc; +use syntax::codemap::CodeMap; +use syntax_pos::{BytePos, FileMap}; + +#[derive(Clone)] +struct CacheEntry { + time_stamp: usize, + line_number: usize, + line_start: BytePos, + line_end: BytePos, + file: Rc, +} + +pub struct CachingCodemapView<'tcx> { + codemap: &'tcx CodeMap, + line_cache: [CacheEntry; 3], + time_stamp: usize, +} + +impl<'tcx> CachingCodemapView<'tcx> { + pub fn new<'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CachingCodemapView<'tcx> { + let codemap = tcx.sess.codemap(); + let first_file = codemap.files.borrow()[0].clone(); + let entry = CacheEntry { + time_stamp: 0, + line_number: 0, + line_start: BytePos(0), + line_end: BytePos(0), + file: first_file, + }; + + CachingCodemapView { + codemap: codemap, + line_cache: [entry.clone(), entry.clone(), entry.clone()], + time_stamp: 0, + } + } + + pub fn codemap(&self) -> &'tcx CodeMap { + self.codemap + } + + pub fn byte_pos_to_line_and_col(&mut self, + pos: BytePos) + -> (Rc, usize, BytePos) { + self.time_stamp += 1; + + // Check if the position is in one of the cached lines + for cache_entry in self.line_cache.iter_mut() { + if pos >= cache_entry.line_start && pos < cache_entry.line_end { + cache_entry.time_stamp = self.time_stamp; + return (cache_entry.file.clone(), + cache_entry.line_number, + pos - cache_entry.line_start); + } + } + + // No cache hit ... + let mut oldest = 0; + for index in 1 .. self.line_cache.len() { + if self.line_cache[index].time_stamp < self.line_cache[oldest].time_stamp { + oldest = index; + } + } + + let cache_entry = &mut self.line_cache[oldest]; + + // If the entry doesn't point to the correct file, fix it up + if pos < cache_entry.file.start_pos || pos >= cache_entry.file.end_pos { + let file_index = self.codemap.lookup_filemap_idx(pos); + cache_entry.file = self.codemap.files.borrow()[file_index].clone(); + } + + let line_index = cache_entry.file.lookup_line(pos).unwrap(); + let line_bounds = cache_entry.file.line_bounds(line_index); + + cache_entry.line_number = line_index + 1; + cache_entry.line_start = line_bounds.0; + cache_entry.line_end = line_bounds.1; + cache_entry.time_stamp = self.time_stamp; + + return (cache_entry.file.clone(), + cache_entry.line_number, + pos - cache_entry.line_start); + } +} diff --git a/src/librustc_incremental/calculate_svh/mod.rs b/src/librustc_incremental/calculate_svh/mod.rs index 6ad93d8f4733c..c54fe2114517e 100644 --- a/src/librustc_incremental/calculate_svh/mod.rs +++ b/src/librustc_incremental/calculate_svh/mod.rs @@ -40,9 +40,11 @@ use rustc::session::config::DebugInfoLevel::NoDebugInfo; use self::def_path_hash::DefPathHashes; use self::svh_visitor::StrictVersionHashVisitor; +use self::caching_codemap_view::CachingCodemapView; mod def_path_hash; mod svh_visitor; +mod caching_codemap_view; pub type IncrementalHashesMap = FnvHashMap, u64>; @@ -55,7 +57,8 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) tcx: tcx, hashes: FnvHashMap(), def_path_hashes: DefPathHashes::new(tcx), - hash_spans: hash_spans + codemap: CachingCodemapView::new(tcx), + hash_spans: hash_spans, }; record_time(&tcx.sess.perf_stats.incr_comp_hashes_time, || { visitor.calculate_def_id(DefId::local(CRATE_DEF_INDEX), @@ -69,6 +72,7 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) struct HashItemsVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, def_path_hashes: DefPathHashes<'a, 'tcx>, + codemap: CachingCodemapView<'tcx>, hashes: IncrementalHashesMap, hash_spans: bool, } @@ -92,6 +96,7 @@ impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> { walk_op(&mut StrictVersionHashVisitor::new(&mut state, self.tcx, &mut self.def_path_hashes, + &mut self.codemap, self.hash_spans)); let item_hash = state.finish(); self.hashes.insert(DepNode::Hir(def_id), item_hash); @@ -132,6 +137,7 @@ impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> { let mut visitor = StrictVersionHashVisitor::new(&mut crate_state, self.tcx, &mut self.def_path_hashes, + &mut self.codemap, self.hash_spans); visitor.hash_attributes(&krate.attrs); } diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index e871dd5bcbffe..417f09c1c9d55 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -17,18 +17,17 @@ use self::SawExprComponent::*; use self::SawAbiComponent::*; use syntax::ast::{self, Name, NodeId, Attribute}; use syntax::parse::token; -use syntax::codemap::CodeMap; -use syntax_pos::{Span, NO_EXPANSION, COMMAND_LINE_EXPN, BytePos, FileMap}; +use syntax_pos::{Span, NO_EXPANSION, COMMAND_LINE_EXPN, BytePos}; use rustc::hir; use rustc::hir::*; use rustc::hir::def::{Def, PathResolution}; use rustc::hir::def_id::DefId; use rustc::hir::intravisit as visit; use rustc::ty::TyCtxt; -use std::rc::Rc; use std::hash::{Hash, SipHasher}; use super::def_path_hash::DefPathHashes; +use super::caching_codemap_view::CachingCodemapView; const IGNORED_ATTRIBUTES: &'static [&'static str] = &["cfg", "rustc_clean", @@ -40,92 +39,14 @@ pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> { // collect a deterministic hash of def-ids that we have seen def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>, hash_spans: bool, - codemap: CachingCodemapView<'tcx>, -} - -#[derive(Clone)] -struct CacheEntry { - time_stamp: usize, - line_number: usize, - line_start: BytePos, - line_end: BytePos, - file: Rc, -} - -struct CachingCodemapView<'tcx> { - codemap: &'tcx CodeMap, - line_cache: [CacheEntry; 3], - time_stamp: usize, -} - -impl<'tcx> CachingCodemapView<'tcx> { - fn new<'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CachingCodemapView<'tcx> { - let codemap = tcx.sess.codemap(); - let first_file = codemap.files.borrow()[0].clone(); - let entry = CacheEntry { - time_stamp: 0, - line_number: 0, - line_start: BytePos(0), - line_end: BytePos(0), - file: first_file, - }; - - CachingCodemapView { - codemap: codemap, - line_cache: [entry.clone(), entry.clone(), entry.clone()], - time_stamp: 0, - } - } - - fn byte_pos_to_line_and_col(&mut self, - pos: BytePos) - -> (Rc, usize, BytePos) { - self.time_stamp += 1; - - // Check if the position is in one of the cached lines - for cache_entry in self.line_cache.iter_mut() { - if pos >= cache_entry.line_start && pos < cache_entry.line_end { - cache_entry.time_stamp = self.time_stamp; - return (cache_entry.file.clone(), - cache_entry.line_number, - pos - cache_entry.line_start); - } - } - - // No cache hit ... - let mut oldest = 0; - for index in 1 .. self.line_cache.len() { - if self.line_cache[index].time_stamp < self.line_cache[oldest].time_stamp { - oldest = index; - } - } - - let cache_entry = &mut self.line_cache[oldest]; - - // If the entry doesn't point to the correct file, fix it up - if pos < cache_entry.file.start_pos || pos >= cache_entry.file.end_pos { - let file_index = self.codemap.lookup_filemap_idx(pos); - cache_entry.file = self.codemap.files.borrow()[file_index].clone(); - } - - let line_index = cache_entry.file.lookup_line(pos).unwrap(); - let line_bounds = cache_entry.file.line_bounds(line_index); - - cache_entry.line_number = line_index + 1; - cache_entry.line_start = line_bounds.0; - cache_entry.line_end = line_bounds.1; - cache_entry.time_stamp = self.time_stamp; - - return (cache_entry.file.clone(), - cache_entry.line_number, - pos - cache_entry.line_start); - } + codemap: &'a mut CachingCodemapView<'tcx>, } impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { pub fn new(st: &'a mut SipHasher, tcx: TyCtxt<'hash, 'tcx, 'tcx>, def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>, + codemap: &'a mut CachingCodemapView<'tcx>, hash_spans: bool) -> Self { StrictVersionHashVisitor { @@ -133,7 +54,7 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { tcx: tcx, def_path_hashes: def_path_hashes, hash_spans: hash_spans, - codemap: CachingCodemapView::new(tcx), + codemap: codemap, } } @@ -178,7 +99,8 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { expansion_kind).hash(self.st); if expansion_kind == SawSpanExpnKind::SomeExpansion { - self.hash_span(self.codemap.codemap.source_callsite(span)); + let call_site = self.codemap.codemap().source_callsite(span); + self.hash_span(call_site); } } From 7310a8ffea95400c2c61d4fb27c224eb0e64e244 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 1 Sep 2016 14:39:31 -0400 Subject: [PATCH 13/14] ICH: Adapt to changes in the MetaItem AST representation. --- src/librustc_data_structures/fnv.rs | 6 ++++ .../calculate_svh/svh_visitor.rs | 35 +++++++++---------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/librustc_data_structures/fnv.rs b/src/librustc_data_structures/fnv.rs index 0000c283a7a00..47f623266f3b7 100644 --- a/src/librustc_data_structures/fnv.rs +++ b/src/librustc_data_structures/fnv.rs @@ -57,3 +57,9 @@ impl Hasher for FnvHasher { self.0 } } + +pub fn hash(v: &T) -> u64 { + let mut state = FnvHasher::default(); + v.hash(&mut state); + state.finish() +} diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index 417f09c1c9d55..05a2f751d2921 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -15,7 +15,7 @@ use self::SawExprComponent::*; use self::SawAbiComponent::*; -use syntax::ast::{self, Name, NodeId, Attribute}; +use syntax::ast::{self, Name, NodeId}; use syntax::parse::token; use syntax_pos::{Span, NO_EXPANSION, COMMAND_LINE_EXPN, BytePos}; use rustc::hir; @@ -24,6 +24,7 @@ use rustc::hir::def::{Def, PathResolution}; use rustc::hir::def_id::DefId; use rustc::hir::intravisit as visit; use rustc::ty::TyCtxt; +use rustc_data_structures::fnv; use std::hash::{Hash, SipHasher}; use super::def_path_hash::DefPathHashes; @@ -526,7 +527,7 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has visit::walk_assoc_type_binding(self, type_binding) } - fn visit_attribute(&mut self, _: &Attribute) { + fn visit_attribute(&mut self, _: &ast::Attribute) { // We explicitly do not use this method, since doing that would // implicitly impose an order on the attributes being hashed, while we // explicitly don't want their order to matter @@ -658,28 +659,34 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { s.hash(self.st); // Sort subitems so the hash does not depend on their order let indices = self.indices_sorted_by(&items, |p| { - meta_item_sort_key(&*p) + (p.name(), fnv::hash(&p.literal().map(|i| &i.node))) }); items.len().hash(self.st); for (index, &item_index) in indices.iter().enumerate() { index.hash(self.st); - self.hash_meta_item(&items[item_index]); + let nested_meta_item: &ast::NestedMetaItemKind = &items[item_index].node; + self.hash_discriminant(nested_meta_item); + match *nested_meta_item { + ast::NestedMetaItemKind::MetaItem(ref meta_item) => { + self.hash_meta_item(meta_item); + } + ast::NestedMetaItemKind::Literal(ref lit) => { + lit.node.hash(self.st); + } + } } } } } - pub fn hash_attributes(&mut self, attributes: &[Attribute]) { + pub fn hash_attributes(&mut self, attributes: &[ast::Attribute]) { debug!("hash_attributes: st={:?}", self.st); - let indices = self.indices_sorted_by(attributes, |attr| { - meta_item_sort_key(&attr.node.value) - }); + let indices = self.indices_sorted_by(attributes, |attr| attr.name()); for i in indices { let attr = &attributes[i].node; - if !attr.is_sugared_doc && - !IGNORED_ATTRIBUTES.contains(&&*meta_item_sort_key(&attr.value)) { + !IGNORED_ATTRIBUTES.contains(&&*attr.value.name()) { SawAttribute(attr.style).hash(self.st); self.hash_meta_item(&*attr.value); } @@ -696,11 +703,3 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { indices } } - -fn meta_item_sort_key(item: &ast::MetaItem) -> token::InternedString { - match item.node { - ast::MetaItemKind::Word(ref s) | - ast::MetaItemKind::NameValue(ref s, _) | - ast::MetaItemKind::List(ref s, _) => s.clone() - } -} From 3057b7b9749f16ad982b54b4bdeee7e2c8a845bb Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 6 Sep 2016 12:14:43 -0400 Subject: [PATCH 14/14] ICH: Make CachingCodemapView robustly handle invalid spans. --- .../calculate_svh/caching_codemap_view.rs | 36 ++++++++++++++----- .../calculate_svh/svh_visitor.rs | 15 ++++---- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/caching_codemap_view.rs b/src/librustc_incremental/calculate_svh/caching_codemap_view.rs index 32aa5a4272871..ad9c48420e217 100644 --- a/src/librustc_incremental/calculate_svh/caching_codemap_view.rs +++ b/src/librustc_incremental/calculate_svh/caching_codemap_view.rs @@ -53,16 +53,16 @@ impl<'tcx> CachingCodemapView<'tcx> { pub fn byte_pos_to_line_and_col(&mut self, pos: BytePos) - -> (Rc, usize, BytePos) { + -> Option<(Rc, usize, BytePos)> { self.time_stamp += 1; // Check if the position is in one of the cached lines for cache_entry in self.line_cache.iter_mut() { if pos >= cache_entry.line_start && pos < cache_entry.line_end { cache_entry.time_stamp = self.time_stamp; - return (cache_entry.file.clone(), - cache_entry.line_number, - pos - cache_entry.line_start); + return Some((cache_entry.file.clone(), + cache_entry.line_number, + pos - cache_entry.line_start)); } } @@ -78,8 +78,26 @@ impl<'tcx> CachingCodemapView<'tcx> { // If the entry doesn't point to the correct file, fix it up if pos < cache_entry.file.start_pos || pos >= cache_entry.file.end_pos { - let file_index = self.codemap.lookup_filemap_idx(pos); - cache_entry.file = self.codemap.files.borrow()[file_index].clone(); + let file_valid; + let files = self.codemap.files.borrow(); + + if files.len() > 0 { + let file_index = self.codemap.lookup_filemap_idx(pos); + let file = files[file_index].clone(); + + if pos >= file.start_pos && pos < file.end_pos { + cache_entry.file = file; + file_valid = true; + } else { + file_valid = false; + } + } else { + file_valid = false; + } + + if !file_valid { + return None; + } } let line_index = cache_entry.file.lookup_line(pos).unwrap(); @@ -90,8 +108,8 @@ impl<'tcx> CachingCodemapView<'tcx> { cache_entry.line_end = line_bounds.1; cache_entry.time_stamp = self.time_stamp; - return (cache_entry.file.clone(), - cache_entry.line_number, - pos - cache_entry.line_start); + return Some((cache_entry.file.clone(), + cache_entry.line_number, + pos - cache_entry.line_start)); } } diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index 05a2f751d2921..b1cad10f60aa8 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -86,8 +86,8 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { span.hi }; - let (file1, line1, col1) = self.codemap.byte_pos_to_line_and_col(span.lo); - let (file2, line2, col2) = self.codemap.byte_pos_to_line_and_col(span_hi); + let loc1 = self.codemap.byte_pos_to_line_and_col(span.lo); + let loc2 = self.codemap.byte_pos_to_line_and_col(span_hi); let expansion_kind = match span.expn_id { NO_EXPANSION => SawSpanExpnKind::NoExpansion, @@ -95,9 +95,10 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { _ => SawSpanExpnKind::SomeExpansion, }; - SawSpan(&file1.name[..], line1, col1, - &file2.name[..], line2, col2, - expansion_kind).hash(self.st); + SawSpan(loc1.as_ref().map(|&(ref fm, line, col)| (&fm.name[..], line, col)), + loc2.as_ref().map(|&(ref fm, line, col)| (&fm.name[..], line, col)), + expansion_kind) + .hash(self.st); if expansion_kind == SawSpanExpnKind::SomeExpansion { let call_site = self.codemap.codemap().source_callsite(span); @@ -171,7 +172,9 @@ enum SawAbiComponent<'a> { SawAssocTypeBinding, SawAttribute(ast::AttrStyle), SawMacroDef, - SawSpan(&'a str, usize, BytePos, &'a str, usize, BytePos, SawSpanExpnKind), + SawSpan(Option<(&'a str, usize, BytePos)>, + Option<(&'a str, usize, BytePos)>, + SawSpanExpnKind), } /// SawExprComponent carries all of the information that we want