Auto merge of #46490 - michaelwoerister:hash-spans-wip, r=<try>

EXPERIMENTAL: Hash spans unconditionally during incr. comp.

It would be good for reliability if we could do without all this special-casing around change detection for spans. This PR removes most extra logic around span hashing and is meant to gauge how much of a performance regression this would cause. Much of the potential performance loss should be recoverable by better factoring queries -- but that's a medium term undertaking and should not block incr. comp. stabilization.

cc @nikomatsakis @Mark-Simulacrum

Author: bors

src/librustc/ich/hcx.rs

@@ -14,7 +14,6 @@ use hir::map::DefPathHash;
 use hir::map::definitions::Definitions;
 use ich::{self, CachingCodemapView};
 use middle::cstore::CrateStore;
-use session::config::DebugInfoLevel::NoDebugInfo;
 use ty::{TyCtxt, fast_reject};
 use session::Session;
 
@@ -24,7 +23,7 @@ use std::cell::RefCell;
 use std::collections::HashMap;
 
 use syntax::ast;
-use syntax::attr;
+
 use syntax::codemap::CodeMap;
 use syntax::ext::hygiene::SyntaxContext;
 use syntax::symbol::Symbol;
@@ -51,7 +50,6 @@ pub struct StableHashingContext<'gcx> {
     body_resolver: BodyResolver<'gcx>,
     hash_spans: bool,
     hash_bodies: bool,
-    overflow_checks_enabled: bool,
     node_id_hashing_mode: NodeIdHashingMode,
 
     // Very often, we are hashing something that does not need the
@@ -89,8 +87,7 @@ impl<'gcx> StableHashingContext<'gcx> {
                definitions: &'gcx Definitions,
                cstore: &'gcx CrateStore)
                -> Self {
-        let hash_spans_initial = sess.opts.debuginfo != NoDebugInfo;
-        let check_overflow_initial = sess.overflow_checks();
+        let hash_spans_initial = !sess.opts.debugging_opts.incremental_ignore_spans;
 
         debug_assert!(ich::IGNORED_ATTRIBUTES.len() > 0);
         IGNORED_ATTR_NAMES.with(|names| {
@@ -110,7 +107,6 @@ impl<'gcx> StableHashingContext<'gcx> {
             raw_codemap: sess.codemap(),
             hash_spans: hash_spans_initial,
             hash_bodies: true,
-            overflow_checks_enabled: check_overflow_initial,
             node_id_hashing_mode: NodeIdHashingMode::HashDefPath,
         }
     }
@@ -120,11 +116,6 @@ impl<'gcx> StableHashingContext<'gcx> {
         self.sess
     }
 
-    pub fn force_span_hashing(mut self) -> Self {
-        self.hash_spans = true;
-        self
-    }
-
     #[inline]
     pub fn while_hashing_hir_bodies<F: FnOnce(&mut Self)>(&mut self,
                                                           hash_bodies: bool,
@@ -174,11 +165,6 @@ impl<'gcx> StableHashingContext<'gcx> {
         self.definitions.node_to_hir_id(node_id)
     }
 
-    #[inline]
-    pub fn hash_spans(&self) -> bool {
-        self.hash_spans
-    }
-
     #[inline]
     pub fn hash_bodies(&self) -> bool {
         self.hash_bodies
@@ -204,58 +190,13 @@ impl<'gcx> StableHashingContext<'gcx> {
         })
     }
 
-    pub fn hash_hir_item_like<F: FnOnce(&mut Self)>(&mut self,
-                                                    item_attrs: &[ast::Attribute],
-                                                    is_const: bool,
-                                                    f: F) {
-        let prev_overflow_checks = self.overflow_checks_enabled;
-        if is_const || attr::contains_name(item_attrs, "rustc_inherit_overflow_checks") {
-            self.overflow_checks_enabled = true;
-        }
+    pub fn hash_hir_item_like<F: FnOnce(&mut Self)>(&mut self, f: F) {
         let prev_hash_node_ids = self.node_id_hashing_mode;
         self.node_id_hashing_mode = NodeIdHashingMode::Ignore;
 
         f(self);
 
         self.node_id_hashing_mode = prev_hash_node_ids;
-        self.overflow_checks_enabled = prev_overflow_checks;
-    }
-
-    #[inline]
-    pub fn binop_can_panic_at_runtime(&self, binop: hir::BinOp_) -> bool
-    {
-        match binop {
-            hir::BiAdd |
-            hir::BiSub |
-            hir::BiShl |
-            hir::BiShr |
-            hir::BiMul => self.overflow_checks_enabled,
-
-            hir::BiDiv |
-            hir::BiRem => true,
-
-            hir::BiAnd |
-            hir::BiOr |
-            hir::BiBitXor |
-            hir::BiBitAnd |
-            hir::BiBitOr |
-            hir::BiEq |
-            hir::BiLt |
-            hir::BiLe |
-            hir::BiNe |
-            hir::BiGe |
-            hir::BiGt => false
-        }
-    }
-
-    #[inline]
-    pub fn unop_can_panic_at_runtime(&self, unop: hir::UnOp) -> bool
-    {
-        match unop {
-            hir::UnDeref |
-            hir::UnNot => false,
-            hir::UnNeg => self.overflow_checks_enabled,
-        }
     }
 }
 

src/librustc/ich/impls_hir.rs

@@ -529,63 +529,9 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::Expr {
                 ref attrs
             } = *self;
 
-            let spans_always_on = match *node {
-                hir::ExprBox(..)        |
-                hir::ExprArray(..)      |
-                hir::ExprCall(..)       |
-                hir::ExprLit(..)        |
-                hir::ExprCast(..)       |
-                hir::ExprType(..)       |
-                hir::ExprIf(..)         |
-                hir::ExprWhile(..)      |
-                hir::ExprLoop(..)       |
-                hir::ExprMatch(..)      |
-                hir::ExprClosure(..)    |
-                hir::ExprBlock(..)      |
-                hir::ExprAssign(..)     |
-                hir::ExprTupField(..)   |
-                hir::ExprAddrOf(..)     |
-                hir::ExprBreak(..)      |
-                hir::ExprAgain(..)      |
-                hir::ExprRet(..)        |
-                hir::ExprYield(..)      |
-                hir::ExprInlineAsm(..)  |
-                hir::ExprRepeat(..)     |
-                hir::ExprTup(..)        |
-                hir::ExprMethodCall(..) |
-                hir::ExprPath(..)       |
-                hir::ExprStruct(..)     |
-                hir::ExprField(..)      => {
-                    // For these we only hash the span when debuginfo is on.
-                    false
-                }
-                // For the following, spans might be significant because of
-                // panic messages indicating the source location.
-                hir::ExprBinary(op, ..) => {
-                    hcx.binop_can_panic_at_runtime(op.node)
-                }
-                hir::ExprUnary(op, _) => {
-                    hcx.unop_can_panic_at_runtime(op)
-                }
-                hir::ExprAssignOp(op, ..) => {
-                    hcx.binop_can_panic_at_runtime(op.node)
-                }
-                hir::ExprIndex(..) => {
-                    true
-                }
-            };
-
-            if spans_always_on {
-                hcx.while_hashing_spans(true, |hcx| {
-                    span.hash_stable(hcx, hasher);
-                    node.hash_stable(hcx, hasher);
-                    attrs.hash_stable(hcx, hasher);
-                });
-            } else {
-                span.hash_stable(hcx, hasher);
-                node.hash_stable(hcx, hasher);
-                attrs.hash_stable(hcx, hasher);
-            }
+            span.hash_stable(hcx, hasher);
+            node.hash_stable(hcx, hasher);
+            attrs.hash_stable(hcx, hasher);
         })
     }
 }
@@ -712,15 +658,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::TraitItem {
             span
         } = *self;
 
-        let is_const = match *node {
-            hir::TraitItemKind::Const(..) |
-            hir::TraitItemKind::Type(..) => true,
-            hir::TraitItemKind::Method(hir::MethodSig { constness, .. }, _) => {
-                constness == hir::Constness::Const
-            }
-        };
-
-        hcx.hash_hir_item_like(attrs, is_const, |hcx| {
+        hcx.hash_hir_item_like(|hcx| {
             name.hash_stable(hcx, hasher);
             attrs.hash_stable(hcx, hasher);
             generics.hash_stable(hcx, hasher);
@@ -757,15 +695,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::ImplItem {
             span
         } = *self;
 
-        let is_const = match *node {
-            hir::ImplItemKind::Const(..) |
-            hir::ImplItemKind::Type(..) => true,
-            hir::ImplItemKind::Method(hir::MethodSig { constness, .. }, _) => {
-                constness == hir::Constness::Const
-            }
-        };
-
-        hcx.hash_hir_item_like(attrs, is_const, |hcx| {
+        hcx.hash_hir_item_like(|hcx| {
             name.hash_stable(hcx, hasher);
             vis.hash_stable(hcx, hasher);
             defaultness.hash_stable(hcx, hasher);
@@ -884,30 +814,6 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::Item {
     fn hash_stable<W: StableHasherResult>(&self,
                                           hcx: &mut StableHashingContext<'gcx>,
                                           hasher: &mut StableHasher<W>) {
-        let is_const = match self.node {
-            hir::ItemStatic(..)      |
-            hir::ItemConst(..)       => {
-                true
-            }
-            hir::ItemFn(_, _, constness, ..) => {
-                constness == hir::Constness::Const
-            }
-            hir::ItemUse(..)         |
-            hir::ItemExternCrate(..) |
-            hir::ItemForeignMod(..)  |
-            hir::ItemGlobalAsm(..)   |
-            hir::ItemMod(..)         |
-            hir::ItemAutoImpl(..) |
-            hir::ItemTrait(..)       |
-            hir::ItemImpl(..)        |
-            hir::ItemTy(..)          |
-            hir::ItemEnum(..)        |
-            hir::ItemStruct(..)      |
-            hir::ItemUnion(..)       => {
-                false
-            }
-        };
-
         let hir::Item {
             name,
             ref attrs,
@@ -918,7 +824,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::Item {
             span
         } = *self;
 
-        hcx.hash_hir_item_like(attrs, is_const, |hcx| {
+        hcx.hash_hir_item_like(|hcx| {
             name.hash_stable(hcx, hasher);
             attrs.hash_stable(hcx, hasher);
             node.hash_stable(hcx, hasher);

src/librustc/ich/impls_mir.rs

@@ -55,48 +55,11 @@ for mir::UnsafetyViolationKind {
         }
     }
 }
-impl<'gcx> HashStable<StableHashingContext<'gcx>>
-for mir::Terminator<'gcx> {
-    #[inline]
-    fn hash_stable<W: StableHasherResult>(&self,
-                                          hcx: &mut StableHashingContext<'gcx>,
-                                          hasher: &mut StableHasher<W>) {
-        let mir::Terminator {
-            ref kind,
-            ref source_info,
-        } = *self;
 
-        let hash_spans_unconditionally = match *kind {
-            mir::TerminatorKind::Assert { .. } => {
-                // Assert terminators generate a panic message that contains the
-                // source location, so we always have to feed its span into the
-                // ICH.
-                true
-            }
-            mir::TerminatorKind::Goto { .. } |
-            mir::TerminatorKind::SwitchInt { .. } |
-            mir::TerminatorKind::Resume |
-            mir::TerminatorKind::Return |
-            mir::TerminatorKind::GeneratorDrop |
-            mir::TerminatorKind::Unreachable |
-            mir::TerminatorKind::Drop { .. } |
-            mir::TerminatorKind::DropAndReplace { .. } |
-            mir::TerminatorKind::Yield { .. } |
-            mir::TerminatorKind::Call { .. } |
-            mir::TerminatorKind::FalseEdges { .. } => false,
-        };
-
-        if hash_spans_unconditionally {
-            hcx.while_hashing_spans(true, |hcx| {
-                source_info.hash_stable(hcx, hasher);
-            })
-        } else {
-            source_info.hash_stable(hcx, hasher);
-        }
-
-        kind.hash_stable(hcx, hasher);
-    }
-}
+impl_stable_hash_for!(struct mir::Terminator<'tcx> {
+    kind,
+    source_info
+});
 
 impl<'gcx, T> HashStable<StableHashingContext<'gcx>> for mir::ClearCrossCrate<T>
     where T: HashStable<StableHashingContext<'gcx>>

src/librustc/session/config.rs

@@ -1084,6 +1084,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
         "dump hash information in textual format to stdout"),
     incremental_verify_ich: bool = (false, parse_bool, [UNTRACKED],
         "verify incr. comp. hashes of green query instances"),
+    incremental_ignore_spans: bool = (false, parse_bool, [UNTRACKED],
+        "ignore spans during ICH computation -- used for testing"),
     dump_dep_graph: bool = (false, parse_bool, [UNTRACKED],
           "dump the dependency graph to $RUST_DEP_GRAPH (default: /tmp/dep_graph.gv)"),
     query_dep_graph: bool = (false, parse_bool, [UNTRACKED],

src/librustc_metadata/astencode.rs

@@ -47,9 +47,7 @@ impl<'a, 'b, 'tcx> IsolatedEncoder<'a, 'b, 'tcx> {
             let mut hasher = StableHasher::new();
 
             hcx.while_hashing_hir_bodies(true, |hcx| {
-                hcx.while_hashing_spans(false, |hcx| {
-                    body.hash_stable(hcx, &mut hasher);
-                });
+                body.hash_stable(hcx, &mut hasher);
             });
 
             hasher.finish()

src/test/incremental/hashes/call_expressions.rs

@@ -18,7 +18,7 @@
 
 // must-compile-successfully
 // revisions: cfail1 cfail2 cfail3
-// compile-flags: -Z query-dep-graph
+// compile-flags: -Z query-dep-graph -Zincremental-ignore-spans
 
 
 #![allow(warnings)]

src/test/incremental/hashes/closure_expressions.rs

@@ -18,7 +18,7 @@
 
 // must-compile-successfully
 // revisions: cfail1 cfail2 cfail3
-// compile-flags: -Z query-dep-graph
+// compile-flags: -Z query-dep-graph -Zincremental-ignore-spans
 
 #![allow(warnings)]
 #![feature(rustc_attrs)]

src/test/incremental/hashes/consts.rs

@@ -18,7 +18,7 @@
 
 // must-compile-successfully
 // revisions: cfail1 cfail2 cfail3
-// compile-flags: -Z query-dep-graph
+// compile-flags: -Z query-dep-graph -Zincremental-ignore-spans
 
 #![allow(warnings)]
 #![feature(rustc_attrs)]

src/test/incremental/hashes/enum_constructors.rs

@@ -18,7 +18,7 @@
 
 // must-compile-successfully
 // revisions: cfail1 cfail2 cfail3
-// compile-flags: -Z query-dep-graph
+// compile-flags: -Z query-dep-graph -Zincremental-ignore-spans
 
 #![allow(warnings)]
 #![feature(rustc_attrs)]

src/test/incremental/hashes/enum_defs.rs

@@ -23,7 +23,7 @@
 
 // must-compile-successfully
 // revisions: cfail1 cfail2 cfail3
-// compile-flags: -Z query-dep-graph
+// compile-flags: -Z query-dep-graph -Zincremental-ignore-spans
 
 #![allow(warnings)]
 #![feature(rustc_attrs)]

src/test/incremental/hashes/exported_vs_not.rs

@@ -10,7 +10,7 @@
 
 // must-compile-successfully
 // revisions: cfail1 cfail2 cfail3
-// compile-flags: -Z query-dep-graph
+// compile-flags: -Z query-dep-graph -Zincremental-ignore-spans
 
 #![allow(warnings)]
 #![feature(rustc_attrs)]