From 0b3d0cde8b502802d4d53924975fa82072b60f8d Mon Sep 17 00:00:00 2001 From: Robert Bartlensky Date: Sat, 10 Jul 2021 21:49:17 +0100 Subject: [PATCH] Add `Limit` struct. Fixes #9286. --- Cargo.lock | 8 +++++ crates/hir_def/Cargo.toml | 1 + crates/hir_def/src/body.rs | 7 +++-- crates/hir_def/src/nameres/collector.rs | 13 ++++---- crates/hir_def/src/nameres/mod_resolution.rs | 5 +-- crates/hir_expand/Cargo.toml | 1 + crates/hir_expand/src/db.rs | 9 ++++-- crates/hir_ty/Cargo.toml | 1 + crates/hir_ty/src/autoderef.rs | 9 +++--- .../replace_derive_with_manual_impl.rs | 2 +- crates/ide_completion/src/lib.rs | 2 +- crates/ide_db/Cargo.toml | 1 + crates/ide_db/src/helpers/import_assets.rs | 6 ++-- crates/ide_db/src/items_locator.rs | 3 +- crates/limit/Cargo.toml | 9 ++++++ crates/limit/src/lib.rs | 31 +++++++++++++++++++ 16 files changed, 84 insertions(+), 24 deletions(-) create mode 100644 crates/limit/Cargo.toml create mode 100644 crates/limit/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index a3fa70bdb3d..7c1db12df27 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -487,6 +487,7 @@ dependencies = [ "indexmap", "itertools", "la-arena", + "limit", "log", "mbe", "once_cell", @@ -508,6 +509,7 @@ dependencies = [ "either", "expect-test", "la-arena", + "limit", "log", "mbe", "parser", @@ -534,6 +536,7 @@ dependencies = [ "hir_expand", "itertools", "la-arena", + "limit", "log", "once_cell", "profile", @@ -640,6 +643,7 @@ dependencies = [ "fst", "hir", "itertools", + "limit", "log", "once_cell", "profile", @@ -792,6 +796,10 @@ dependencies = [ "cc", ] +[[package]] +name = "limit" +version = "0.0.0" + [[package]] name = "lock_api" version = "0.4.4" diff --git a/crates/hir_def/Cargo.toml b/crates/hir_def/Cargo.toml index e47824c56ea..09a9fb27ba7 100644 --- a/crates/hir_def/Cargo.toml +++ b/crates/hir_def/Cargo.toml @@ -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" } diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs index c521879c873..81956f35d62 100644 --- a/crates/hir_def/src/body.rs +++ b/crates/hir_def/src/body.rs @@ -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>, 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(), diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index f4f481f4dc0..3a5559a4ca0 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.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, @@ -357,7 +358,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; } @@ -924,7 +925,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!"); } @@ -1157,7 +1158,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; diff --git a/crates/hir_def/src/nameres/mod_resolution.rs b/crates/hir_def/src/nameres/mod_resolution.rs index d9cec0e27ac..04427ffc801 100644 --- a/crates/hir_def/src/nameres/mod_resolution.rs +++ b/crates/hir_def/src/nameres/mod_resolution.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 { 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; diff --git a/crates/hir_expand/Cargo.toml b/crates/hir_expand/Cargo.toml index f7817c9271d..92d6c3e96a1 100644 --- a/crates/hir_expand/Cargo.toml +++ b/crates/hir_expand/Cargo.toml @@ -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" } diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index 66f44202ba3..7c83fcd639c 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -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, ¯o_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(), )); } diff --git a/crates/hir_ty/Cargo.toml b/crates/hir_ty/Cargo.toml index f8280e0c5c4..134a7892d61 100644 --- a/crates/hir_ty/Cargo.toml +++ b/crates/hir_ty/Cargo.toml @@ -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" } diff --git a/crates/hir_ty/src/autoderef.rs b/crates/hir_ty/src/autoderef.rs index fbbb0c8208a..267d08d69a6 100644 --- a/crates/hir_ty/src/autoderef.rs +++ b/crates/hir_ty/src/autoderef.rs @@ -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( diff --git a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs index f9474c9f583..73b623d1222 100644 --- a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.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_), diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index a9f32a42acf..386b6bf0e76 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.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 diff --git a/crates/ide_db/Cargo.toml b/crates/ide_db/Cargo.toml index b99089e08fd..f8a2cbef8df 100644 --- a/crates/ide_db/Cargo.toml +++ b/crates/ide_db/Cargo.toml @@ -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" } diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 58fea3b1de5..5017c7f5225 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -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| { diff --git a/crates/ide_db/src/items_locator.rs b/crates/ide_db/src/items_locator.rs index 39aa47fc903..1ececb6c857 100644 --- a/crates/ide_db/src/items_locator.rs +++ b/crates/ide_db/src/items_locator.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)] diff --git a/crates/limit/Cargo.toml b/crates/limit/Cargo.toml new file mode 100644 index 00000000000..7676f3644b3 --- /dev/null +++ b/crates/limit/Cargo.toml @@ -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] diff --git a/crates/limit/src/lib.rs b/crates/limit/src/lib.rs new file mode 100644 index 00000000000..8c96c748dbf --- /dev/null +++ b/crates/limit/src/lib.rs @@ -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(()) + } + } +}