Rollup merge of rust-lang#70087 - ecstatic-morse:remove-const-eval-loop-detector, r=RalfJung

Remove const eval loop detector

Now that there is a configurable instruction limit for CTFE (see rust-lang#67260), we can replace the loop detector with something much simpler. See rust-lang#66946 for more discussion about this. Although the instruction limit is nightly-only, the only practical way to reach the default limit uses nightly-only features as well (although CTFE will still execute code using such features inside an array initializer on stable).

This will at the very least require a crater run, since it will result in an error wherever the "long running const eval" warning appeared before. We may need to increase the default for `const_eval_limit` to work around this.

Resolves rust-lang#54384 cc rust-lang#49980
r? @oli-obk cc @RalfJung

Author: Centril

src/librustc/mir/interpret/error.rs

@@ -494,8 +494,10 @@ impl fmt::Debug for UnsupportedOpInfo {
 pub enum ResourceExhaustionInfo {
     /// The stack grew too big.
     StackFrameLimitReached,
-    /// The program ran into an infinite loop.
-    InfiniteLoop,
+    /// The program ran for too long.
+    ///
+    /// The exact limit is set by the `const_eval_limit` attribute.
+    StepLimitReached,
 }
 
 impl fmt::Debug for ResourceExhaustionInfo {
@@ -505,11 +507,9 @@ impl fmt::Debug for ResourceExhaustionInfo {
             StackFrameLimitReached => {
                 write!(f, "reached the configured maximum number of stack frames")
             }
-            InfiniteLoop => write!(
-                f,
-                "duplicate interpreter state observed here, const evaluation will never \
-                    terminate"
-            ),
+            StepLimitReached => {
+                write!(f, "exceeded interpreter step limit (see `#[const_eval_limit]`)")
+            }
         }
     }
 }

src/librustc_mir/const_eval/machine.rs

@@ -3,7 +3,6 @@ use rustc::ty::layout::HasTyCtxt;
 use rustc::ty::{self, Ty};
 use std::borrow::{Borrow, Cow};
 use std::collections::hash_map::Entry;
-use std::convert::TryFrom;
 use std::hash::Hash;
 
 use rustc_data_structures::fx::FxHashMap;
@@ -13,13 +12,13 @@ use rustc_span::source_map::Span;
 use rustc_span::symbol::Symbol;
 
 use crate::interpret::{
-    self, snapshot, AllocId, Allocation, GlobalId, ImmTy, InterpCx, InterpResult, Memory,
-    MemoryKind, OpTy, PlaceTy, Pointer, Scalar,
+    self, AllocId, Allocation, GlobalId, ImmTy, InterpCx, InterpResult, Memory, MemoryKind, OpTy,
+    PlaceTy, Pointer, Scalar,
 };
 
 use super::error::*;
 
-impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> {
+impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter> {
     /// Evaluate a const function where all arguments (if any) are zero-sized types.
     /// The evaluation is memoized thanks to the query system.
     ///
@@ -86,22 +85,13 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> {
     }
 }
 
-/// The number of steps between loop detector snapshots.
-/// Should be a power of two for performance reasons.
-const DETECTOR_SNAPSHOT_PERIOD: isize = 256;
-
-// Extra machine state for CTFE, and the Machine instance
-pub struct CompileTimeInterpreter<'mir, 'tcx> {
-    /// When this value is negative, it indicates the number of interpreter
-    /// steps *until* the loop detector is enabled. When it is positive, it is
-    /// the number of steps after the detector has been enabled modulo the loop
-    /// detector period.
-    pub(super) steps_since_detector_enabled: isize,
-
-    pub(super) is_detector_enabled: bool,
-
-    /// Extra state to detect loops.
-    pub(super) loop_detector: snapshot::InfiniteLoopDetector<'mir, 'tcx>,
+/// Extra machine state for CTFE, and the Machine instance
+pub struct CompileTimeInterpreter {
+    /// For now, the number of terminators that can be evaluated before we throw a resource
+    /// exhuastion error.
+    ///
+    /// Setting this to `0` disables the limit and allows the interpreter to run forever.
+    pub steps_remaining: usize,
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -110,16 +100,9 @@ pub struct MemoryExtra {
     pub(super) can_access_statics: bool,
 }
 
-impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
+impl CompileTimeInterpreter {
     pub(super) fn new(const_eval_limit: usize) -> Self {
-        let steps_until_detector_enabled =
-            isize::try_from(const_eval_limit).unwrap_or(std::isize::MAX);
-
-        CompileTimeInterpreter {
-            loop_detector: Default::default(),
-            steps_since_detector_enabled: -steps_until_detector_enabled,
-            is_detector_enabled: const_eval_limit != 0,
-        }
+        CompileTimeInterpreter { steps_remaining: const_eval_limit }
     }
 }
 
@@ -173,8 +156,7 @@ impl<K: Hash + Eq, V> interpret::AllocMap<K, V> for FxHashMap<K, V> {
     }
 }
 
-crate type CompileTimeEvalContext<'mir, 'tcx> =
-    InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>;
+crate type CompileTimeEvalContext<'mir, 'tcx> = InterpCx<'mir, 'tcx, CompileTimeInterpreter>;
 
 impl interpret::MayLeak for ! {
     #[inline(always)]
@@ -184,7 +166,7 @@ impl interpret::MayLeak for ! {
     }
 }
 
-impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, 'tcx> {
+impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {
     type MemoryKinds = !;
     type PointerTag = ();
     type ExtraFnVal = !;
@@ -345,26 +327,17 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
     }
 
     fn before_terminator(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
-        if !ecx.machine.is_detector_enabled {
+        // The step limit has already been hit in a previous call to `before_terminator`.
+        if ecx.machine.steps_remaining == 0 {
             return Ok(());
         }
 
-        {
-            let steps = &mut ecx.machine.steps_since_detector_enabled;
-
-            *steps += 1;
-            if *steps < 0 {
-                return Ok(());
-            }
-
-            *steps %= DETECTOR_SNAPSHOT_PERIOD;
-            if *steps != 0 {
-                return Ok(());
-            }
+        ecx.machine.steps_remaining -= 1;
+        if ecx.machine.steps_remaining == 0 {
+            throw_exhaust!(StepLimitReached)
         }
 
-        let span = ecx.frame().span;
-        ecx.machine.loop_detector.observe_and_analyze(*ecx.tcx, span, &ecx.memory, &ecx.stack[..])
+        Ok(())
     }
 
     #[inline(always)]

src/librustc_mir/interpret/memory.rs

@@ -112,25 +112,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for Memory<'mir, 'tcx, M>
     }
 }
 
-// FIXME: Really we shouldn't clone memory, ever. Snapshot machinery should instead
-// carefully copy only the reachable parts.
-impl<'mir, 'tcx, M> Clone for Memory<'mir, 'tcx, M>
-where
-    M: Machine<'mir, 'tcx, PointerTag = (), AllocExtra = ()>,
-    M::MemoryExtra: Copy,
-    M::MemoryMap: AllocMap<AllocId, (MemoryKind<M::MemoryKinds>, Allocation)>,
-{
-    fn clone(&self) -> Self {
-        Memory {
-            alloc_map: self.alloc_map.clone(),
-            extra_fn_ptr_map: self.extra_fn_ptr_map.clone(),
-            dead_alloc_map: self.dead_alloc_map.clone(),
-            extra: self.extra,
-            tcx: self.tcx,
-        }
-    }
-}
-
 impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
     pub fn new(tcx: TyCtxtAt<'tcx>, extra: M::MemoryExtra) -> Self {
         Memory {

src/librustc_mir/interpret/mod.rs

@@ -9,7 +9,6 @@ mod memory;
 mod operand;
 mod operator;
 mod place;
-pub(crate) mod snapshot; // for const_eval
 mod step;
 mod terminator;
 mod traits;

src/librustc_mir/interpret/snapshot.rs

@@ -2,7 +2,6 @@ fn main() {
     // Tests the Collatz conjecture with an incorrect base case (0 instead of 1).
     // The value of `n` will loop indefinitely (4 - 2 - 1 - 4).
     let _ = [(); {
-        //~^ WARNING Constant evaluating a complex constant, this might take some time
         let mut n = 113383; // #20 in https://oeis.org/A006884
         while n != 0 {
         //~^ ERROR `while` is not allowed in a `const`

src/test/ui/consts/const-eval/infinite_loop.rs

@@ -1,5 +1,5 @@
 error[E0658]: `while` is not allowed in a `const`
-  --> $DIR/infinite_loop.rs:7:9
+  --> $DIR/infinite_loop.rs:6:9
    |
 LL | /         while n != 0 {
 LL | |
@@ -14,32 +14,19 @@ LL | |         }
    = help: add `#![feature(const_if_match)]` to the crate attributes to enable
 
 error[E0658]: `if` is not allowed in a `const`
-  --> $DIR/infinite_loop.rs:9:17
+  --> $DIR/infinite_loop.rs:8:17
    |
 LL |             n = if n % 2 == 0 { n/2 } else { 3*n + 1 };
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #49146 <https://github.com/rust-lang/rust/issues/49146> for more information
    = help: add `#![feature(const_if_match)]` to the crate attributes to enable
 
-warning: Constant evaluating a complex constant, this might take some time
-  --> $DIR/infinite_loop.rs:4:18
-   |
-LL |       let _ = [(); {
-   |  __________________^
-LL | |
-LL | |         let mut n = 113383; // #20 in https://oeis.org/A006884
-LL | |         while n != 0 {
-...  |
-LL | |         n
-LL | |     }];
-   | |_____^
-
 error[E0080]: evaluation of constant value failed
-  --> $DIR/infinite_loop.rs:9:20
+  --> $DIR/infinite_loop.rs:8:20
    |
 LL |             n = if n % 2 == 0 { n/2 } else { 3*n + 1 };
-   |                    ^^^^^^^^^^ duplicate interpreter state observed here, const evaluation will never terminate
+   |                    ^^^^^^^^^^ exceeded interpreter step limit (see `#[const_eval_limit]`)
 
 error: aborting due to 3 previous errors
 

src/test/ui/consts/const-eval/infinite_loop.stderr

@@ -1,6 +1,5 @@
 fn main() {
     let _ = [(); {
-        //~^ WARNING Constant evaluating a complex constant, this might take some time
         let mut x = &0;
         let mut n = 0;
         while n < 5 {

src/test/ui/consts/const-eval/issue-52475.rs

@@ -1,5 +1,5 @@
 error[E0658]: `while` is not allowed in a `const`
-  --> $DIR/issue-52475.rs:6:9
+  --> $DIR/issue-52475.rs:5:9
    |
 LL | /         while n < 5 {
 LL | |
@@ -12,24 +12,11 @@ LL | |         }
    = help: add `#![feature(const_loop)]` to the crate attributes to enable
    = help: add `#![feature(const_if_match)]` to the crate attributes to enable
 
-warning: Constant evaluating a complex constant, this might take some time
-  --> $DIR/issue-52475.rs:2:18
-   |
-LL |       let _ = [(); {
-   |  __________________^
-LL | |
-LL | |         let mut x = &0;
-LL | |         let mut n = 0;
-...  |
-LL | |         0
-LL | |     }];
-   | |_____^
-
 error[E0080]: evaluation of constant value failed
-  --> $DIR/issue-52475.rs:8:17
+  --> $DIR/issue-52475.rs:7:17
    |
 LL |             n = (n + 1) % 5;
-   |                 ^^^^^^^^^^^ duplicate interpreter state observed here, const evaluation will never terminate
+   |                 ^^^^^^^^^^^ exceeded interpreter step limit (see `#[const_eval_limit]`)
 
 error: aborting due to 2 previous errors
 

src/test/ui/consts/const-eval/issue-52475.stderr

@@ -1,15 +1,21 @@
 // check-pass
+
 #![feature(const_eval_limit)]
-#![const_eval_limit="1000"]
+#![feature(const_loop, const_if_match)]
 
-const CONSTANT: usize = limit();
+// This needs to be higher than the number of loop iterations since each pass through the loop may
+// hit more than one terminator.
+#![const_eval_limit="4000"]
 
-fn main() {
-    assert_eq!(CONSTANT, 1764);
-}
+const X: usize = {
+    let mut x = 0;
+    while x != 1000 {
+        x += 1;
+    }
 
-const fn limit() -> usize {
-    let x = 42;
+    x
+};
 
-    x * 42
+fn main() {
+    assert_eq!(X, 1000);
 }

src/test/ui/consts/const_limit/const_eval_limit_not_reached.rs

@@ -1,21 +1,18 @@
-// ignore-tidy-linelength
-// only-x86_64
-// check-pass
-// NOTE: We always compile this test with -Copt-level=0 because higher opt-levels
-//       optimize away the const function
-// compile-flags:-Copt-level=0
 #![feature(const_eval_limit)]
-#![const_eval_limit="2"]
+#![feature(const_loop, const_if_match)]
 
-const CONSTANT: usize = limit();
-//~^ WARNING Constant evaluating a complex constant, this might take some time
+#![const_eval_limit="500"]
 
-fn main() {
-    assert_eq!(CONSTANT, 1764);
-}
+const X: usize = {
+    let mut x = 0;
+    while x != 1000 {
+        //~^ ERROR any use of this value will cause an error
+        x += 1;
+    }
 
-const fn limit() -> usize { //~ WARNING Constant evaluating a complex constant, this might take some time
-    let x = 42;
+    x
+};
 
-    x * 42
+fn main() {
+    assert_eq!(X, 1000);
 }

src/test/ui/consts/const_limit/const_eval_limit_reached.rs

@@ -1,16 +1,17 @@
-warning: Constant evaluating a complex constant, this might take some time
-  --> $DIR/const_eval_limit_reached.rs:17:1
+error: any use of this value will cause an error
+  --> $DIR/const_eval_limit_reached.rs:8:11
    |
-LL | / const fn limit() -> usize {
-LL | |     let x = 42;
+LL | / const X: usize = {
+LL | |     let mut x = 0;
+LL | |     while x != 1000 {
+   | |           ^^^^^^^^^ exceeded interpreter step limit (see `#[const_eval_limit]`)
 LL | |
-LL | |     x * 42
-LL | | }
-   | |_^
-
-warning: Constant evaluating a complex constant, this might take some time
-  --> $DIR/const_eval_limit_reached.rs:10:1
+...  |
+LL | |     x
+LL | | };
+   | |__-
    |
-LL | const CONSTANT: usize = limit();
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: `#[deny(const_err)]` on by default
+
+error: aborting due to previous error