From 003b1699c0044aef757c9c32df13a529fd4c485b Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 15 Nov 2016 11:55:34 -0500 Subject: [PATCH 1/5] Add error message when not finding the ICH of a DepNode. --- src/librustc_incremental/calculate_svh/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/librustc_incremental/calculate_svh/mod.rs b/src/librustc_incremental/calculate_svh/mod.rs index c08519090d273..0339f48827257 100644 --- a/src/librustc_incremental/calculate_svh/mod.rs +++ b/src/librustc_incremental/calculate_svh/mod.rs @@ -88,7 +88,12 @@ impl<'a> ::std::ops::Index<&'a DepNode> for IncrementalHashesMap { type Output = Fingerprint; fn index(&self, index: &'a DepNode) -> &Fingerprint { - &self.hashes[index] + match self.hashes.get(index) { + Some(fingerprint) => fingerprint, + None => { + bug!("Could not find ICH for {:?}", index); + } + } } } From a5137afe8cddf2366045c8d3de23a17c5ac8dc25 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 15 Nov 2016 15:20:39 -0500 Subject: [PATCH 2/5] ICH: Hash MacroDefs in a mostly stable way. --- src/librustc_incremental/calculate_svh/mod.rs | 7 +- .../calculate_svh/svh_visitor.rs | 139 +++++++++++++++++- 2 files changed, 143 insertions(+), 3 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/mod.rs b/src/librustc_incremental/calculate_svh/mod.rs index 0339f48827257..f98e698a1c9d4 100644 --- a/src/librustc_incremental/calculate_svh/mod.rs +++ b/src/librustc_incremental/calculate_svh/mod.rs @@ -46,6 +46,7 @@ use self::caching_codemap_view::CachingCodemapView; use self::hasher::IchHasher; use ich::Fingerprint; + mod def_path_hash; mod svh_visitor; mod caching_codemap_view; @@ -113,8 +114,12 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) 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)); - // FIXME(#37713) if foreign items were item likes, could use ItemLikeVisitor krate.visit_all_item_likes(&mut visitor.as_deep_visitor()); + + for macro_def in krate.exported_macros.iter() { + visitor.calculate_node_id(macro_def.id, + |v| v.visit_macro_def(macro_def)); + } }); tcx.sess.perf_stats.incr_comp_hashes_count.set(visitor.hashes.len() as u64); diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index fa2eff817eaa8..e8608b187d84c 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -24,6 +24,7 @@ use syntax::ast::{self, Name, NodeId}; use syntax::attr; use syntax::parse::token; use syntax_pos::{Span, NO_EXPANSION, COMMAND_LINE_EXPN, BytePos}; +use syntax::tokenstream; use rustc::hir; use rustc::hir::*; use rustc::hir::def::{Def, PathResolution}; @@ -769,9 +770,10 @@ impl<'a, 'hash, 'tcx> visit::Visitor<'tcx> for StrictVersionHashVisitor<'a, 'has debug!("visit_macro_def: st={:?}", self.st); SawMacroDef.hash(self.st); hash_attrs!(self, ¯o_def.attrs); + for tt in ¯o_def.body { + self.hash_token_tree(tt); + } visit::walk_macro_def(self, macro_def) - // FIXME(mw): We should hash the body of the macro too but we don't - // have a stable way of doing so yet. } } @@ -941,4 +943,137 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { self.overflow_checks_enabled = true; } } + + fn hash_token_tree(&mut self, tt: &tokenstream::TokenTree) { + self.hash_discriminant(tt); + match *tt { + tokenstream::TokenTree::Token(span, ref token) => { + hash_span!(self, span); + self.hash_token(token); + } + tokenstream::TokenTree::Delimited(span, ref delimited) => { + hash_span!(self, span); + let tokenstream::Delimited { + ref delim, + open_span, + ref tts, + close_span, + } = **delimited; + + delim.hash(self.st); + hash_span!(self, open_span); + tts.len().hash(self.st); + for sub_tt in tts { + self.hash_token_tree(sub_tt); + } + hash_span!(self, close_span); + } + tokenstream::TokenTree::Sequence(span, ref sequence_repetition) => { + hash_span!(self, span); + let tokenstream::SequenceRepetition { + ref tts, + ref separator, + op, + num_captures, + } = **sequence_repetition; + + tts.len().hash(self.st); + for sub_tt in tts { + self.hash_token_tree(sub_tt); + } + self.hash_discriminant(separator); + if let Some(ref separator) = *separator { + self.hash_token(separator); + } + op.hash(self.st); + num_captures.hash(self.st); + } + } + } + + fn hash_token(&mut self, token: &token::Token) { + self.hash_discriminant(token); + match *token { + token::Token::Eq | + token::Token::Lt | + token::Token::Le | + token::Token::EqEq | + token::Token::Ne | + token::Token::Ge | + token::Token::Gt | + token::Token::AndAnd | + token::Token::OrOr | + token::Token::Not | + token::Token::Tilde | + token::Token::At | + token::Token::Dot | + token::Token::DotDot | + token::Token::DotDotDot | + token::Token::Comma | + token::Token::Semi | + token::Token::Colon | + token::Token::ModSep | + token::Token::RArrow | + token::Token::LArrow | + token::Token::FatArrow | + token::Token::Pound | + token::Token::Dollar | + token::Token::Question | + token::Token::Underscore | + token::Token::Whitespace | + token::Token::Comment | + token::Token::Eof => {} + + token::Token::BinOp(bin_op_token) | + token::Token::BinOpEq(bin_op_token) => bin_op_token.hash(self.st), + + token::Token::OpenDelim(delim_token) | + token::Token::CloseDelim(delim_token) => delim_token.hash(self.st), + + token::Token::Literal(ref lit, ref opt_name) => { + self.hash_discriminant(lit); + match *lit { + token::Lit::Byte(val) | + token::Lit::Char(val) | + token::Lit::Integer(val) | + token::Lit::Float(val) | + token::Lit::Str_(val) | + token::Lit::ByteStr(val) => val.as_str().hash(self.st), + token::Lit::StrRaw(val, n) | + token::Lit::ByteStrRaw(val, n) => { + val.as_str().hash(self.st); + n.hash(self.st); + } + }; + opt_name.map(ast::Name::as_str).hash(self.st); + } + + token::Token::Ident(ident) | + token::Token::Lifetime(ident) | + token::Token::SubstNt(ident) => ident.name.as_str().hash(self.st), + token::Token::MatchNt(ident1, ident2) => { + ident1.name.as_str().hash(self.st); + ident2.name.as_str().hash(self.st); + } + + token::Token::Interpolated(ref non_terminal) => { + // FIXME(mw): This could be implemented properly. It's just a + // lot of work, since we would need to hash the AST + // in a stable way, in addition to the HIR. + // Since this is hardly used anywhere, just emit a + // warning for now. + if self.tcx.sess.opts.debugging_opts.incremental.is_some() { + let msg = format!("Quasi-quoting might make incremental \ + compilation very inefficient: {:?}", + non_terminal); + self.tcx.sess.warn(&msg[..]); + } + + non_terminal.hash(self.st); + } + + token::Token::DocComment(val) | + token::Token::Shebang(val) => val.as_str().hash(self.st), + } + } } From e6b30bda495eaf927a3bca66580f13095585e496 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 15 Nov 2016 15:22:15 -0500 Subject: [PATCH 3/5] Remove outdated comment about SVH. --- src/librustc_incremental/calculate_svh/svh_visitor.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index e8608b187d84c..faf4da55ee5db 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -8,11 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// FIXME (#14132): Even this SVH computation still has implementation -// artifacts: namely, the order of item declaration will affect the -// hash computation, but for many kinds of items the order of -// declaration should be irrelevant to the ABI. - use self::SawExprComponent::*; use self::SawAbiComponent::*; use self::SawItemComponent::*; From 52d250efabed644a15c797bfcc747a4113d2433d Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 15 Nov 2016 16:16:40 -0500 Subject: [PATCH 4/5] Add test case for exported macros vs incremental compilation. --- .../incr_comp_with_macro_export.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/test/compile-fail/incr_comp_with_macro_export.rs diff --git a/src/test/compile-fail/incr_comp_with_macro_export.rs b/src/test/compile-fail/incr_comp_with_macro_export.rs new file mode 100644 index 0000000000000..eafef17230367 --- /dev/null +++ b/src/test/compile-fail/incr_comp_with_macro_export.rs @@ -0,0 +1,23 @@ +// 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. + +// compile-flags: -Zincremental=tmp/cfail-tests/incr_comp_with_macro_export +// must-compile-successfully + + +// This test case makes sure that we can compile with incremental compilation +// enabled when there are macros exported from this crate. (See #37756) + +#![crate_type="rlib"] + +#[macro_export] +macro_rules! some_macro { + ($e:expr) => ($e + 1) +} From c722a1eb9992ac76f5c3dd0aee36b8734d613f11 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 16 Nov 2016 10:18:46 -0500 Subject: [PATCH 5/5] Add span to warning about incr. comp. vs Token::Interpolated. --- src/librustc_incremental/calculate_svh/svh_visitor.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index faf4da55ee5db..a1ece48462b14 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -944,7 +944,7 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { match *tt { tokenstream::TokenTree::Token(span, ref token) => { hash_span!(self, span); - self.hash_token(token); + self.hash_token(token, span); } tokenstream::TokenTree::Delimited(span, ref delimited) => { hash_span!(self, span); @@ -978,7 +978,7 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { } self.hash_discriminant(separator); if let Some(ref separator) = *separator { - self.hash_token(separator); + self.hash_token(separator, span); } op.hash(self.st); num_captures.hash(self.st); @@ -986,7 +986,9 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { } } - fn hash_token(&mut self, token: &token::Token) { + fn hash_token(&mut self, + token: &token::Token, + error_reporting_span: Span) { self.hash_discriminant(token); match *token { token::Token::Eq | @@ -1061,7 +1063,7 @@ impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { let msg = format!("Quasi-quoting might make incremental \ compilation very inefficient: {:?}", non_terminal); - self.tcx.sess.warn(&msg[..]); + self.tcx.sess.span_warn(error_reporting_span, &msg[..]); } non_terminal.hash(self.st);