Fix: reject infinite struct field definitions in analyzer, fixes: eth…

…ereum#682

crates/analyzer/src/db.rs

@@ -154,8 +154,9 @@ pub trait AnalyzerDb: SourceDb + Upcast<dyn SourceDb> + UpcastMut<dyn SourceDb>
     fn struct_all_functions(&self, id: StructId) -> Rc<[FunctionId]>;
     #[salsa::invoke(queries::structs::struct_function_map)]
     fn struct_function_map(&self, id: StructId) -> Analysis<Rc<IndexMap<SmolStr, FunctionId>>>;
+    #[salsa::cycle(queries::structs::struct_cycle)]
     #[salsa::invoke(queries::structs::struct_dependency_graph)]
-    fn struct_dependency_graph(&self, id: StructId) -> DepGraphWrapper;
+    fn struct_dependency_graph(&self, id: StructId) -> Analysis<DepGraphWrapper>;
 
     // Event
     #[salsa::invoke(queries::events::event_type)]

crates/analyzer/src/db/queries/structs.rs

@@ -184,12 +184,16 @@ pub fn struct_function_map(
     Analysis::new(Rc::new(map), scope.diagnostics.take().into())
 }
 
-pub fn struct_dependency_graph(db: &dyn AnalyzerDb, struct_: StructId) -> DepGraphWrapper {
+pub fn struct_dependency_graph(
+    db: &dyn AnalyzerDb,
+    struct_: StructId,
+) -> Analysis<DepGraphWrapper> {
     // A struct depends on the types of its fields and on everything they depend on.
     // It *does not* depend on its public functions; those will only be part of
     // the broader dependency graph if they're in the call graph of some public
     // contract function.
 
+    let scope = ItemScope::new(db, struct_.module(db));
     let root = Item::Type(TypeDef::Struct(struct_));
     let fields = struct_
         .fields(db)
@@ -214,5 +218,31 @@ pub fn struct_dependency_graph(db: &dyn AnalyzerDb, struct_: StructId) -> DepGra
             graph.extend(subgraph.all_edges())
         }
     }
-    DepGraphWrapper(Rc::new(graph))
+
+    Analysis::new(
+        DepGraphWrapper(Rc::new(graph)),
+        scope.diagnostics.take().into(),
+    )
+}
+
+pub fn struct_cycle(
+    db: &dyn AnalyzerDb,
+    _cycle: &[String],
+    struct_: &StructId,
+) -> Analysis<DepGraphWrapper> {
+    let mut scope = ItemScope::new(db, struct_.module(db));
+    let struct_data = &struct_.data(db).ast;
+    scope.error(
+        &format!("recursive struct `{}`", struct_data.name()),
+        struct_data.kind.name.span,
+        &format!(
+            "struct `{}` has infinite size due to recursive definition",
+            struct_data.name(),
+        ),
+    );
+
+    Analysis::new(
+        DepGraphWrapper(Rc::new(DepGraph::new())),
+        scope.diagnostics.take().into(),
+    )
 }

crates/analyzer/src/namespace/items.rs

@@ -1340,10 +1340,12 @@ impl StructId {
         )
     }
     pub fn dependency_graph(&self, db: &dyn AnalyzerDb) -> Rc<DepGraph> {
-        db.struct_dependency_graph(*self).0
+        db.struct_dependency_graph(*self).value.0
     }
     pub fn sink_diagnostics(&self, db: &dyn AnalyzerDb, sink: &mut impl DiagnosticSink) {
         sink.push_all(db.struct_field_map(*self).diagnostics.iter());
+        sink.push_all(db.struct_dependency_graph(*self).diagnostics.iter());
+
         db.struct_all_fields(*self)
             .iter()
             .for_each(|id| id.sink_diagnostics(db, sink));

crates/analyzer/tests/errors.rs

@@ -306,6 +306,7 @@ test_file! { strict_boolean_if_else }
 test_file! { struct_private_constructor }
 test_file! { struct_call_bad_args }
 test_file! { struct_call_without_kw_args }
+test_file! { struct_recursive_cycles }
 test_file! { non_pub_init }
 test_file! { init_wrong_return_type }
 test_file! { init_duplicate_def }

crates/analyzer/tests/snapshots/errors__struct_recursive_cycles.snap

@@ -0,0 +1,23 @@
+---
+source: crates/analyzer/tests/errors.rs
+expression: "error_string(&path, test_files::fixture(path))"
+---
+error: recursive struct `Foo`
+  ┌─ compile_errors/struct_recursive_cycles.fe:4:8
+  │
+4 │ struct Foo:
+  │        ^^^ struct `Foo` has infinite size due to recursive definition
+
+error: recursive struct `Bar`
+  ┌─ compile_errors/struct_recursive_cycles.fe:8:8
+  │
+8 │ struct Bar:
+  │        ^^^ struct `Bar` has infinite size due to recursive definition
+
+error: recursive struct `Foo2`
+   ┌─ compile_errors/struct_recursive_cycles.fe:12:8
+   │
+12 │ struct Foo2:
+   │        ^^^^ struct `Foo2` has infinite size due to recursive definition
+
+

crates/test-files/fixtures/compile_errors/struct_recursive_cycles.fe

@@ -0,0 +1,22 @@
+
+# structs `Foo` and `Bar` contains indirect recursion
+# struct Foo depends on `Bar`
+struct Foo:
+    x: Bar
+
+# struct Bar depends on `Foo`
+struct Bar:
+    x: Foo
+
+# struct `Foo2` depends on itself - contains direct recursion
+struct Foo2:
+    x: Foo2
+
+# struct `Bar2` has no recursion
+struct Bar2:
+    x: Foo
+    y: Bar
+
+contract MyContract:
+    pub fn func(foo: Foo, foo2: Foo2, bar2: Bar2):
+       pass