diff --git a/crates/analyzer/src/db.rs b/crates/analyzer/src/db.rs index eb0b56238a..350954041d 100644 --- a/crates/analyzer/src/db.rs +++ b/crates/analyzer/src/db.rs @@ -154,8 +154,9 @@ pub trait AnalyzerDb: SourceDb + Upcast + UpcastMut fn struct_all_functions(&self, id: StructId) -> Rc<[FunctionId]>; #[salsa::invoke(queries::structs::struct_function_map)] fn struct_function_map(&self, id: StructId) -> Analysis>>; + #[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; // Event #[salsa::invoke(queries::events::event_type)] diff --git a/crates/analyzer/src/db/queries/structs.rs b/crates/analyzer/src/db/queries/structs.rs index b6ade4be5b..8cf77cb35d 100644 --- a/crates/analyzer/src/db/queries/structs.rs +++ b/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 { // 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 { + 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(), + ) } diff --git a/crates/analyzer/src/namespace/items.rs b/crates/analyzer/src/namespace/items.rs index e414a60500..3011b43baa 100644 --- a/crates/analyzer/src/namespace/items.rs +++ b/crates/analyzer/src/namespace/items.rs @@ -1340,10 +1340,12 @@ impl StructId { ) } pub fn dependency_graph(&self, db: &dyn AnalyzerDb) -> Rc { - 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)); diff --git a/crates/analyzer/tests/errors.rs b/crates/analyzer/tests/errors.rs index 73ef6e8d43..8b3fa97164 100644 --- a/crates/analyzer/tests/errors.rs +++ b/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 } diff --git a/crates/analyzer/tests/snapshots/errors__struct_recursive_cycles.snap b/crates/analyzer/tests/snapshots/errors__struct_recursive_cycles.snap new file mode 100644 index 0000000000..cad9b7eefa --- /dev/null +++ b/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 + + diff --git a/crates/test-files/fixtures/compile_errors/struct_recursive_cycles.fe b/crates/test-files/fixtures/compile_errors/struct_recursive_cycles.fe new file mode 100644 index 0000000000..a5d67c93c0 --- /dev/null +++ b/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 diff --git a/newsfragments/682.bugfix.md b/newsfragments/682.bugfix.md new file mode 100644 index 0000000000..aa78de2551 --- /dev/null +++ b/newsfragments/682.bugfix.md @@ -0,0 +1,3 @@ +reject infinite size struct definitions. + +Fe `structs` having infinite size due to recursive definitions were not rejected earlier and would cause ICE in the analyzer since they were not properly handled. Now `structs` having infinite size are properly identified by detecting cycles in the dependency graph of the struct field definitions and an error is thrown by the analyzer.