Rollup merge of #72540 - davidtwco:issue-67552-mono-collector-comparison, r=varkor

mir: adjust conditional in recursion limit check

Fixes #67552.

This PR adjusts the condition used in the recursion limit check of
the monomorphization collector, from `>` to `>=`.

In #67552, the test case had infinite indirect recursion, repeating a
handful of functions (from the perspective of the monomorphization
collector): `rec` -> `identity` -> `Iterator::count` -> `Iterator::fold`
-> `Iterator::next` -> `rec`.

During this process, `resolve_associated_item` was invoked for
`Iterator::fold` (during the construction of an `Instance`), and
ICE'd due to substitutions needing inference. However, previous
iterations of this recursion would have called this function for
`Iterator::fold` - and did! - and succeeded in doing so (trivially
checkable from debug logging, `()` is present where `_` is in the substs
of the failing execution).

The expected outcome of this test case would be a recursion limit error
(which is present when the `identity` fn indirection is removed), and
the recursion depth of `rec` is increasing (other functions finish
collecting their neighbours and thus have their recursion depths reset).

When the ICE occurs, the recursion depth of `rec` is 256 (which matches
the recursion limit), which suggests perhaps that a different part of
the compiler is using a `>=` comparison and returning a different result
on this recursion rather than what it returned in every previous
recursion, thus stopping the monomorphization collector from reporting
an error on the next recursion, where `recursion_depth_of_rec > 256`
would have been true.

With grep and some educated guesses, we can determine that
the recursion limit check at line 818 in
`src/librustc_trait_selection/traits/project.rs` is the other check that
is using a different comparison. Modifying either comparison to be `>` or
`>=` respectively will fix the error, but changing the monomorphization
collector produces the nicer error.

Author: RalfJung

src/librustc_error_codes/error_codes/E0055.md

@@ -6,7 +6,7 @@ recursion limit (which can be set via the `recursion_limit` attribute).
 For a somewhat artificial example:
 
 ```compile_fail,E0055
-#![recursion_limit="5"]
+#![recursion_limit="4"]
 
 struct Foo;
 

src/librustc_expand/base.rs

@@ -12,7 +12,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sync::{self, Lrc};
 use rustc_errors::{DiagnosticBuilder, ErrorReported};
 use rustc_parse::{self, parser, MACRO_ARGUMENTS};
-use rustc_session::parse::ParseSess;
+use rustc_session::{parse::ParseSess, Limit};
 use rustc_span::def_id::DefId;
 use rustc_span::edition::Edition;
 use rustc_span::hygiene::{AstPass, ExpnData, ExpnId, ExpnKind};
@@ -941,7 +941,7 @@ pub struct ExpansionData {
 pub struct ExtCtxt<'a> {
     pub parse_sess: &'a ParseSess,
     pub ecfg: expand::ExpansionConfig<'a>,
-    pub reduced_recursion_limit: Option<usize>,
+    pub reduced_recursion_limit: Option<Limit>,
     pub root_path: PathBuf,
     pub resolver: &'a mut dyn Resolver,
     pub current_expansion: ExpansionData,

src/librustc_expand/expand.rs

@@ -24,6 +24,7 @@ use rustc_parse::validate_attr;
 use rustc_session::lint::builtin::UNUSED_DOC_COMMENTS;
 use rustc_session::lint::BuiltinLintDiagnostics;
 use rustc_session::parse::{feature_err, ParseSess};
+use rustc_session::Limit;
 use rustc_span::source_map::respan;
 use rustc_span::symbol::{sym, Ident, Symbol};
 use rustc_span::{FileName, Span, DUMMY_SP};
@@ -664,7 +665,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
     ) -> ExpandResult<AstFragment, Invocation> {
         let recursion_limit =
             self.cx.reduced_recursion_limit.unwrap_or(self.cx.ecfg.recursion_limit);
-        if self.cx.current_expansion.depth > recursion_limit {
+        if !recursion_limit.value_within_limit(self.cx.current_expansion.depth) {
             if self.cx.reduced_recursion_limit.is_none() {
                 self.error_recursion_limit_reached();
             }
@@ -1784,7 +1785,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
 pub struct ExpansionConfig<'feat> {
     pub crate_name: String,
     pub features: Option<&'feat Features>,
-    pub recursion_limit: usize,
+    pub recursion_limit: Limit,
     pub trace_mac: bool,
     pub should_test: bool, // If false, strip `#[test]` nodes
     pub keep_macs: bool,
@@ -1795,7 +1796,7 @@ impl<'feat> ExpansionConfig<'feat> {
         ExpansionConfig {
             crate_name,
             features: None,
-            recursion_limit: 1024,
+            recursion_limit: Limit::new(1024),
             trace_mac: false,
             should_test: false,
             keep_macs: false,

src/librustc_middle/middle/limits.rs

@@ -8,7 +8,7 @@
 use crate::bug;
 use rustc_ast::ast;
 use rustc_data_structures::sync::OnceCell;
-use rustc_session::Session;
+use rustc_session::{Limit, Session};
 use rustc_span::symbol::{sym, Symbol};
 
 use std::num::IntErrorKind;
@@ -22,7 +22,7 @@ pub fn update_limits(sess: &Session, krate: &ast::Crate) {
 fn update_limit(
     sess: &Session,
     krate: &ast::Crate,
-    limit: &OnceCell<usize>,
+    limit: &OnceCell<Limit>,
     name: Symbol,
     default: usize,
 ) {
@@ -34,7 +34,7 @@ fn update_limit(
         if let Some(s) = attr.value_str() {
             match s.as_str().parse() {
                 Ok(n) => {
-                    limit.set(n).unwrap();
+                    limit.set(Limit::new(n)).unwrap();
                     return;
                 }
                 Err(e) => {
@@ -62,5 +62,5 @@ fn update_limit(
             }
         }
     }
-    limit.set(default).unwrap();
+    limit.set(Limit::new(default)).unwrap();
 }

src/librustc_middle/ty/layout.rs

@@ -187,10 +187,9 @@ fn layout_raw<'tcx>(
     query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
 ) -> Result<&'tcx Layout, LayoutError<'tcx>> {
     ty::tls::with_related_context(tcx, move |icx| {
-        let rec_limit = tcx.sess.recursion_limit.get().copied().unwrap();
         let (param_env, ty) = query.into_parts();
 
-        if icx.layout_depth > rec_limit {
+        if !tcx.sess.recursion_limit().value_within_limit(icx.layout_depth) {
             tcx.sess.fatal(&format!("overflow representing the type `{}`", ty));
         }
 

src/librustc_mir/const_eval/machine.rs

@@ -10,6 +10,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_ast::ast::Mutability;
 use rustc_hir::def_id::DefId;
 use rustc_middle::mir::AssertMessage;
+use rustc_session::Limit;
 use rustc_span::symbol::Symbol;
 
 use crate::interpret::{
@@ -109,8 +110,8 @@ pub struct MemoryExtra {
 }
 
 impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
-    pub(super) fn new(const_eval_limit: usize) -> Self {
-        CompileTimeInterpreter { steps_remaining: const_eval_limit, stack: Vec::new() }
+    pub(super) fn new(const_eval_limit: Limit) -> Self {
+        CompileTimeInterpreter { steps_remaining: const_eval_limit.0, stack: Vec::new() }
     }
 }
 

src/librustc_mir/interpret/eval_context.rs

@@ -651,7 +651,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         M::after_stack_push(self)?;
         info!("ENTERING({}) {}", self.frame_idx(), self.frame().instance);
 
-        if self.stack().len() > self.tcx.sess.recursion_limit() {
+        if !self.tcx.sess.recursion_limit().value_within_limit(self.stack().len()) {
             throw_exhaust!(StackFrameLimitReached)
         } else {
             Ok(())

src/librustc_mir/monomorphize/collector.rs

@@ -430,7 +430,7 @@ fn check_recursion_limit<'tcx>(
     // Code that needs to instantiate the same function recursively
     // more than the recursion limit is assumed to be causing an
     // infinite expansion.
-    if adjusted_recursion_depth > tcx.sess.recursion_limit() {
+    if !tcx.sess.recursion_limit().value_within_limit(adjusted_recursion_depth) {
         let error = format!("reached the recursion limit while instantiating `{}`", instance);
         if let Some(def_id) = def_id.as_local() {
             let hir_id = tcx.hir().as_local_hir_id(def_id);
@@ -463,7 +463,7 @@ fn check_type_length_limit<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) {
     // which means that rustc basically hangs.
     //
     // Bail out in these cases to avoid that bad user experience.
-    if type_length > tcx.sess.type_length_limit() {
+    if !tcx.sess.type_length_limit().value_within_limit(type_length) {
         // The instance name is already known to be too long for rustc.
         // Show only the first and last 32 characters to avoid blasting
         // the user's terminal with thousands of lines of type-name.

src/librustc_session/session.rs

@@ -29,8 +29,10 @@ use rustc_target::spec::{Target, TargetTriple, TlsModel};
 
 use std::cell::{self, RefCell};
 use std::env;
+use std::fmt;
 use std::io::Write;
 use std::num::NonZeroU32;
+use std::ops::{Div, Mul};
 use std::path::PathBuf;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -55,6 +57,46 @@ pub enum CtfeBacktrace {
     Immediate,
 }
 
+/// New-type wrapper around `usize` for representing limits. Ensures that comparisons against
+/// limits are consistent throughout the compiler.
+#[derive(Clone, Copy, Debug)]
+pub struct Limit(pub usize);
+
+impl Limit {
+    /// Create a new limit from a `usize`.
+    pub fn new(value: usize) -> Self {
+        Limit(value)
+    }
+
+    /// Check that `value` is within the limit. Ensures that the same comparisons are used
+    /// throughout the compiler, as mismatches can cause ICEs, see #72540.
+    pub fn value_within_limit(&self, value: usize) -> bool {
+        value <= self.0
+    }
+}
+
+impl fmt::Display for Limit {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.0)
+    }
+}
+
+impl Div<usize> for Limit {
+    type Output = Limit;
+
+    fn div(self, rhs: usize) -> Self::Output {
+        Limit::new(self.0 / rhs)
+    }
+}
+
+impl Mul<usize> for Limit {
+    type Output = Limit;
+
+    fn mul(self, rhs: usize) -> Self::Output {
+        Limit::new(self.0 * rhs)
+    }
+}
+
 /// Represents the data associated with a compilation
 /// session for a single crate.
 pub struct Session {
@@ -89,13 +131,13 @@ pub struct Session {
 
     /// The maximum recursion limit for potentially infinitely recursive
     /// operations such as auto-dereference and monomorphization.
-    pub recursion_limit: OnceCell<usize>,
+    pub recursion_limit: OnceCell<Limit>,
 
     /// The maximum length of types during monomorphization.
-    pub type_length_limit: OnceCell<usize>,
+    pub type_length_limit: OnceCell<Limit>,
 
     /// The maximum blocks a const expression can evaluate.
-    pub const_eval_limit: OnceCell<usize>,
+    pub const_eval_limit: OnceCell<Limit>,
 
     incr_comp_session: OneThread<RefCell<IncrCompSession>>,
     /// Used for incremental compilation tests. Will only be populated if
@@ -255,15 +297,15 @@ impl Session {
         self.crate_types.set(crate_types).expect("`crate_types` was initialized twice")
     }
 
-    pub fn recursion_limit(&self) -> usize {
+    pub fn recursion_limit(&self) -> Limit {
         self.recursion_limit.get().copied().unwrap()
     }
 
-    pub fn type_length_limit(&self) -> usize {
+    pub fn type_length_limit(&self) -> Limit {
         self.type_length_limit.get().copied().unwrap()
     }
 
-    pub fn const_eval_limit(&self) -> usize {
+    pub fn const_eval_limit(&self) -> Limit {
         self.const_eval_limit.get().copied().unwrap()
     }
 

src/librustc_trait_selection/traits/project.rs

@@ -333,10 +333,10 @@ impl<'a, 'b, 'tcx> TypeFolder<'tcx> for AssocTypeNormalizer<'a, 'b, 'tcx> {
 
                     Reveal::All => {
                         let recursion_limit = self.tcx().sess.recursion_limit();
-                        if self.depth >= recursion_limit {
+                        if !recursion_limit.value_within_limit(self.depth) {
                             let obligation = Obligation::with_depth(
                                 self.cause.clone(),
-                                recursion_limit,
+                                recursion_limit.0,
                                 self.param_env,
                                 ty,
                             );
@@ -522,7 +522,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
             // But for now, let's classify this as an overflow:
             let recursion_limit = selcx.tcx().sess.recursion_limit();
             let obligation =
-                Obligation::with_depth(cause, recursion_limit, param_env, projection_ty);
+                Obligation::with_depth(cause, recursion_limit.0, param_env, projection_ty);
             selcx.infcx().report_overflow_error(&obligation, false);
         }
         Err(ProjectionCacheEntry::NormalizedTy(ty)) => {
@@ -814,8 +814,7 @@ fn project_type<'cx, 'tcx>(
 ) -> Result<ProjectedTy<'tcx>, ProjectionTyError<'tcx>> {
     debug!("project(obligation={:?})", obligation);
 
-    let recursion_limit = selcx.tcx().sess.recursion_limit();
-    if obligation.recursion_depth >= recursion_limit {
+    if !selcx.tcx().sess.recursion_limit().value_within_limit(obligation.recursion_depth) {
         debug!("project: overflow!");
         return Err(ProjectionTyError::TraitSelectionError(SelectionError::Overflow));
     }

src/librustc_trait_selection/traits/query/normalize.rs

@@ -109,10 +109,10 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for QueryNormalizer<'cx, 'tcx> {
 
                     Reveal::All => {
                         let recursion_limit = self.tcx().sess.recursion_limit();
-                        if self.anon_depth >= recursion_limit {
+                        if !recursion_limit.value_within_limit(self.anon_depth) {
                             let obligation = Obligation::with_depth(
                                 self.cause.clone(),
-                                recursion_limit,
+                                recursion_limit.0,
                                 self.param_env,
                                 ty,
                             );

src/librustc_trait_selection/traits/select.rs

@@ -919,7 +919,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         obligation: &Obligation<'tcx, T>,
         error_obligation: &Obligation<'tcx, V>,
     ) -> Result<(), OverflowError> {
-        if obligation.recursion_depth >= self.infcx.tcx.sess.recursion_limit() {
+        if !self.infcx.tcx.sess.recursion_limit().value_within_limit(obligation.recursion_depth) {
             match self.query_mode {
                 TraitQueryMode::Standard => {
                     self.infcx().report_overflow_error(error_obligation, true);

src/librustc_traits/dropck_outlives.rs

@@ -163,7 +163,7 @@ fn dtorck_constraint_for_ty<'tcx>(
 ) -> Result<(), NoSolution> {
     debug!("dtorck_constraint_for_ty({:?}, {:?}, {:?}, {:?})", span, for_ty, depth, ty);
 
-    if depth >= tcx.sess.recursion_limit() {
+    if !tcx.sess.recursion_limit().value_within_limit(depth) {
         constraints.overflows.push(ty);
         return Ok(());
     }

src/librustc_ty/needs_drop.rs

@@ -5,6 +5,7 @@ use rustc_hir::def_id::DefId;
 use rustc_middle::ty::subst::Subst;
 use rustc_middle::ty::util::{needs_drop_components, AlwaysRequiresDrop};
 use rustc_middle::ty::{self, Ty, TyCtxt};
+use rustc_session::Limit;
 use rustc_span::DUMMY_SP;
 
 type NeedsDropResult<T> = Result<T, AlwaysRequiresDrop>;
@@ -30,7 +31,7 @@ struct NeedsDropTypes<'tcx, F> {
     /// if it needs drop. If the result depends on whether some other types
     /// need drop we push them onto the stack.
     unchecked_tys: Vec<(Ty<'tcx>, usize)>,
-    recursion_limit: usize,
+    recursion_limit: Limit,
     adt_components: F,
 }
 
@@ -66,7 +67,7 @@ where
         let tcx = self.tcx;
 
         while let Some((ty, level)) = self.unchecked_tys.pop() {
-            if level > self.recursion_limit {
+            if !self.recursion_limit.value_within_limit(level) {
                 // Not having a `Span` isn't great. But there's hopefully some other
                 // recursion limit error as well.
                 tcx.sess.span_err(

src/librustc_typeck/check/autoderef.rs

@@ -48,7 +48,7 @@ impl<'a, 'tcx> Iterator for Autoderef<'a, 'tcx> {
             return Some((self.cur_ty, 0));
         }
 
-        if self.steps.len() >= tcx.sess.recursion_limit() {
+        if !tcx.sess.recursion_limit().value_within_limit(self.steps.len()) {
             if !self.silence_errors {
                 report_autoderef_recursion_limit_error(tcx, self.span, self.cur_ty);
             }

src/test/ui/did_you_mean/recursion_limit.stderr

@@ -1,4 +1,4 @@
-error[E0275]: overflow evaluating the requirement `J: std::marker::Send`
+error[E0275]: overflow evaluating the requirement `K: std::marker::Send`
   --> $DIR/recursion_limit.rs:34:5
    |
 LL | fn is_send<T:Send>() { }
@@ -8,6 +8,7 @@ LL |     is_send::<A>();
    |     ^^^^^^^^^^^^
    |
    = help: consider adding a `#![recursion_limit="20"]` attribute to your crate (`recursion_limit`)
+   = note: required because it appears within the type `J`
    = note: required because it appears within the type `I`
    = note: required because it appears within the type `H`
    = note: required because it appears within the type `G`

src/test/ui/did_you_mean/recursion_limit_deref.stderr

@@ -1,4 +1,4 @@
-error[E0055]: reached the recursion limit while auto-dereferencing `I`
+error[E0055]: reached the recursion limit while auto-dereferencing `J`
   --> $DIR/recursion_limit_deref.rs:50:22
    |
 LL |     let x: &Bottom = &t;

src/test/ui/dropck/dropck_no_diverge_on_nonregular_1.stderr

@@ -4,15 +4,15 @@ error[E0320]: overflow while adding drop-check rules for FingerTree<i32>
 LL |     let ft =
    |         ^^
    |
-   = note: overflowed on FingerTree<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<i32>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+   = note: overflowed on FingerTree<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<i32>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 
 error[E0320]: overflow while adding drop-check rules for FingerTree<i32>
   --> $DIR/dropck_no_diverge_on_nonregular_1.rs:25:9
    |
 LL |         FingerTree::Single(1);
    |         ^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: overflowed on FingerTree<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<i32>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+   = note: overflowed on FingerTree<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<Node<i32>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 
 error: aborting due to 2 previous errors