Skip to content

Commit

Permalink
closes #682; reject infinite struct field definitions in analyzer
Browse files Browse the repository at this point in the history
  • Loading branch information
Narasimha1997 committed May 9, 2022
1 parent 4aa56e2 commit 1b5a991
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 4 deletions.
3 changes: 2 additions & 1 deletion crates/analyzer/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
34 changes: 32 additions & 2 deletions crates/analyzer/src/db/queries/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(),
)
}
4 changes: 3 additions & 1 deletion crates/analyzer/src/namespace/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
1 change: 1 addition & 0 deletions crates/analyzer/tests/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
@@ -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
4struct Foo:
^^^ struct `Foo` has infinite size due to recursive definition

error: recursive struct `Bar`
┌─ compile_errors/struct_recursive_cycles.fe:8:8
8struct Bar:
^^^ struct `Bar` has infinite size due to recursive definition

error: recursive struct `Foo2`
┌─ compile_errors/struct_recursive_cycles.fe:12:8
12struct Foo2:
^^^^ struct `Foo2` has infinite size due to recursive definition


Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions newsfragments/682.bugfix.md
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 1b5a991

Please sign in to comment.