move loop detector constants to the module that uses them; make lifetime order in ConstPropagator consistent with Memory

Author: RalfJung

src/librustc_mir/const_eval.rs

@@ -34,6 +34,13 @@ use interpret::{self,
     snapshot,
 };
 
+/// Number of steps until the detector even starts doing anything.
+/// Also, a warning is shown to the user when this number is reached.
+const STEPS_UNTIL_DETECTOR_ENABLED: isize = 1_000_000;
+/// The number of steps between loop detector snapshots.
+/// Should be a power of two for performance reasons.
+const DETECTOR_SNAPSHOT_PERIOD: isize = 256;
+
 pub fn mk_borrowck_eval_cx<'a, 'mir, 'tcx>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     instance: Instance<'tcx>,
@@ -245,7 +252,7 @@ impl<'a, 'mir, 'tcx> CompileTimeInterpreter<'a, 'mir, 'tcx> {
     fn new() -> Self {
         CompileTimeInterpreter {
             loop_detector: Default::default(),
-            steps_since_detector_enabled: -snapshot::STEPS_UNTIL_DETECTOR_ENABLED,
+            steps_since_detector_enabled: -STEPS_UNTIL_DETECTOR_ENABLED,
         }
     }
 }
@@ -349,22 +356,15 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx>
                 return Ok(());
             }
 
-            *steps %= snapshot::DETECTOR_SNAPSHOT_PERIOD;
+            *steps %= DETECTOR_SNAPSHOT_PERIOD;
             if *steps != 0 {
                 return Ok(());
             }
         }
 
-        if ecx.machine.loop_detector.is_empty() {
-            // First run of the loop detector
-
-            // FIXME(#49980): make this warning a lint
-            ecx.tcx.sess.span_warn(ecx.frame().span,
-                "Constant evaluating a complex constant, this might take some time");
-        }
-
         ecx.machine.loop_detector.observe_and_analyze(
             &ecx.tcx,
+            ecx.frame().span,
             &ecx.memory,
             &ecx.stack[..],
         )

src/librustc_mir/interpret/snapshot.rs

@@ -28,13 +28,6 @@ use super::eval_context::{LocalValue, StackPopCleanup};
 use super::{Frame, Memory, Operand, MemPlace, Place, Value};
 use const_eval::CompileTimeInterpreter;
 
-/// Number of steps until the detector even starts doing anything.
-/// Also, a warning is shown to the user when this number is reached.
-pub(crate) const STEPS_UNTIL_DETECTOR_ENABLED: isize = 1_000_000;
-/// The number of steps between loop detector snapshots.
-/// Should be a power of two for performance reasons.
-pub(crate) const DETECTOR_SNAPSHOT_PERIOD: isize = 256;
-
 #[derive(Default)]
 pub(crate) struct InfiniteLoopDetector<'a, 'mir, 'tcx: 'a + 'mir> {
     /// The set of all `EvalSnapshot` *hashes* observed by this detector.
@@ -53,28 +46,31 @@ pub(crate) struct InfiniteLoopDetector<'a, 'mir, 'tcx: 'a + 'mir> {
 
 impl<'a, 'mir, 'tcx> InfiniteLoopDetector<'a, 'mir, 'tcx>
 {
-    /// Returns `true` if the loop detector has not yet observed a snapshot.
-    pub fn is_empty(&self) -> bool {
-        self.hashes.is_empty()
-    }
-
     pub fn observe_and_analyze<'b>(
         &mut self,
         tcx: &TyCtxt<'b, 'tcx, 'tcx>,
+        span: Span,
         memory: &Memory<'a, 'mir, 'tcx, CompileTimeInterpreter<'a, 'mir, 'tcx>>,
         stack: &[Frame<'mir, 'tcx>],
     ) -> EvalResult<'tcx, ()> {
-
+        // Compute stack's hash before copying anything
         let mut hcx = tcx.get_stable_hashing_context();
         let mut hasher = StableHasher::<u64>::new();
         stack.hash_stable(&mut hcx, &mut hasher);
         let hash = hasher.finish();
 
+        // Check if we know that hash already
+        if self.hashes.is_empty() {
+            // FIXME(#49980): make this warning a lint
+            tcx.sess.span_warn(span,
+                "Constant evaluating a complex constant, this might take some time");
+        }
         if self.hashes.insert(hash) {
             // No collision
             return Ok(())
         }
 
+        // We need to make a full copy. NOW things that to get really expensive.
         info!("snapshotting the state of the interpreter");
 
         if self.snapshots.insert(EvalSnapshot::new(memory, stack)) {
@@ -461,6 +457,9 @@ impl<'a, 'mir, 'tcx> Eq for EvalSnapshot<'a, 'mir, 'tcx>
 impl<'a, 'mir, 'tcx> PartialEq for EvalSnapshot<'a, 'mir, 'tcx>
 {
     fn eq(&self, other: &Self) -> bool {
+        // FIXME: This looks to be a *ridicolously expensive* comparison operation.
+        // Doesn't this make tons of copies?  Either `snapshot` is very badly named,
+        // or it does!
         self.snapshot() == other.snapshot()
     }
 }

src/librustc_mir/transform/const_prop.rs

@@ -68,9 +68,9 @@ impl MirPass for ConstProp {
 type Const<'tcx> = (OpTy<'tcx>, Span);
 
 /// Finds optimization opportunities on the MIR.
-struct ConstPropagator<'b, 'a, 'tcx:'a+'b> {
-    ecx: EvalContext<'a, 'b, 'tcx, CompileTimeInterpreter<'a, 'b, 'tcx>>,
-    mir: &'b Mir<'tcx>,
+struct ConstPropagator<'a, 'mir, 'tcx:'a+'mir> {
+    ecx: EvalContext<'a, 'mir, 'tcx, CompileTimeInterpreter<'a, 'mir, 'tcx>>,
+    mir: &'mir Mir<'tcx>,
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     source: MirSource,
     places: IndexVec<Local, Option<Const<'tcx>>>,
@@ -101,12 +101,12 @@ impl<'a, 'b, 'tcx> HasTyCtxt<'tcx> for &'a ConstPropagator<'a, 'b, 'tcx> {
     }
 }
 
-impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
+impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
     fn new(
-        mir: &'b Mir<'tcx>,
+        mir: &'mir Mir<'tcx>,
         tcx: TyCtxt<'a, 'tcx, 'tcx>,
         source: MirSource,
-    ) -> ConstPropagator<'b, 'a, 'tcx> {
+    ) -> ConstPropagator<'a, 'mir, 'tcx> {
         let param_env = tcx.param_env(source.def_id);
         let substs = Substs::identity_for_item(tcx, source.def_id);
         let instance = Instance::new(source.def_id, substs);