Merge #6467

6467: Don't stack overflow on circular modules r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

Author: bors[bot]

crates/hir_def/src/nameres/collector.rs

@@ -1116,17 +1116,20 @@ impl ModCollector<'_, '_> {
                     &self.item_tree[module.visibility],
                 );
 
-                ModCollector {
-                    def_collector: &mut *self.def_collector,
-                    macro_depth: self.macro_depth,
-                    module_id,
-                    file_id: self.file_id,
-                    item_tree: self.item_tree,
-                    mod_dir: self.mod_dir.descend_into_definition(&module.name, path_attr),
-                }
-                .collect(&*items);
-                if is_macro_use {
-                    self.import_all_legacy_macros(module_id);
+                if let Some(mod_dir) = self.mod_dir.descend_into_definition(&module.name, path_attr)
+                {
+                    ModCollector {
+                        def_collector: &mut *self.def_collector,
+                        macro_depth: self.macro_depth,
+                        module_id,
+                        file_id: self.file_id,
+                        item_tree: self.item_tree,
+                        mod_dir,
+                    }
+                    .collect(&*items);
+                    if is_macro_use {
+                        self.import_all_legacy_macros(module_id);
+                    }
                 }
             }
             // out of line module, resolve, parse and recurse

crates/hir_def/src/nameres/mod_resolution.rs

@@ -2,9 +2,12 @@
 use base_db::FileId;
 use hir_expand::name::Name;
 use syntax::SmolStr;
+use test_utils::mark;
 
 use crate::{db::DefDatabase, HirFileId};
 
+const MOD_DEPTH_LIMIT: u32 = 32;
+
 #[derive(Clone, Debug)]
 pub(super) struct ModDir {
     /// `` for `mod.rs`, `lib.rs`
@@ -14,18 +17,28 @@ pub(super) struct ModDir {
     dir_path: DirPath,
     /// inside `./foo.rs`, mods with `#[path]` should *not* be relative to `./foo/`
     root_non_dir_owner: bool,
+    depth: u32,
 }
 
 impl ModDir {
     pub(super) fn root() -> ModDir {
-        ModDir { dir_path: DirPath::empty(), root_non_dir_owner: false }
+        ModDir { dir_path: DirPath::empty(), root_non_dir_owner: false, depth: 0 }
+    }
+    fn child(&self, dir_path: DirPath, root_non_dir_owner: bool) -> Option<ModDir> {
+        let depth = self.depth + 1;
+        if depth > MOD_DEPTH_LIMIT {
+            log::error!("MOD_DEPTH_LIMIT exceeded");
+            mark::hit!(circular_mods);
+            return None;
+        }
+        Some(ModDir { dir_path, root_non_dir_owner, depth })
     }
 
     pub(super) fn descend_into_definition(
         &self,
         name: &Name,
         attr_path: Option<&SmolStr>,
-    ) -> ModDir {
+    ) -> Option<ModDir> {
         let path = match attr_path.map(|it| it.as_str()) {
             None => {
                 let mut path = self.dir_path.clone();
@@ -40,7 +53,7 @@ impl ModDir {
                 DirPath::new(path)
             }
         };
-        ModDir { dir_path: path, root_non_dir_owner: false }
+        self.child(path, false)
     }
 
     pub(super) fn resolve_declaration(
@@ -72,7 +85,9 @@ impl ModDir {
                 } else {
                     (DirPath::new(format!("{}/", name)), true)
                 };
-                return Ok((file_id, is_mod_rs, ModDir { dir_path, root_non_dir_owner }));
+                if let Some(mod_dir) = self.child(dir_path, root_non_dir_owner) {
+                    return Ok((file_id, is_mod_rs, mod_dir));
+                }
             }
         }
         Err(candidate_files.remove(0))

crates/hir_def/src/nameres/tests.rs

@@ -20,9 +20,8 @@ fn compute_crate_def_map(fixture: &str) -> Arc<CrateDefMap> {
 }
 
 fn check(ra_fixture: &str, expect: Expect) {
-    let db = TestDB::with_files(ra_fixture);
-    let krate = db.crate_graph().iter().next().unwrap();
-    let actual = db.crate_def_map(krate).dump();
+    let def_map = compute_crate_def_map(ra_fixture);
+    let actual = def_map.dump();
     expect.assert_eq(&actual);
 }
 

crates/hir_def/src/nameres/tests/mod_resolution.rs

@@ -771,3 +771,30 @@ struct X;
         "#]],
     );
 }
+
+#[test]
+fn circular_mods() {
+    mark::check!(circular_mods);
+    compute_crate_def_map(
+        r#"
+//- /lib.rs
+mod foo;
+//- /foo.rs
+#[path = "./foo.rs"]
+mod foo;
+"#,
+    );
+
+    compute_crate_def_map(
+        r#"
+//- /lib.rs
+mod foo;
+//- /foo.rs
+#[path = "./bar.rs"]
+mod bar;
+//- /bar.rs
+#[path = "./foo.rs"]
+mod foo;
+"#,
+    );
+}