Merge #9453

9453: Add first-class limits. r=matklad,lnicola a=rbartlensky

Partially fixes #9286.

This introduces a new `Limits` structure which is passed as an input
to `SourceDatabase`. This makes limits accessible almost everywhere in
the code, since most places have a database in scope.

One downside of this approach is that whenever you query limits, you
essentially do an `Arc::clone` which is less than ideal.

Let me know if I missed anything, or would like me to take a different approach!

Co-authored-by: Robert Bartlensky <[email protected]>

Author: bors[bot]

Cargo.lock

@@ -31,6 +31,7 @@ hir_expand = { path = "../hir_expand", version = "0.0.0" }
 mbe = { path = "../mbe", version = "0.0.0" }
 cfg = { path = "../cfg", version = "0.0.0" }
 tt = { path = "../tt", version = "0.0.0" }
+limit = { path = "../limit", version = "0.0.0" }
 
 [dev-dependencies]
 test_utils = { path = "../test_utils" }

crates/hir_def/Cargo.toml

@@ -15,6 +15,7 @@ use hir_expand::{
     ast_id_map::AstIdMap, hygiene::Hygiene, AstId, ExpandResult, HirFileId, InFile, MacroDefId,
 };
 use la_arena::{Arena, ArenaMap};
+use limit::Limit;
 use profile::Count;
 use rustc_hash::FxHashMap;
 use syntax::{ast, AstNode, AstPtr, SyntaxNodePtr};
@@ -53,10 +54,10 @@ pub struct Expander {
 }
 
 #[cfg(test)]
-const EXPANSION_RECURSION_LIMIT: usize = 32;
+const EXPANSION_RECURSION_LIMIT: Limit = Limit::new(32);
 
 #[cfg(not(test))]
-const EXPANSION_RECURSION_LIMIT: usize = 128;
+const EXPANSION_RECURSION_LIMIT: Limit = Limit::new(128);
 
 impl CfgExpander {
     pub(crate) fn new(
@@ -99,7 +100,7 @@ impl Expander {
         db: &dyn DefDatabase,
         macro_call: ast::MacroCall,
     ) -> Result<ExpandResult<Option<(Mark, T)>>, UnresolvedMacro> {
-        if self.recursion_limit + 1 > EXPANSION_RECURSION_LIMIT {
+        if EXPANSION_RECURSION_LIMIT.check(self.recursion_limit + 1).is_err() {
             cov_mark::hit!(your_stack_belongs_to_me);
             return Ok(ExpandResult::str_err(
                 "reached recursion limit during macro expansion".into(),

crates/hir_def/src/body.rs

@@ -19,6 +19,7 @@ use hir_expand::{
 use hir_expand::{InFile, MacroCallLoc};
 use itertools::Itertools;
 use la_arena::Idx;
+use limit::Limit;
 use rustc_hash::{FxHashMap, FxHashSet};
 use syntax::ast;
 
@@ -49,9 +50,9 @@ use crate::{
     UnresolvedMacro,
 };
 
-const GLOB_RECURSION_LIMIT: usize = 100;
-const EXPANSION_DEPTH_LIMIT: usize = 128;
-const FIXED_POINT_LIMIT: usize = 8192;
+const GLOB_RECURSION_LIMIT: Limit = Limit::new(100);
+const EXPANSION_DEPTH_LIMIT: Limit = Limit::new(128);
+const FIXED_POINT_LIMIT: Limit = Limit::new(8192);
 
 pub(super) fn collect_defs(
     db: &dyn DefDatabase,
@@ -356,7 +357,7 @@ impl DefCollector<'_> {
                 }
 
                 i += 1;
-                if i == FIXED_POINT_LIMIT {
+                if FIXED_POINT_LIMIT.check(i).is_err() {
                     log::error!("name resolution is stuck");
                     break 'outer;
                 }
@@ -925,7 +926,7 @@ impl DefCollector<'_> {
         import_type: ImportType,
         depth: usize,
     ) {
-        if depth > GLOB_RECURSION_LIMIT {
+        if GLOB_RECURSION_LIMIT.check(depth).is_err() {
             // prevent stack overflows (but this shouldn't be possible)
             panic!("infinite recursion in glob imports!");
         }
@@ -1158,7 +1159,7 @@ impl DefCollector<'_> {
         macro_call_id: MacroCallId,
         depth: usize,
     ) {
-        if depth > EXPANSION_DEPTH_LIMIT {
+        if EXPANSION_DEPTH_LIMIT.check(depth).is_err() {
             cov_mark::hit!(macro_expansion_overflow);
             log::warn!("macro expansion is too deep");
             return;

crates/hir_def/src/nameres/collector.rs

@@ -1,11 +1,12 @@
 //! This module resolves `mod foo;` declaration to file.
 use base_db::{AnchoredPath, FileId};
 use hir_expand::name::Name;
+use limit::Limit;
 use syntax::SmolStr;
 
 use crate::{db::DefDatabase, HirFileId};
 
-const MOD_DEPTH_LIMIT: u32 = 32;
+const MOD_DEPTH_LIMIT: Limit = Limit::new(32);
 
 #[derive(Clone, Debug)]
 pub(super) struct ModDir {
@@ -25,7 +26,7 @@ impl ModDir {
     }
     fn child(&self, dir_path: DirPath, root_non_dir_owner: bool) -> Option<ModDir> {
         let depth = self.depth + 1;
-        if depth > MOD_DEPTH_LIMIT {
+        if MOD_DEPTH_LIMIT.check(depth as usize).is_err() {
             log::error!("MOD_DEPTH_LIMIT exceeded");
             cov_mark::hit!(circular_mods);
             return None;

crates/hir_def/src/nameres/mod_resolution.rs

@@ -21,6 +21,7 @@ parser = { path = "../parser", version = "0.0.0" }
 profile = { path = "../profile", version = "0.0.0" }
 tt = { path = "../tt", version = "0.0.0" }
 mbe = { path = "../mbe", version = "0.0.0" }
+limit = { path = "../limit", version = "0.0.0" }
 
 [dev-dependencies]
 test_utils = { path = "../test_utils" }

crates/hir_expand/Cargo.toml

@@ -3,6 +3,7 @@
 use std::sync::Arc;
 
 use base_db::{salsa, SourceDatabase};
+use limit::Limit;
 use mbe::{ExpandError, ExpandResult};
 use parser::FragmentKind;
 use syntax::{
@@ -21,7 +22,7 @@ use crate::{
 ///
 /// If an invocation produces more tokens than this limit, it will not be stored in the database and
 /// an error will be emitted.
-const TOKEN_LIMIT: usize = 524288;
+const TOKEN_LIMIT: Limit = Limit::new(524288);
 
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub enum TokenExpander {
@@ -356,10 +357,12 @@ fn macro_expand_with_arg(
     let ExpandResult { value: tt, err } = macro_rules.expand(db, id, &macro_arg.0);
     // Set a hard limit for the expanded tt
     let count = tt.count();
-    if count > TOKEN_LIMIT {
+    // XXX: Make ExpandResult a real error and use .map_err instead?
+    if TOKEN_LIMIT.check(count).is_err() {
         return ExpandResult::str_err(format!(
             "macro invocation exceeds token limit: produced {} tokens, limit is {}",
-            count, TOKEN_LIMIT,
+            count,
+            TOKEN_LIMIT.inner(),
         ));
     }
 

crates/hir_expand/src/db.rs

@@ -29,6 +29,7 @@ hir_expand = { path = "../hir_expand", version = "0.0.0" }
 base_db = { path = "../base_db", version = "0.0.0" }
 profile = { path = "../profile", version = "0.0.0" }
 syntax = { path = "../syntax", version = "0.0.0" }
+limit = { path = "../limit", version = "0.0.0" }
 
 [dev-dependencies]
 test_utils = { path = "../test_utils" }

crates/hir_ty/Cargo.toml

@@ -9,6 +9,7 @@ use base_db::CrateId;
 use chalk_ir::{cast::Cast, fold::Fold, interner::HasInterner, VariableKind};
 use hir_def::lang_item::LangItemTarget;
 use hir_expand::name::name;
+use limit::Limit;
 use log::{info, warn};
 
 use crate::{
@@ -17,6 +18,8 @@ use crate::{
     Ty, TyBuilder, TyKind,
 };
 
+const AUTODEREF_RECURSION_LIMIT: Limit = Limit::new(10);
+
 pub(crate) enum AutoderefKind {
     Builtin,
     Overloaded,
@@ -63,7 +66,7 @@ impl Iterator for Autoderef<'_> {
             return Some((self.ty.clone(), 0));
         }
 
-        if self.steps.len() >= AUTODEREF_RECURSION_LIMIT {
+        if AUTODEREF_RECURSION_LIMIT.check(self.steps.len() + 1).is_err() {
             return None;
         }
 
@@ -87,8 +90,6 @@ impl Iterator for Autoderef<'_> {
     }
 }
 
-const AUTODEREF_RECURSION_LIMIT: usize = 10;
-
 // FIXME: replace uses of this with Autoderef above
 pub fn autoderef<'a>(
     db: &'a dyn HirDatabase,
@@ -99,7 +100,7 @@ pub fn autoderef<'a>(
     successors(Some(ty), move |ty| {
         deref(db, krate?, InEnvironment { goal: ty, environment: environment.clone() })
     })
-    .take(AUTODEREF_RECURSION_LIMIT)
+    .take(AUTODEREF_RECURSION_LIMIT.inner())
 }
 
 pub(crate) fn deref(

crates/hir_ty/src/autoderef.rs

@@ -65,7 +65,7 @@ pub(crate) fn replace_derive_with_manual_impl(
         current_crate,
         NameToImport::Exact(trait_name.to_string()),
         items_locator::AssocItemSearch::Exclude,
-        Some(items_locator::DEFAULT_QUERY_SEARCH_LIMIT),
+        Some(items_locator::DEFAULT_QUERY_SEARCH_LIMIT.inner()),
     )
     .filter_map(|item| match ModuleDef::from(item.as_module_def_id()?) {
         ModuleDef::Trait(trait_) => Some(trait_),

crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs

@@ -187,7 +187,7 @@ pub fn resolve_completion_edits(
         current_crate,
         NameToImport::Exact(imported_name),
         items_locator::AssocItemSearch::Include,
-        Some(items_locator::DEFAULT_QUERY_SEARCH_LIMIT),
+        Some(items_locator::DEFAULT_QUERY_SEARCH_LIMIT.inner()),
     )
     .filter_map(|candidate| {
         current_module

crates/ide_completion/src/lib.rs

@@ -26,6 +26,7 @@ profile = { path = "../profile", version = "0.0.0" }
 # ide should depend only on the top-level `hir` package. if you need
 # something from some `hir_xxx` subpackage, reexport the API via `hir`.
 hir = { path = "../hir", version = "0.0.0" }
+limit = { path = "../limit", version = "0.0.0" }
 
 [dev-dependencies]
 test_utils = { path = "../test_utils" }

crates/ide_db/Cargo.toml

@@ -282,7 +282,7 @@ fn path_applicable_imports(
                 //
                 // see also an ignored test under FIXME comment in the qualify_path.rs module
                 AssocItemSearch::Exclude,
-                Some(DEFAULT_QUERY_SEARCH_LIMIT),
+                Some(DEFAULT_QUERY_SEARCH_LIMIT.inner()),
             )
             .filter_map(|item| {
                 let mod_path = mod_path(item)?;
@@ -299,7 +299,7 @@ fn path_applicable_imports(
                 current_crate,
                 path_candidate.name.clone(),
                 AssocItemSearch::Include,
-                Some(DEFAULT_QUERY_SEARCH_LIMIT),
+                Some(DEFAULT_QUERY_SEARCH_LIMIT.inner()),
             )
             .filter_map(|item| {
                 import_for_item(
@@ -445,7 +445,7 @@ fn trait_applicable_items(
         current_crate,
         trait_candidate.assoc_item_name.clone(),
         AssocItemSearch::AssocItemsOnly,
-        Some(DEFAULT_QUERY_SEARCH_LIMIT),
+        Some(DEFAULT_QUERY_SEARCH_LIMIT.inner()),
     )
     .filter_map(|input| item_as_assoc(db, input))
     .filter_map(|assoc| {

crates/ide_db/src/helpers/import_assets.rs

@@ -7,6 +7,7 @@ use hir::{
     import_map::{self, ImportKind},
     AsAssocItem, Crate, ItemInNs, ModuleDef, Semantics,
 };
+use limit::Limit;
 use syntax::{ast, AstNode, SyntaxKind::NAME};
 
 use crate::{
@@ -17,7 +18,7 @@ use crate::{
 };
 
 /// A value to use, when uncertain which limit to pick.
-pub const DEFAULT_QUERY_SEARCH_LIMIT: usize = 40;
+pub const DEFAULT_QUERY_SEARCH_LIMIT: Limit = Limit::new(40);
 
 /// Three possible ways to search for the name in associated and/or other items.
 #[derive(Debug, Clone, Copy)]

crates/ide_db/src/items_locator.rs

@@ -0,0 +1,9 @@
+[package]
+name = "limit"
+version = "0.0.0"
+description = "TBD"
+license = "MIT OR Apache-2.0"
+authors = ["rust-analyzer developers"]
+edition = "2018"
+
+[dependencies]

crates/limit/Cargo.toml

@@ -0,0 +1,31 @@
+//! limit defines a struct to enforce limits.
+
+/// Represents a struct used to enforce a numerical limit.
+pub struct Limit {
+    upper_bound: usize,
+}
+
+impl Limit {
+    /// Creates a new limit.
+    #[inline]
+    pub const fn new(upper_bound: usize) -> Self {
+        Self { upper_bound }
+    }
+
+    /// Gets the underlying numeric limit.
+    #[inline]
+    pub const fn inner(&self) -> usize {
+        self.upper_bound
+    }
+
+    /// Checks whether the given value is below the limit.
+    /// Returns `Ok` when `other` is below `self`, and `Err` otherwise.
+    #[inline]
+    pub const fn check(&self, other: usize) -> Result<(), ()> {
+        if other > self.upper_bound {
+            Err(())
+        } else {
+            Ok(())
+        }
+    }
+}